- changed status to open
- removed comment
Handle requirements recursively
[This patch includes CreateConfigurationBindings-cleanup.patch (#685):
once CreateConfigurationBindings-cleanup.patch has been applied, the
diffs in this patch will be much reduced in size.]
Handle requirements recursively: If A requires B, and B requires C, then A also requires C. This is necessary e.g. for include directories: If A includes a file from B, which in turn includes a file from C, then C's include directory must be in the search path of A.
Complete implementation of INCLUDE directives when parsing configuration.ccl scripts.
Add more stringent checking of capability names: Only identifiers are allowed. This prevents the capability ":" from appearing when syntax errors in configuration.ccl files are not detected.
Keyword:
Comments (9)
-
reporter -
- removed comment
The patch failed when trying to apply it after
#685 -
- removed comment
The intent of the patch looks fine - the include directories and the restrictions on capability names. Please apply, unless you also feel a code-review is needed, in which case it would be good to provide a patch with just the new features (i.e. after
#685). -
- removed comment
Erik, please comment
-
- removed comment
Erik? Should we move the issue back to 'open'?
-
- changed status to open
- assigned issue to
- changed milestone to ET_2012_05
- removed comment
-
- removed comment
I have attached a cleaner version of the original patch, now taking into account that
#685has been applied. I have also split the patch in 3 pieces corresponding to the 3 files modified just for clarity. The changes are basically the following:1) The ConfigScriptParser.pl patch only add the include statements appearing in the configuration.ccl. In my opinion is ready to be applied.
2) ConfigurationParser.pl patch simply implements logic regarding illegal capabilities names. I would suggest to modify the error message to emphasize that it is its name that it is illegal: "Illegal optional capability name ..."
3) CreateConfigurationBindings.pl patch implements the recursive requires per se. The logic seems correct and it doesn't break current configuration. However I haven't tested enough to say about corner cases (eg A requires B that requires C that requires A). The logic is there, but I haven't tested. I would recommend to apply this patch after someone take another look (it is much cleaner now).
Thanks, Bruno.
-
- changed status to open
- removed comment
-
reporter - changed status to resolved
- removed comment
Applied.
- Log in to comment