The breakage in rules_haskell is due to c481351 which is already in nixpkgs master. So I now consider that my current PR (update to 0.29.0) is not worse than the current status of bazel in nixpkgs.

I'll try to fix rules_haskell later and see if the fix leads to a change in nixpkgs.

@Profpatsch @kalbasit review please.

@GrahamcOfBorg build bazel bazel.tests

I would like to see a review from @aherrmann. He fixed a Python issue recently (see #66718), which seems to have resurfaced. Also see #66723, which should avoid the problem from coming back, but has yet to be merged.

cc @flokli @Profpatsch

@mboes I don't see the relation between your comment and my PR (and the python issue detailled inside).

I bisected the regression to this commit c481351 which replaced an hardcoded python path (used to launch python executable in py_binary) by the original SearchPath function from bazel, which is supposed to look for python in the PATH.

My understanding is that bazel in nixpkgs is not correctly passing the PATH environment variable to the python script runner. For now I'm fixing it by manually injecting PATH: tweag/rules_haskell@0882c7c

My point of view is that I don't think the current PR have anything to do with the error I'm describing (considering that I can observe the same issue with bazel 0.28.1 from nixpkgs master) and that we can merge this PR independently of #66718 and #66723 and any future work done to work on the python path issue.

Runtime bazel now also depends on the distdir setting which appears
in the global configuration file. This increases the bazel closure
size by 85 MO for stuffs which can normally be downloaded at runtime
by bazel. However, any invocation of buildBazelPackage (such as in
bazel-watcher) may fail in nix sandbox if theses files are not
available at runtime.

Are you sure this isn’t an upstream regression? It looks highly dubious to me.

Runtime bazel now also depends on the distdir setting which appears
in the global configuration file. This increases the bazel closure
size by 85 MO for stuffs which can normally be downloaded at runtime
by bazel. However, any invocation of buildBazelPackage (such as in
bazel-watcher) may fail in nix sandbox if theses files are not
available at runtime.

Are you sure this isn’t an upstream regression? It looks highly dubious to me.

Runtime bazel now also depends on the distdir setting which appears
in the global configuration file. This increases the bazel closure
size by 85 MO for stuffs which can normally be downloaded at runtime
by bazel. However, any invocation of buildBazelPackage (such as in
bazel-watcher) may fail in nix sandbox if theses files are not
available at runtime.

Are you sure this isn’t an upstream regression? It looks highly dubious to me.

Apparently bazel is trying to externalize all the rules. You can see here the reference for, amongst others, rules_cc: https://github.com/bazelbuild/bazel/blob/master/WORKSPACE#L456

Here you can see the discussion for rules_python: bazelbuild/bazel#9006 which will be soon used by default, according to the release note: https://blog.bazel.build/2019/08/27/bazel-0.29.0.html

Added --incompatible_load_python_rules_from_bzl, which will be flipped in a future Bazel version. To migrate, load Python rules from @rules_python.

This seems to be orthogonal to #66723, indeed. I've run the shebang tests from that branch on this PR and they passed, so there are no new unpatched shebangs.

considering that I can observe the same issue with bazel 0.28.1 from nixpkgs master

That's odd, #66718 solved the similar issue for me with bazel 0.28.1. I see how c481351 is related. But, it seems that another change in Bazel 0.29 triggers this issue in combination with c481351.

My understanding is that bazel in nixpkgs is not correctly passing the PATH environment variable to the python script runner. For now I'm fixing it by manually injecting PATH: tweag/rules_haskell@0882c7c

Looking at the //tests/version-macros failure, the Python stub template is instantiated with %python_binary% = "python". Bazel has some logic to calculate that value based on the Bazel configuration. Maybe nixpkgs should adjust the default config to produce a more sensible value for %python_binary% in the context of nixpkgs. If it was an absolute path, then Bazel wouldn't consult PATH.

Alternatively, did you try extending the defaultShellPath in bazel/default.nix to include the Python interpreter? Bazel seems to assume that python is in PATH. So, maybe that's another solution to the problem. However, I'm not sure how that plays with Python 2 vs. 3.

Working around the issue downstream in rules sets by extending PATH does not seem like a good solution. Most rules sets are not written with nixpkgs in mind.

That's odd, #66718 solved the similar issue for me with bazel 0.28.1. I see how c481351 is related. But, it seems that another change in Bazel 0.29 triggers this issue in combination with c481351.

With bazel 0.28.1 from nixpkgs before or after c481351? I initially thought that the issue was in bazel 0.29, but when I updated rules_haskell to use latest nixpkgs (including bazel 0.28.1 and c481351), I had the same issue.

Alternatively, did you try extending the defaultShellPath in bazel/default.nix to include the Python interpreter? Bazel seems to assume that python is in PATH. So, maybe that's another solution to the problem. However, I'm not sure how that plays with Python 2 vs. 3.

Not yet, that's indeed a good idea. However I'm afraid of the semantic. The point of c481351 was exactly to avoid an hardcoded python interpreter and instead rely on PATH detection, which seems to be the bazel semantic.

I'll try to create a simple reproduction code (i.e.: simpler than rules_haskell) and I'll open a nixpkgs issue. In the meantime I've opened tweag/rules_haskell#1067 which tracks this as a rules_haskell issue and get the same details / conclusion as you did right now. I apologize for duplicate work, I should have put a notification here. I however do agree now that the issue is in nixpkgs and not in rules_haskell.

Here you can see the discussion for rules_python: bazelbuild/bazel#9006 which will be soon used by default,

Luckily, it won’t happen for 1.0 yet: bazelbuild/bazel#9006 (comment)

I however do agree now that the issue is in nixpkgs and not in rules_haskell.

Do we merge this PR then, nonetheless?

Do we merge this PR then, nonetheless?

From my point of view there is no argument against merging it.

That's odd, #66718 solved the similar issue for me with bazel 0.28.1. I see how c481351 is related. But, it seems that another change in Bazel 0.29 triggers this issue in combination with c481351.

With bazel 0.28.1 from nixpkgs before or after c481351? I initially thought that the issue was in bazel 0.29, but when I updated rules_haskell to use latest nixpkgs (including bazel 0.28.1 and c481351), I had the same issue.

After c481351, #66718 directly addresses a change from that commit.

Not yet, that's indeed a good idea. However I'm afraid of the semantic. The point of c481351 was exactly to avoid an hardcoded python interpreter and instead rely on PATH detection, which seems to be the bazel semantic.

Yes, that's what I meant by

However, I'm not sure how that plays with Python 2 vs. 3.

Modifying the Bazel default config to provide a better substitution for %python_binary% might be more in line with that.