PR #2487 introduced a regression, where a completed command without
an output would force a newline, preventing the next status update
to appear on the same line in smart terminals.
This fixes the issue by adding the missing `!outputs.empty()`
condition + adding a proper regression test to catch future
breaks.
Fixed: #2499
The parser used asserts to verify the consistency of the
deps log, which resulted in crashes in production. Replace
them with correct checks and return conditions.
This allows the parser to survive and report corrupted
files, and Ninja to ignore the latter.
+ Add related unit-test.
Fixes#2472Fixes#2309
Call fopen() with "wb" instead of "w" to avoid translating
\n into \r\n on Windows, based on the machine's configuration.
This prevents random test breakages on Windows/x64 CI builders
that do not happen on Windows/arm64 or other operating systems.
For most actions, the depfile will be in the same directory as one
of its outputs, and their parent directory will be created by Ninja
before running the command. However, this is not always the case.
In particular, the GN build tool is changing its Ninja build plan
generation logic, switching from using stamp files to phony targets,
after issue #478 was fixed in Ninja. For additionnal context
see https://gn-review.googlesource.com/c/gn/+/11380.
The newly generated build plans trigger this condition more frequently,
which results in flaky build failures for large GN-based projects such
as Fuchsia.
This patch ensures the depfile's parent directory is always created
before the command is launched to get rid of the issue entirely.
Just check for the ESC character in the `output` string,
if none is present, or if the terminal supports color,
avoid unnecessary busy work (string allocations and
parsing).
Get rid of std::regex and std::vector usage in ElideMiddleInPlace()
function. These are replaced with custom iterator classes that do
not perform any memory allocation at runtime.
Instead the input string will be parsed twice (once to determine the
visible width, and another time to create the elided result string).
With this patch, elide_middle_perftest goes from 87.1ms to 15.0ms
on my machine (x5.8 faster!). This also removes 65 kiB from the
stripped Linux binary for Ninja (!)
Add a fast-path for the case where there is no escape sequence
in the input, that avoids un-necessary string allocations.
Properly handle ANSI color sequences that appear in the elided
part of the input string. They *must* be preserved to ensure
that the right span is printed with the right color.
For example, consider the following input:
|GREEN|aaaaaaaaaaaaa|RED|bbbbbbbbbbb|BLACK|
With different levels of elision:
max_width=0 -> |GREEN|RED|BLACK| instead of ""
max_width=5 -> |GREEN|a..|RED|b|BLACK|
(this was |GREEN|a..b|BLACK| before this fix)
Moreoever, do not call std::regex_iterator::str() to avoid
un-necessary string allocations and concatenations.
With this patch, 'elide_middle_perftest' goes from 159.9ms
to 87.1 ms on my machine (x1.8 faster)
Move the function to its own source file since it is
no longer trivial (and will get optimized in future
patches).
+ Add a new ElideMiddleInPlace() function that doesn't
do anything if no elision is required + use it in
status.cc.
+ Add a new elide_middle_perftest program to benchmark
the performance of the function.
On my machine, the 'avg' result is 159.9ms.
Use C string macros to hold ANSI escape sequences in order
to make the test much easier to understand and follow.
NOTE: This does not change the test in any way!
Add new options to the `inputs` tool in order to change
the format of its output:
- `--no-shell-escape` to avoid shell-escaping the results.
- `--dependency-order` to return results in dependency order,
instead of sorting them alphabetically.
- `--print0` to use \0 as the list separator in the list,
useful to process the target paths with `xargs -0` and
similar tools.
This uses the InputsCollector class introduced in the
previous patch to implement the tool properly. Results
are still shell-escaped and sorted alphabetically.
Fixed#2482
Technically, these are not required since Ninja is built with -std=c++11,
but using these directives is good practice to clarify the code and
avoid simple human errors.
This adds `override` statements, and removes an un-necessary
destructor declaration from status_printer.h in order to remove
annoying compiler warnings when building Ninja tests with CMake
with recent compilers. For example, clang 16.0 complains with:
```
/path/to/ninja/src/status_printer.h:29:16: warning: 'EdgeAddedToPlan' overrides a member function but is not marked 'override' [
-Winconsistent-missing-override]
virtual void EdgeAddedToPlan(const Edge* edge);
^
```
The root issue is that while libninja is compiled explicitly for C++11,
which disable the warning, the tests themselves are not, since they rely
on GTest which now requires C++14, and thus the standard being used is
determined by the compiler's default configuration.
It seems that new versions of CTest now require case-sensitive arguments,
resulting in an "unknown argument" error for "-vv", while documentation
shows the argument for the full verbosity option as "-VV".
In CMakeLists.txt, there is currently only 1 test registered to CMake
with add_test(), and that is to run the ninja_test binary.
Since using CTest is not reducing the number of commands used for testing,
fix the testing invocation by replacing CTest with a direct run of the test.
Signed-off-by: Michael Pratt <mcpratt@pm.me>
Using std::min() and/or std::max() in sources where windows.h has been
include fails with
error C2589: '(': illegal token on right side of '::' [D:\a\ninja\ninja\build\libninja.vcxproj]
error C2062: type 'unknown-type' unexpected [D:\a\ninja\ninja\build\libninja.vcxproj]
error C2059: syntax error: ')' [D:\a\ninja\ninja\build\libninja.vcxproj]
error C2589: '(': illegal token on right side of '::' [D:\a\ninja\ninja\build\libninja.vcxproj]
error C2059: syntax error: ')' [D:\a\ninja\ninja\build\libninja.vcxproj]
Avoid this by defining NOMINMAX for windows builds, which causes the
windows.h header to skip defining those two macros.
Remove the global variable from debug_flags.cc, instead
make each Builder instance provide its own optional
Explanations class instance, if `-d explain` is used.
Adjust all Builder dependencies to use a pointer to
this instance, instead of writing to a global mutable
variable from random places.
- Add `Explanations*` argument do DependencyScanner()
and DyndepLoader() constructor. The value can be null
if nothing needs to be recorded (e.g. in unit tests).
+ Remove record_explanation() and print_explanations(),
and EXPLAIN() macro.
+ Remove the deprecated code path from status_printer.cc.
Allow the StatusPrinter::PrintStatus() method to
use an Explanations instance, instead of calling
the global `print_explanations()` method.
Since the rest of Ninja still uses `record_explanation()`
at the moment, keep the old code path until we can get
rid of it as well.
This class will be used to replace the global
mutable variable |explanations_| currently defined
in debug_flags.cc, and updated from random locations
of the source code through the `record_explanation()`
function.
Allow the Ninja build plan generated by configure.py to
build `ninja_test` by compiling GoogleTest from source if
the path to the library if passed through the new option
`--gtest-source-dir` or the GTEST_SOURCE_DIR environment
variable.
For simplicity, probing for an installed version of the
library, and linking to it, is not supported (use the
CMake build for this).
This also removes the obsolete `--gtest-dir` option.
+ Update README.md
Fixes#2447
Use a topological sort to get a sorted list of edges to perform
the critical path weigth propagation as fast as possible.
The previous version used a data flow algorithm that forced
the values to be recomputed several times for far too many
edges on complex build plans.
For example, for a Fuchsia build plan with 93339 edges,
this reduces the ComputeCriticalPath metric from 1.9s to 80ms!
The unit-tests still pass, and manual testing shows that the
order of commands does not change before and after this change
for the example build plan above.
Before #2067, ninja would have printed this instead for the second
`ninja` invocation:
ninja explain: output .FORCE of phony edge with no inputs doesn't exist
ninja explain: .FORCE is dirty
ninja explain: input is dirty
ninja explain: mid is dirty
[1/3] [ -e input ] || touch input
The explanations are printed just before each command is run. Before, it
would potentially print pages & pages of explanations at the beginning,
and it was hard to tell which explanation caused a particular command to
run.
Edges that are pruned due to "restat" used to print all their
explanations at the beginning, cluttering up the output with
explanations that didn't actually cause any rebuilding. Now, those
explanations aren't printed if those edges aren't run.
