ninja: pipe ninja output through cat in hooks

[pennae]

Motivation for this change

it would be nice to have progress output from builds using ninja. (ninja progress reports are hidden because ninja never really writes out full lines, it keeps overwriting the same line)

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 22.05 Release Notes (or backporting 21.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[lovesegfault]

Would using -v instead suffice?

[pennae]

-v would work, but it also prints the full command lines (it might make sense to set that when NIX_DEBUG is set though). another workaround we've seen was setting TERM=dumb, but adding to the process environment has more potential for side-effects than going through cat.

[dasJ]

Code looks good, I didn't test this but I don't think this should be able to break anything ;)

[pennae]

systemd and glib built fine with it, after those we stopped checking which derivations in the test build used ninja :D

[milahu]

alternative solution:

export TERM=dumb
ninja

[nagy]

I think I have found a problem with this. The piping into cat ignores the exitcode of ninja. So I built this: nagy@ff0db14

The PoC is in the commit message. I have not found a package where this is a problem, but if we ever rely on the ninja*Phasees alone to build a package, then errors may be ignored.

Does this need a pull request?

[pennae]

yes please. ignoring exit codes in a builder should never happen 😳</del?

edit: after experimenting locally, probably not. stdenv does set -o pipefail so this won't affect builds within nix itself, only in nix-shell. and that's not even limited to ninja.

[nagy]

stdenv does set -o pipefail so this won't affect builds within nix itself

That would explain why I did not see this failing in normal package builds, thanks.

only in nix-shell. and that's not even limited to ninja.

It is a bit unfortunate, but understandable that pipefail is not set in nix-shell because this means that it can sometimes be surprising if you rely on the exitcode of any phase.

But ok, then this does not effect this code after all. Thank you for the investigation.