Status changes to support frontends #1899
Conversation
colincross
force-pushed
the
status_for_serialize
branch
from
e44be94 to
ba34348
Compare
The times that end up in the build log currently originate in the status printer, and are propagated back out to the Builder. Move the edge times into the Builder instead, and move the overall start time into NinjaMain so it doesn't get reset during manifest rebuilds.
Store the number of running edges instead of trying to compute it from the started and finshed edge counts, which may be different for starting and ending status messages. Allows removing the status parameter to PrintStatus and the EdgeStatus enum.
Make BuildStatus an abstract interface, and move the current implementation to StatusPrinter, to make way for a serialized Status output.
colincross
force-pushed
the
status_for_serialize
branch
from
ba34348 to
d57f520
Compare
Send all output after manifest parsing is finished to the Status interface, so that when status frontends are added they can handle build messages.
colincross
force-pushed
the
status_for_serialize
branch
from
d57f520 to
ad3d29f
Compare
|
FYI, this does break pipelines that previously relied on all output being printed on stdout. This changes error messages when a subcommand fails to print to stderr now, which could cause issues with processes that read both stdout and stderr and previously relied on errors being serialized with normal output. Actual subcommands' stderrs are still printed to stdout. It's only ninja's own error messages that are now stderr. Was this intentional? @colincross EDIT: Specifically, we use ninja in our Jenkins pipeline, and use |
This pull request changed moved the calls for "ninja: no work to do", "ninja: build stopped: subcommand failed" and "ninja: Entering directory" from printf to the Status interface, which calls the pre-existing Info method. The Info method has always used stderr, but there were no users of the Info method before my change, so it could be switched to stdout to restore the previous behavior. |
See comment in #1899. Also adds two tests to output_test.py which check this behaviour by relying on Python's suprocess.check_output not piping stderr.
|
Thanks for bringing this up! I restored the old behavior and added a test so that we don't regress. |
This pull request takes the less controversial status refactoring changes from #1210 as an incremental step towards supporting frontends. It moves the status printing code into a separate file and calls it through an interface to allow multiple implementations.