-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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: 2.0.0 -> 2.1.0 #80739
bazel: 2.0.0 -> 2.1.0 #80739
Conversation
27982b7
to
e091923
Compare
Looking at the current implementation of the script on master, it doesn't enforce that naming convention for the binary, it still supports calling it In that case the |
That looks like what we should be doing. |
Please don't just update this version. Does 2.0 remain compatible with 2.1 in all cases? If not we should consider having another attribute within nixpkgs. The update to Bazel 2.0 already caused some breakage that we didn't get rid of yet. See the relevant conversations in #76851 and my WIP PR #80953. |
bazel promises no backwards-compatible changes for three months at a time If they release a breaking change, they bump the major version. |
Let’s put this in another commit. |
Will do! Apologies for the messy intermixing of changes. Should I close this PR and resubmit separate ones, or is it sufficient to create a new one with the version change? |
It’s fine if you pull it out in a separate commit, they can both be in this PR. |
related comment in the original issue: |
So I guess as long as we have a normal |
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.
Once the two commits are split, this is good to go. 👍
d7166ba
to
124196f
Compare
@GrahamcOfBorg build bazel.tests |
Regarding the failed tests on darwin, I do not have a machine where I can work to fix the issue, so I'd appreciate any assistance in figuring it out, if it's a blocker (which it sounds like it is). :) |
@kalbasit you seem to have fixed the protobuf test last time, could you take a look? |
The failure I'm getting on darwin is the one that was fixed in #80635. |
Rebased on current master (b1ec189) it works. |
From Bazel 2.0.0 onwards, Bazel looks for a binary named `bazel-${version}-${os_arch}` if the project root contains a `.bazelversion` file or the USE_BAZEL_VERSION environment variable is set. This change ensures we output a binary with the correct name for the current version and OS/arch combination.
124196f
to
3ef9a18
Compare
Rebased it on current master right now. |
cool |
Motivation for this change
The official Bazel wrapper script included from version 2.0.0 onwards enforces the actual
bazel
binary (previously namedbazel-real
in this package) has the following naming convention, if the root of your Bazel project contains a.bazelversion
file:bazel-${version}-${os}-${arch}
.This modifies output binary name and, while we're at it, upgrades Bazel to 2.1.0.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)