| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
| |
This is step 1 on #931. Duplicated edges will become an error by default in
the future.
|
| |
|
|
|
|
|
|
|
|
| |
This will make it easier to optionally make this an error (because
ManifestParser has a way of printing errors), and it'll also make
it easier to make the tests quiet again.
No behavior change.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
Patch #933 fixed a crash with duplicate edges by not adding edges to the
graph if all the edge's outputs are already built by other edges.
However, it added the edge to the out_edges of the edge's input nodes
before deleting it, letting inputs refer to dead edges.
To fix, move the check for deleting an edge above the code that adds
inputs. Expand VerifyGraph() to check that nodes don't refer to edges
that aren't present in the state.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Fixes #867, both the crashes and "[stuck]" issues.
The problem was that a duplicate edge would modify the in_edge of the
outputs of the new build rule, but the edge corresponding to the old
build rule would still think that the in_edge points to itself.
`old_edge->outputs_[0]->in_edge()` would not return `old_edge`, which
confused the scan logic.
As fix, let `State::AddOut()` reject changing in_edge if it's already
set. This changes behavior in a minor way: Previously, if there were
multiple edges for a single output, the last edge would be kept. Now,
the first edge is kept. This only had mostly-well-defined semantics if
all duplicate edges are the same (which is the only case I've seen in
practice), and for that case the behavior doesn't change.
For testing, add a VerifyGraph() function and call that every time any
test graph is parsed. That's a bit more code than just copying the test
cases from the bug into build_test.cc, but it also yields better test
coverage overall.
|
|
|
|
|
|
|
|
| |
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.
|
|
|
|
|
|
|
| |
Seems more correct to name it has_indent_token and to use the
unix_hacker style.
Signed-off-by: Thiago Farina <tfarina@chromium.org>
|
| |
|
| |
|
| |
|
| |
|
| |
|
|\
| |
| | |
Introduce the "console" pool
|
| |
| |
| |
| |
| |
| |
| | |
This is a pre-defined pool with a depth of 1. It has the special property
that any task in the pool has direct access to the console. This can be
useful for interactive tasks or long-running tasks which produce status
updates on the console (such as test suites).
|
|/
|
|
|
|
|
| |
In chrome, only 2000 of 22000 build edges have bindings. A BindingEnv is
64 bytes, so allocating these only when needed saves a bit over 1 MB of
memory. Since env chains are shorter for lookups, builds also become a
tiny bit faster.
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
Ninja regressed to include a location for every file on the include stack for
nested diagnostics, i.e. it would print:
input:1: include.ninja:1: expected path
Fix this so that it prints only the current file location, like it used to:
include.ninja:1: expected path
Also add a test for this.
|
| |
|
|
|
|
|
|
| |
Loads of included ninja files were covered by the ".ninja parse" in Parse()
further up the stack from the toplevel file, but the load of the toplevel
file wasn't counted. Fix that.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
ParseFileInclude() was doing the work that Load() was doing. Instead, just
make it call Load().
Also, remove a FIXME about using ReadPath() in ParseFileInclude() -- it's
already being used. (The FIXME was added in the same commit that added the
call to ReadPath() -- 8a0c96075786c19 -- it probably should've been deleted
before that commit.)
Also, remove a `contents.resize(contents.size() + 10);`. It's not clear what
it's for, but if it was important then ManifestParser::ParseFileInclude()
would have needed that call too, and it didn't have it.
No intended behavior change.
|
|
|
|
|
| |
Also add more tests for invalid manifests. According to gcov, these invalid
inputs weren't tested before.
|
|
|
|
|
| |
This attempts to fix issue #600. `man atol` claims that atol() is in
stdlib.h, which wasn't included yet.
|
| |
|
| |
|
| |
|
|
|
|
|
| |
Now, a 'build' block can override any special binding like 'command'
or 'description' if it needs to.
|
| |
|
| |
|
| |
|
| |
|
| |
|
| |
|
|
So it matches with the class name in there.
Signed-off-by: Thiago Farina <tfarina@chromium.org>
|