Allow scoping rules through subninja #921
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
March 2, 2015 00:57
Ninja didn't support scoping rules through subninja and assumed a unique rule name in the whole namespace. With this change, this behavior is changed to allow scoping rules. Two rules can have the same name if they belong to two different scopes. However, two rules can NOT have the same name in the same scope.
The new test shows the added value of scoped rules by demonstrating a multi-level build where a single rules file gets included at all the levels. By scoping rules, this is possible.
|
Let's do this. I'm not super happy that Rule is moving into EvalEnv, but I can try to move it around later if I manage to figure out a better way of doing this. (Chances are I won't either.) Also, this needed some minor changes to build with libc++ for me, see nico@f6461f0 . I'll apply that myself after merging. Thanks for the patch! |
nico
added a commit
that referenced
this pull request
Mar 9, 2015
Allow scoping rules through subninja
nico
added a commit
to nico/ninja
that referenced
this pull request
Mar 9, 2015
It failed with
error: field has incomplete type 'EvalString'
note: in instantiation of exception specification for 'map' requested here
explicit Rule(const string& name) : name_(name) {}
^
nico
added a commit
to nico/ninja
that referenced
this pull request
Mar 9, 2015
It failed with
error: field has incomplete type 'EvalString'
note: in instantiation of exception specification for 'map' requested here
explicit Rule(const string& name) : name_(name) {}
^
| string err; | ||
| EXPECT_TRUE(parser.ParseTest("include rules.ninja\n" | ||
| "subninja test.ninja\n" | ||
| "build y : cat\n", &err)); |
There was a problem hiding this comment.
This triggers an assert in debug builds (./configure --debug):
[166/198] ParserTest.DuplicateRuleInDifferentSubninjasAssertion failed: (LookupRule(rule->name()) == NULL),
function AddRule, file src/eval_env.cc, line 33.
I guess AddRule needs to assert with LookupRuleCurrentScope now?
There was a problem hiding this comment.
Yes. Sorry for that!
nico
added a commit
to nico/ninja
that referenced
this pull request
Apr 9, 2015
nico
added a commit
that referenced
this pull request
Apr 9, 2015
Fix an assert (and tests in --debug mode) after #921.
nico
added a commit
to nico/shipshape
that referenced
this pull request
Jun 2, 2015
rules.ninja only defines rules. Up to ninja 1.5.x, include and subninja do the same thing for ninja files that only define rules. In ninja 1.6, we hope to change rules to be scoped to the current subninja ( ninja-build/ninja#921 ). With that change, `subninja rules.ninja` would define the rules in a new scope that closes at the end of the line and all the rules in rules.ninja would be ignores. `include` won't introduce a new scope and instead add the rules in the context of the parent build.ninja file. This change should have no effect on ninja up to 1.5.x, and it should keep the build working as previously with newer ninja versions.
nico
added a commit
to nico/ninja
that referenced
this pull request
Jun 19, 2015
nico
added a commit
to nico/ninja
that referenced
this pull request
Jun 19, 2015
nico
added a commit
that referenced
this pull request
Jun 19, 2015
Document the change from #921 in the manual.
This was referenced Jun 21, 2015
jimon
pushed a commit
to jimon/ninja
that referenced
this pull request
Aug 29, 2015
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ninja didn't support scoping rules through subninja and assumed
a unique rule name in the whole namespace. With this change, this
behavior is changed to allow scoping rules. Two rules can have the
same name if they belong to two different scopes. However, two
rules can NOT have the same name in the same scope.