Handle requirements recursively

Issue #686 closed
Erik Schnetter created an issue

[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 (10)

  1. Ian Hinder
    • 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).

  2. Bruno Mundim
    • removed comment

    I have attached a cleaner version of the original patch, now taking into account that #685 has 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.

  3. Log in to comment