Skip to content

Bazel test shebangs#66723

Merged
flokli merged 5 commits intoNixOS:masterfrom
aherrmann:bazel-test-shebangs
Oct 11, 2019
Merged

Bazel test shebangs#66723
flokli merged 5 commits intoNixOS:masterfrom
aherrmann:bazel-test-shebangs

Conversation

@aherrmann
Copy link
Member

As suggested by @Profpatsch in #66718 (comment) this PR adds a test to Bazel checking that shebangs have been patched appropriately.

This test flagged a few more instances of invalid python shebangs which are also fixed by this PR.

Motivation for this change

See #66718 (comment)

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 nix-review --run "nix-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @Profpatsch

@aherrmann aherrmann requested a review from Profpatsch as a code owner August 16, 2019 15:02
Copy link
Member

@Profpatsch Profpatsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, this removes another source of easy-to-make and hard-to-debug mistakes.

@Profpatsch
Copy link
Member

@GrahamcOfBorg build bazel.tests

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Aug 16, 2019
Copy link
Member Author

@aherrmann aherrmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Profpatsch Thanks for the review, I've addressed your comments, PTAL.

@aherrmann aherrmann force-pushed the bazel-test-shebangs branch from 8f40298 to f99d585 Compare August 19, 2019 12:34
@mboes mboes mentioned this pull request Sep 1, 2019
10 tasks
@kalbasit
Copy link
Member

kalbasit commented Sep 1, 2019

@GrahamcOfBorg build bazel bazel.tests

If test pass, I will merge so we can assert #67835 before merging.

@Profpatsch
Copy link
Member

@GrahamcOfBorg build bazel.tests

@Profpatsch
Copy link
Member

The Darwin build has lots of instances of this error:

ERROR: /private/tmp/nix-build-bazel-test-cpp.drv-1/wd/BUILD:1:1: Target '//:ProjectRunner' depends on toolchain '@local_config_cc//:cc-compiler-darwin', which cannot be found: error loading package '@local_config_cc//': Unable to find package for @rules_cc//cc:defs.bzl: The repository '@rules_cc' could not be resolved.'

@aherrmann
Copy link
Member Author

@Profpatsch also

ERROR: /private/tmp/nix-build-bazel-test-builtin-rules.drv-0/wd/python/BUILD.bazel:6:1: Target '//python:bin' depends on toolchain '@local_config_cc//:cc-compiler-darwin', which cannot be found: error loading package '@local_config_cc//': Unable to find package for @rules_cc//cc:defs.bzl: The repository '@rules_cc' could not be resolved.'
ERROR: Analysis of target '//python:bin' failed; build aborted: error loading package '@local_config_cc//': Unable to find package for @rules_cc//cc:defs.bzl: The repository '@rules_cc' could not be resolved.
INFO: Elapsed time: 9.871s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (12 packages loaded, 27 targets configured)
ERROR: Build failed. Not running target

That's strange, this PR doesn't touch @local_config_cc or @rules_cc. Do we know that this did not already happen before this PR?

@Profpatsch
Copy link
Member

Do we know that this did not already happen before this PR?

Good question, here’s a dummy PR to test: #67999

@Profpatsch
Copy link
Member

Update: The tests pass on current master.

@aherrmann
Copy link
Member Author

Update: The tests pass on current master.

That's grown to a pretty big diff by now. There's a merge conflict with master now anyway. I've rebased.

@ofborg ofborg bot requested a review from mboes September 3, 2019 12:19
@aherrmann
Copy link
Member Author

@GrahamcOfBorg build bazel.tests

@Profpatsch
Copy link
Member

Profpatsch commented Sep 3, 2019

I think ofBorg isn’t listening to Members yet;
@GrahamcOfBorg build bazel.tests
Edit: one has to be a trusted user for ofborg to react: NixOS/ofborg@0377fd8

@Profpatsch
Copy link
Member

Hm, now there’s still one test failing on Darwin:

src/main/tools/process-wrapper-legacy.cc:58: "execvp(external/local_config_cc/cc_wrapper.sh, ...)": No such file or directory
INFO: Elapsed time: 3,476s, Critical Path: 0,06s
INFO: 0 processes.
FAILED: Build did NOT complete successfully
FAILED: Build did NOT complete successfully
builder for '/nix/store/lzii536cvmaix1bnavmwwn5liha3vjl8-bazel-test-cpp.drv' failed with exit code 1

@lheckemann lheckemann added this to the 20.03 milestone Sep 10, 2019
@Profpatsch
Copy link
Member

Ping. So close :)

@flokli flokli mentioned this pull request Oct 11, 2019
10 tasks
@flokli flokli force-pushed the bazel-test-shebangs branch from 62e843d to 1f3187c Compare October 11, 2019 19:44
@flokli
Copy link
Member

flokli commented Oct 11, 2019

rebased on latest master.

@flokli
Copy link
Member

flokli commented Oct 11, 2019

@GrahamcOfBorg build bazel.tests

@flokli flokli merged commit 74008e2 into NixOS:master Oct 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants