-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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_7: init at 7.0.0 #262152
bazel_7: init at 7.0.0 #262152
Conversation
This is now ready for review. Any insight welcome. I did test it with
And while bazel_6 fails to build on darwin, I consider this pretty unrelated, as it fails to find codesign, so probably a sandbox / patching issue. Tests do pass on linux for all bazel versions, and on darwin for bazel_7. |
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.
Awesome work! I'm afraid the nix functions are a bit beyond my skills, I can understand them and I believe they're correct but I don't know if there's ways to make them better / more idiomatic. Hopefully someone else can take a look as well, but either way I don't think there's any blockers there.
pkgs/development/tools/build-managers/bazel/bazel_7/bazel-repository-cache.nix
Outdated
Show resolved
Hide resolved
pkgs/development/tools/build-managers/bazel/bazel_7/bazel-repository-cache.nix
Outdated
Show resolved
Hide resolved
pkgs/development/tools/build-managers/bazel/bazel_7/default.nix
Outdated
Show resolved
Hide resolved
pkgs/development/tools/build-managers/bazel/bazel_7/default.nix
Outdated
Show resolved
Hide resolved
pkgs/development/tools/build-managers/bazel/bazel_7/default.nix
Outdated
Show resolved
Hide resolved
pkgs/development/tools/build-managers/bazel/bazel_7/default.nix
Outdated
Show resolved
Hide resolved
pkgs/development/tools/build-managers/bazel/bazel_7/default.nix
Outdated
Show resolved
Hide resolved
|
I know I am long overdue here, but there are still some pending issues, and we are very close to bazel 7.0.0 official release. I will try to release that directly. |
If merged with latest master it starts to fail on darwin like so
|
Thanks for the report, I will rebase and try to fix. Any idea what the fix could be though ? I have access to a Darwin builder, but not much knowledge.
Le 9 décembre 2023 12:25:29 GMT+01:00, Dmitry Ivankov ***@***.***> a écrit :
…If merged with latest master it starts to fail on darwin like so
`x86_64-darwin`:
```
...
ld: warning: dylib (/nix/store/x4s0wi6d2z38wa0jb4yf769sl1c0c50z-libcxx-16.0.6/lib/libc++.dylib) was built for newer macOS version (10.12) than being linked (10.11)
Undefined symbols for architecture x86_64:
"vtable for std::out_of_range", referenced from:
std::out_of_range::out_of_range[abi:v160006](char const*) in libijar_cc_binary_main.lo(classfile.o)
NOTE: a missing vtable usually means the first non-inline virtual member function has no definition.
"___cxa_end_catch", referenced from:
std::__1::basic_stringbuf<char, std::__1::char_traits<char>, std::__1::allocator<char>>::overflow(int) in libijar_cc_binary_main.lo(classfile.o)
std::__1::basic_ostream<char, std::__1::char_traits<char>>& std::__1::__put_character_sequence[abi:v160006]<char, std::__1::char_traits<char>>(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, char const*, unsigned long) in libijar_cc_binary_main.lo(classfile.o)
...
```
`aarch64-darwin`:
```
...
Undefined symbols for architecture arm64:
"vtable for std::out_of_range", referenced from:
std::out_of_range::out_of_range[abi:v160006](char const*) in libijar_cc_binary_main.lo(classfile.o)
NOTE: a missing vtable usually means the first non-inline virtual member function has no definition.
"___cxa_end_catch", referenced from:
std::__1::basic_stringbuf<char, std::__1::char_traits<char>, std::__1::allocator<char>>::overflow(int) in libijar_cc_binary_main.lo(classfile.o)
std::__1::basic_ostream<char, std::__1::char_traits<char>>& std::__1::__put_character_sequence[abi:v160006]<char, std::__1::char_traits<char>>(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, char const*, unsigned long) in libijar_cc_binary_main.lo(classfile.o)
"typeinfo for std::bad_array_new_length", referenced from:
std::__throw_bad_array_new_length[abi:v160006]() in libijar_cc_binary_main.lo(classfile.o)
...
```
--
Reply to this email directly or view it on GitHub:
#262152 (comment)
You are receiving this because you were assigned.
Message ID: ***@***.***>
|
7.0.0 was released: https://github.com/bazelbuild/bazel/releases |
Looks like 7.x darwin build breaks after #263535 |
It looks like there's some instability around libcxx and apple sdk version that is going to be addressed in master (after 23.11 release, so ~now). But not exactly sure what goes wrong here |
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.
Would you mind testing it for remote execution.
This backport looks good to me and appears to work fine. Thank you!
@layus Hello, when I tried to use I don't know how to fix this in a better way or if it should be done. But I figured it should be reported. |
@name-snrl I have fixed that use case, because I needed the files it bundled. I cannot however support all repository_rule attributes, so the script should be made more robust to failure. I have improved that a bit. |
Okay, so now all tests pass 🎉 As long as ofborg is happy, I am going to merge (or let anyone else merge). |
Awesome! Looks good! |
Successfully created backport PR for |
Is there any reason why remote_java_tools_linux not patcheelf'ed?
|
It is not because patching Bazel to apply patchelf on downloaded tarballs is an heavy change, and probably unexpected by users.
There is however a fix for that issue which relies on using a local Java toolchain and recompiling ijar from sources. The core parameters are in the hardcoded bazelrc file near the top of the nix file.
Ensure you have JAVA_HOME set, and use toolchain resolution debug flag to see why the non_prebuilt toolchain is not used
Le 3 janvier 2024 03:54:07 GMT+01:00, timothy ***@***.***> a écrit :
…Is there any reason why remote_java_tools_linux not patcheelf'ed?
```
Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
src/main/tools/linux-sandbox-pid1.cc:530: "execvp(external/remote_java_tools_linux/java_tools/ijar/ijar, 0x1a6db30)": No such file or directory
```
--
Reply to this email directly or view it on GitHub:
#262152 (comment)
You are receiving this because you were mentioned.
Message ID: ***@***.***>
|
Is bazel7 in nixpkgs missing the action_env patch that exists in bazel6? I seem to be hitting the case that was solved in bazel 6 by #146389 after upgrading to bazel7, where |
@Strum355 I hit a few cases last night during my upgrade to Bazel 7 where things in |
I had the same problem with my deployment. And I just re-added the patch (locally) for now. What bugs me is that the environment ends up empty, but I see no path yet in the code where it can happen. Re-adding the patch is the quickest fix. My best guess is that the patch made us use run() in an incorrect way, assuming that coreutils is present when it is not by default. Bazel add PATH to the action digest, so for tools that need no path, it's better to have none. The patch fooled us into thinking that run() can be used that way. Not too sure yet, I need to investigate using a vanilla bazel. Oh, and not gonna work on that in the next few days sadly, but probably end of next week. |
Correction on this, it appears that what I thought was a case of this patch causing the passing through action_env wasnt actually the case, but it was masking the fact that action_env wasnt being passed through by hard-coding just enough to make some rules that depend on e.g. coreutils run successfully. A lot of convolution around action_env and where/what passes through $PATH, but the patch is just "good enough" for me to keep working. Ideally everything would pass down action_env, as it would mean I could avoid rebuilding bazel by having to to set a different |
With Another rabbit hole with this is it's possible to undo nixos glibc&bazel patches, tweak bazel build once, and then run builds in specially-crafted buildFHSEnv. So rules that expect to work with empty env/path will fallback to fhs through glibc (hence need to unpatch it, nixos disables fhs fallback) and find whatever is put into fhs (and needs to be in fhs as this is where glibc would look on empty env). Also for dynamic linking it might be needed to also undo ld.so.cache fallback patch and put libs into cache in fhs env. To make it bazel cache friendly probably still makes sense to add |
bazel (and its community of rules) is a bit of a mess when it comes to passing around env values, including $PATH, resulting in some actions not receiving the $PATH value set via `--action_path=PATH=...` that we rely on. Bazel 6 in nixpkgs had [the following patch](https://sourcegraph.com/github.com/NixOS/nixpkgs/-/blob/pkgs/development/tools/build-managers/bazel/bazel_6/actions_path.patch) that hardcodes a set of packages to include in that case, which is currently missing from Bazel 7 in nixpkgs, so we (temporarily, hopefully) reintroduce that patch locally. See: NixOS/nixpkgs#262152 (comment) and NixOS/nixpkgs#94222 for some reading I plan to move more bazel stuff out of shell.nix perhaps, into bazel.nix, as its a tad messy all-in-one ## Test plan `bazel build //client/web/dist` is successful
You probably want to open a new issue for that. Using bazel from nixpkgs correctly is very hard, both with or without |
Description of changes
Introduce bazel 7
What works ?
What does not work ?
More details in a follow-up comment, but here is a PR with two weeks of work, to avoid duplicating work.
Things done
sandbox = true
set innix.conf
? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)