-
Notifications
You must be signed in to change notification settings - Fork 4.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 installer stopped working after d3f8efc, if project has .bazelversion file #10356
Comments
This is caused by d3f8efc @philwo |
Maybe we can package the version.txt file into the debian package and installer, then make the shell script check the Bazel version here? |
Interestingly, bazel project itself is not affected, and:
just works. However, all projects that have So the workaround for now is to remove Also, I would have expected, that higher version number semantic is still preserved. If |
Is this a release blocker for 2.0? |
@dslomov Yes. I'm looking into this now. |
So, here are some thoughts on this: If a project has a .bazelversion file, it means it "should" be built with that exact Bazel version. Because the .bazelversion file is used to basically "version your build tool with your source code", I do not want to implement fuzzy matching like "as long as it's the same major version, it's fine". It will use and require the exact version, so this is working as intended. Let's assume you have a project with a .bazelversion file that specifies "1.1.0" as the required Bazel version, then here's what happens:
I think this behavior makes sense and is pretty user friendly? However, I agree that there's one optimization we should add: If the .bazelversion asks for Bazel X.Y and the Bazel version that was installed via I'll think of some logic for the wrapper script that supports this, then we can do a cherry-pick and it should be fine. |
Also, I don't understand this reasoning:
No, of course not - the workaround is not to remove the .bazelversion file, the "workaround" is to simply do what the (IMHO pretty helpful) error message suggests - install the required Bazel version :) I don't see how this is "broken" or how "Bazel stopped working". Edit: @davido Sorry, to be clear, I totally understand why having to download the same Bazel version twice (once as the "latest" Bazel and once as the "specifically versioned package" Bazel) is weird and not a good UX. I'll fix this before we release Bazel 2.0. The general behavior should be fine and an improvement, though, or what do you think? |
+1. Totally agreed. Thank you for the clarification! Sorry for not very clear bug report, but I was totally confused by the Bazel installer call to download the same Bazel version second time, so confused, that I have not even tried to do, what the error message told, but went ahead and created the error report. |
This means that if you "apt-get install bazel" and that happens to be version "2.0.0" and in your `.bazelversion` file you have "2.0.0", then the new Bazel wrapper will just use the Bazel binary installed as the "latest" Bazel by "apt-get install bazel" and not ask the user to install the same version of Bazel as a side-by-side binary using "apt-get install bazel-2.0.0". The same applies when using the Bazel installer or any other setup that uses the wrapper script. Closes #10356. RELNOTES: None. PiperOrigin-RevId: 285941301
We would like to kindly request this issue to be reopened as it does appears to have broken both Bazel and The requirements of CI and an organisation of 100+ engineers mean that the way in which we manage versions for
To achieve this:
While attempting the move to The repo under tests currently relies on
However, instead, we found that we never even get into
Although the suggested solution is
this manual actions is clearly not an option given the requirement of automating the entire process, which is the very goal of a tool like From reading the code in the wrapper script we have also found that there is no way of deactivating this new logic. As a result we are now faced with the fact that we cannot at any point in the foreseeable future migrate to The questions that we have are:
To be clear: if we had already been using |
Hi @Helcaraxan and others, sorry for the trouble you and others are having with the new feature. I'll respond tomorrow to all of your questions. I designed the new feature to not break any existing users, but obviously this didn't work out. I reopened this issue - let's see that we can fix it together to also work for you and others who check in Bazelisk as |
Hello @philwo. Thank you very much for the quick reply. It is much appreciated. 👌 We'll be waiting for the answers and are happy to collaborate in finding the best way forward. We were genuinely surprised when we realised the conundrum we had just stumbled into with this change in the behaviour of |
cc @laurentlb for the 2.1.0 release. |
Thanks everyone for commenting and/or upvoting @Helcaraxan's post so that I can get a good feeling for the impact and urgency here and communicate this to my colleagues. This really helps prioritize this, so that we can hopefully find a good solution for all users quickly. :) Before Bazel 2.0, users could install Bazel using a variety of ways:
When the wrapper or Bazelisk run All users from above list except the Bazelisk users would now regularly run into this problem: They somehow end up with a version of Bazel that doesn't match the project that they want to build (e.g. downloaded the wrong version or their package manager automatically updated Bazel over night and whamm the entire team's build is broken). How do we fix that? Two ideas:
Although I would have loved option 1, it didn't sound too convincing in practice:
So I went for option 2. The users who cannot use Bazelisk are happy that their Bazel versioning problem is now finally solved, yay. 🎉 The users who actually were using option 1 are now broken. Not-so-yay. 😞 Let's figure out how to support this without breaking anyone. Here's all combinations of users and repository setups. In the third column "Wrapper" refers to a script that simply changes the environment a bit and then delegates to
It looks like we have to fix the case where:
Do you have suggestions how to fix this? Some ideas:
The reason was that users who could not use Bazelisk (or download binaries from the internet) still wanted a way to version Bazel with their source code (= the
I don't see this quite as dark - if people installed Bazel 2.0.0 in a way that gave them the shell-script wrapper that you relied on, then it usually means that they used a package manager, which will automatically update their Bazel to later versions. Also, any Bazel version could potentially be so broken that it doesn't work with your project - it's not great, but we have to fix it and then go on.
See above - I would love if everyone could just Bazelisk. Maybe I should have pushed more for this. If you and others think that adding offline support etc. to Bazelisk is the right direction, we can try to do that instead.
I don't think this is the case - all platforms that ship the shell-script wrapper (which are all platforms for which a If the question is about "why didn't you just add this directly to Bazel" - because I didn't want to add something that "phones home" to the internet to Bazel. User trust is super important for me and others on the Bazel team and we think your build tool shouldn't rely on the internet in order to work. Hope this already answers some questions - I'll follow-up tomorrow in case I have any other ideas / thoughts about this and of course for any comments and questions that come up :) |
What do you think we just do a Update: Just realized it won't work if |
This broke one specific use case: too new Bazel version used, as documented in However, the projects may already implement bazel vesion check in Anyway, one way to solve the conflict for Bazel shell wrapper between:
is to make the check in shell wrapper conditional. It could be |
Thank you for the detailed walkthrough @philwo and your thoughts on it. This is very useful to understand the context in which the various implementation choices were made. ❤️ Based on the information that you've shared we definitely understand the use-case that drove the feature forward, as well as the pain-point that it is trying to address. Some general thoughts from our side:
This is exactly (one of) the problem(s) that
That is indeed true. However we're circumventing that issue on Windows specifically by ensuring that the local
Yes, from our perspective that would definitely be the right direction. It would match Bazelisk's aspirations very well as a tool for managing Bazel versions. The crux of the issue lies in the fact that the error that is being raised is a hard one, instead of simply an explicit warning. If it had been a warning, or if the error could be deactivated as @davido suggests, then there would have been a straightforward fix for the broken users. The preferred way forward for us would be a mix between suggestions 2 & 3:
|
Thanks for the input! I'll go with @Helcaraxan's plan and will make sure that the fixed wrapper (skip version check if |
?Bazel binary, if tools/bazel exists. Instead, we should do our best to populate the $BAZEL_REAL env var with some meaningful path, but then delegate to tools/bazel without aborting or printing anything, in case we couldn't find a requested binary or make sense of the contents of .bazelversion. This is in response to #10356 (comment) and hopefully fixes the use case reported by the affected users. @laurentlb This is the cherry-pick I mentioned. I will import it, if you LGTM this PR. @Helcaraxan @davido FYI. Closes #10664. PiperOrigin-RevId: 291904662
?Bazel binary, if tools/bazel exists. Instead, we should do our best to populate the $BAZEL_REAL env var with some meaningful path, but then delegate to tools/bazel without aborting or printing anything, in case we couldn't find a requested binary or make sense of the contents of .bazelversion. This is in response to #10356 (comment) and hopefully fixes the use case reported by the affected users. @laurentlb This is the cherry-pick I mentioned. I will import it, if you LGTM this PR. @Helcaraxan @davido FYI. Closes #10664. PiperOrigin-RevId: 291904662
?Bazel binary, if tools/bazel exists. Instead, we should do our best to populate the $BAZEL_REAL env var with some meaningful path, but then delegate to tools/bazel without aborting or printing anything, in case we couldn't find a requested binary or make sense of the contents of .bazelversion. This is in response to #10356 (comment) and hopefully fixes the use case reported by the affected users. @laurentlb This is the cherry-pick I mentioned. I will import it, if you LGTM this PR. @Helcaraxan @davido FYI. Closes #10664. PiperOrigin-RevId: 291904662
?Bazel binary, if tools/bazel exists. Instead, we should do our best to populate the $BAZEL_REAL env var with some meaningful path, but then delegate to tools/bazel without aborting or printing anything, in case we couldn't find a requested binary or make sense of the contents of .bazelversion. This is in response to #10356 (comment) and hopefully fixes the use case reported by the affected users. @laurentlb This is the cherry-pick I mentioned. I will import it, if you LGTM this PR. @Helcaraxan @davido FYI. Closes #10664. PiperOrigin-RevId: 291904662
@philwo Thanks for adding the option to skip the check in the script wrapper. Now, to test a minor Bazel upgrade, say 2.1.0rc4 from [1] in Gerrit workspace, where
Now, after installing
Of course, I could temporary update the |
Thanks for testing, @davido! :) |
Just wanted to follow up @philwo about the resulting fix that now is making its way into 2.1.0. The reactiveness and communication has been greatly appreciated. A great thanks on behalf of the entire team. ❤️ |
@Helcaraxan What do you think - Bazel 2.1.0 should be released tomorrow unless some unexpected blocker shows up today. Do we still need a patch release in that case? I would say no, because the patch release would also have to bake for a day or so, so it wouldn't be out any quicker. 🤔 |
@philwo I agree that it wouldn't be required for the release speed of the fix. However the argument can be made that as it stands 2.0.0 is broken to some degree and thus should be patched. That's a much more subtle debate and I trust the Bazel team to make the call on that. A patch would always be appreciated but it'd be understandable if you don't want to bear with the extra work. Sorry. Am aware I am not necessarily making things easier but that's my honest perspective. |
Your input and perspective is always appreciated. :) @dslomov I'll see how many cherry-picks we would need for this and will comment on the release bug of Bazel 2.0! |
?Bazel binary, if tools/bazel exists. Instead, we should do our best to populate the $BAZEL_REAL env var with some meaningful path, but then delegate to tools/bazel without aborting or printing anything, in case we couldn't find a requested binary or make sense of the contents of .bazelversion. This is in response to #10356 (comment) and hopefully fixes the use case reported by the affected users. @laurentlb This is the cherry-pick I mentioned. I will import it, if you LGTM this PR. @Helcaraxan @davido FYI. Closes #10664. PiperOrigin-RevId: 291904662
Bazel 2.0.1, 2.1.1 and 2.2.0 have been released with the fix, so I'm closing this issue. |
After installing of bazel 2.0.0rc3 I am seeing this:
As expected, bazel was downloaded by the bazelisk:
Now, if I try to build with bazel directly, bypassing bazelisk, it is failing:
Now, I have to fetch bazel twice? It also doesn't change if I install Bazel locally, I still see the same breakage, even though bazel-2.0.0rc3 was installed:
And bazel still doesn't work:
The text was updated successfully, but these errors were encountered: