Add awk as a default tool for Bazel shell commands.

[robinp]

Apparently
https://github.com/gflags/gflags/blob/e292e0452fcfd5a8ae055b59052fc041cbab4abf/bazel/gflags.bzl#L8
assumes it should be accessible. Normally we could ask them to fix, but
I would expect awk to be a commonly assumed.

The rough search
https://github.com/search?q=filename%3ABUILD+genrule+awk&type=Code
brings ~1K hits.

Motivation for this change

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Fits CONTRIBUTING.md.

---

[robinp]

• @mboes

[Mic92]

It is part of our stdenv so that should be fine.

[mboes]

LGTM.

[matthewbauer]

IMO we should patch Bazel to respect PATH properly. But I suppose this is okay as a fix for now.

[mboes]

@matthewbauer I don't think so. The rule actions are not meant to depend on just anything that happens to be found on the PATH. That would make the build less hermetic and hurt reproducibility. Upstream Bazel has a hardcoded PATH=/bin:/usr/bin for its "strict" action environment. That's pretty much the empty environment on NixOS, so in Nixpkgs we also hardcode a PATH, just a different one. Unlike in upstream Bazel, we can enforce that our special chosen hardcoded value only exposes a core set of utilities assumed to be universally available.

[matthewbauer]

@matthewbauer I don't think so. The rule actions are not meant to depend on just anything that happens to be found on the PATH. That would make the build less hermetic and hurt reproducibility. Upstream Bazel has a hardcoded PATH=/bin:/usr/bin for its "strict" action environment. That's pretty much the empty environment on NixOS, so in Nixpkgs we also hardcode a PATH, just a different one. Unlike in upstream Bazel, we can enforce that our special chosen hardcoded value only exposes a core set of utilities assumed to be universally available.

But, there's no way to specify what software you're using. Nix can handle this for you, but only if you don't clear the PATH variable. I think the idea that clearing PATH somehow makes things more reproducible is kind of silly. You could have /usr/bin filled with nonstandard binaries in the same way you could fill PATH with nonstandard directories. Assuming all of these commands exist in /usr/bin is breaking portability to not only NixOS but also BSDs where GNU tools are conventionally put in /usr/local/bin. Nix* ecosystem has been working for a long time to prevent tools from hardcoding /usr/bin/ paths. It's really annoying that Bazel seems to go out of its way to hardcode these paths. At the least, we need some way to override this annoying behavior for non-FHS systems.

[mboes]

I think you're misunderstanding how Bazel uses PATH. The BSD's do not put standard tools like awk, grep and find in /usr/local/bin (these don't need to be GNU). They go in /bin or /usr/bin. Bazel does not want to pick out just about anything from /usr/bin. Just some core tools (i.e. coreutils + a few other small things). Everything else, it is assumed, will be part of the build, one way or another (therefore not to be found in /usr/bin). You can ask Nix to handle extra external dependencies not part of the build, via https://github.com/tweag/rules_nixpkgs. This is a better solution than putting everything in a nix-shell, and running Bazel from inside the nix-shell who will grab what it wants from the PATH, because Bazel with rules_nixpkgs Bazel can interleave downloading and building, and also downloading only exactly what is necessary for building the particular target that was given to Bazel.

For those few tools that Bazel wants to find in /usr/bin, it's easy enough to patch that path to point to where these specific tools are in Nixpkgs. If I understand you correctly, you're saying that actions should always use whatever was PATH at the point where the bazel command is called. I don't understand what would be gained though. And it's a lot harder to do.

[robinp]

A question since this is my first nixpkgs PR. Am I expected to tick off all checkboxes?

[robinp]

After running into git in some repo, I asked https://stackoverflow.com/questions/53428532/bazel-list-of-tools-in-scope-for-targets-running-shell-commands.

[7c6f434c]

Note that these are not hard requirements but merely serve as information for reviewers.
(comment in the template markup)

This is indeed the intended policy.

[robinp]

@mboes:

For those few tools that Bazel wants to find in /usr/bin, it's easy enough to patch that path to point to where these specific tools are in Nixpkgs.

By patching you mean replacing foo invocation in the BUILD file with an actual nix-env path for foo , or properly sourcing the tool using rules_nixpkgs and then patching to use that as a dep?

[mboes]

@robinp I mean neither. Bazel rule sets assume that a few small utilities will be available at known locations somewhere on the host (not in the source repo). Those small utilities can indeed be found on a NixOS system - just at different known locations. See the derivation for the Bazel package. It simply replaces /bin/:/usr/bin in one specific place with the equivalent on NixOS, restricted to just those few small utilities Bazel rules assume are not part of the source repo (e.g. grep, sed, mkdir etc).

[robinp]

On above SO question I got pointed to bazelbuild/bazel#5265, which is now temporary stalled, but acknowledges that the set of "small utilities" is not well defined.

We can keep adding implicit tools used in Bazel rules to the Nix derivation, but I suspect there needs to be a cutoff. Without doing some hard-core grepping on big public Bazel repos, I don't know for example if git is a reasonable addition. (In the mean time I ran into bison and flex as well, just to illustrate).

Or, a different point of view could be, if a tool is likely to be present in a baseline Nix install (if there's such), it's ok to be included in the derivation.

[mboes]

I don't think it's reasonable to include Git, nor Bison and flex. Where did you encounter those?

[robinp]

Found in Kythe repo. Git is called in check.sh from https://github.com/kythe/kythe/blob/77e1360beca7903bc97728097d2c5e1b82204092/tools/modules/BUILD#L27. Bison&flex are used in https://github.com/kythe/kythe/blob/e410fa74554d7d024d0080ec7748d6bf85428512/tools/build_rules/lexyacc.bzl, for example.

I agree git would be excessive to add. Maybe it's easiest to roll a customized derivation for Bazel for such one-offs. But if Kythe were to be built with Nix (well, I don't really want, just in case), I assume we would need nix-side patches to properly source these tools.

Maybe there could be a NIX_BASH_PATH env var that lets dynamic setting of these tools. It wouldn't go against hermetic builds (at least when combined with Bazel), as Bazel by default doesn't pass any env to the commands. You have to specify --action_env=NIX_BASH_PATH (and --test_env as well) flag to build which makes the usage kind of explicit.

[Mic92]

@Profpatsch anything further to do in this PR?

[robinp]

@Mic92 I don't think there's anything apart from maybe checking more tickboxes at the top. But these seem quite mysterious to me (first time nixpkgs PR), the only one I tried

NIX_PATH=nixpkgs=/path/to/nixpkgs nix-shell -p nox --run "nox-review wip" tells

No uncommit changes. Did you mean to use the "--against" option?

Given the simplicity of the change maybe it is not needed to tick those, but up to you to say.

[Mic92]

@robinbb try nix-shell -p nix-review --command "nix-review pr 50765" in the nixpkgs repository. It is faster and better maintained.

[Mic92]

Result of nix-review pr 50765

2 package failed to build:

• bazel-deps
• bazel-watcher

1 package were build:

• bazel

However the error shown seems to be not related to this change.

[Profpatsch]

Thanks for the reminder, forgot about this.