-
-
Notifications
You must be signed in to change notification settings - Fork 15.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bazel test shebangs #66723
Bazel test shebangs #66723
Conversation
There was a problem hiding this 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.
@GrahamcOfBorg build bazel.tests |
There was a problem hiding this 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.
8f40298
to
f99d585
Compare
@GrahamcOfBorg build bazel bazel.tests If test pass, I will merge so we can assert #67835 before merging. |
@GrahamcOfBorg build bazel.tests |
The Darwin build has lots of instances of this error:
|
@Profpatsch also
That's strange, this PR doesn't touch |
Good question, here’s a dummy PR to test: #67999 |
Update: The tests pass on current master. |
eeabdb5
to
7cf7307
Compare
That's grown to a pretty big diff by now. There's a merge conflict with master now anyway. I've rebased. |
7cf7307
to
62e843d
Compare
@GrahamcOfBorg build bazel.tests |
I think ofBorg isn’t listening to Members yet; |
Hm, now there’s still one test failing on Darwin:
|
Ping. So close :) |
To point to the custom bash instead of `/nix/store.../bin/env bash`.
Fail on any form of `bin/env ...` shebang.
62e843d
to
1f3187c
Compare
rebased on latest master. |
@GrahamcOfBorg build bazel.tests |
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
sandbox
innix.conf
on non-NixOS)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @Profpatsch