bazel: fix darwin build on hydra #44181

uri-canva · 2018-07-28T17:31:38Z

Motivation for this change

This fixes the build to not build c/c++ code using system tools referred to with absolute paths in /bin and /usr/bin, allowing Nix to build Bazel on macOS without having Xcode installed.
This is important to allow hydra to build Bazel, and submit a prebuilt derivation to the binary cache.

Originally merged in #42832, reverted in #43479.

This PR takes a different approach: instead of fixing the build of Bazel, it changes the built Bazel to refer to Nix tools instead of system tools. The previous PR would allow Nix to build Bazel using Nix tools, but the built Bazel would use system tools.

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 nox --run "nox-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)
Fits CONTRIBUTING.md.

uri-canva · 2018-07-28T17:33:05Z

cc @Profpatsch @mboes @matthewbauer @LnL7

uri-canva · 2018-07-28T17:40:23Z

There is an interesting issue here: should the Bazel derivation declare all the Nix tools as dependencies? The resulting Bazel is fine to use without the Nix tools as long as no c/c++ code is built, or c/c++ code is built using custom CROSSTOOL files, instead of using the default ones.

It might be hard to find out which dependencies to use in your profile to use the default CROSSTOOL files if we don't include them unconditionally though.

matthewbauer · 2018-07-28T19:23:02Z

The xcode_locator is still an issue though, right? The Hydra machines definitely don't have Xcode installed. I feel like that should be the focus - everything else is basically the same between linux & darwin. IIRC enableNixHacks will make sure NIX_LDFLAGS & NIX_CFLAGS_COMPILE are available. We might just need to make that default to true on Darwin or maybe even Linux as well for consistency. It might be better to try to upstream that match and make it conditional on some flag.

The xcode_locator.m thing is something that might be worth trying to convince Bazel maintainers is a VERY bad idea. They use LSCopyApplicationURLsForBundleIdentifier to find XCode.app locations which is both a very ugly hack and unfortunately does not respect sandboxing. Apple even provides xcode-select -p for the very purpose of locating XCode! This is something they should be using - not some obscure API that breaks sandboxing. I suspect they have some reasons for using it but they definitely need to make it possible to fallback to something for users that don't want to install XCode.

Profpatsch · 2018-07-28T21:07:07Z

I will take a look on Monday. Please ping me if I forget.

uri-canva · 2018-07-29T02:30:57Z

The xcode_locator is still an issue though, right? The Hydra machines definitely don't have Xcode installed.

There are two xcode_locator related issues: one is that it's built during bootstrapping by hardcoding the compilation command:
https://github.com/bazelbuild/bazel/blob/696442c2cf3ab935b328000b78cac210300a7004/scripts/bootstrap/compile.sh#L317
This is fixed by this:
https://github.com/NixOS/nixpkgs/pull/44181/files#diff-4746b81564a01e241a84d455575437e7R80
The second issue is that it gets run to look for Xcode:
https://github.com/bazelbuild/bazel/blob/696442c2cf3ab935b328000b78cac210300a7004/tools/cpp/osx_cc_configure.bzl#L67
When that fails, it will fall back to the same logic Bazel uses on Linux:
https://github.com/bazelbuild/bazel/blob/696442c2cf3ab935b328000b78cac210300a7004/tools/cpp/osx_cc_configure.bzl#L160
So when building with Nix either on hydra or without Xcode installed it should all be ok.

uri-canva · 2018-07-29T02:33:46Z

pkgs/development/tools/build-managers/bazel/default.nix

    zip
    python
    unzip
    makeWrapper
    which
    customBash
-  ] ++ lib.optionals (stdenv.isDarwin) [ libcxx CoreFoundation CoreServices Foundation ];
+    binutils
+  ] ++ lib.optionals (stdenv.isDarwin) [ cctools clang Libsystem libcxx CoreFoundation CoreServices Foundation ];


https://github.com/NixOS/nixpkgs/pull/42832/files#r199536836

Clang shouldn't be needed - it will be provided by stdenv.

Is there no way of having a stdenv with gcc instead of clang on darwin? If so, should I replace all instances of ${clang} with ${stdenv.cc}?

yes, gccStdenv or clangStdenv for the reverse on linux.

uri-canva · 2018-07-29T11:56:02Z

Found out I misunderstood the purpose of the absolute paths in CROSSTOOL, they're just defaults. Also found out about BAZEL_USE_CPP_ONLY_TOOLCHAIN which disables the whole xcode locator part.

uri-canva · 2018-07-31T01:08:56Z

ping @Profpatsch

Profpatsch · 2018-07-31T16:04:41Z

@GrahamcOfBorg build bazel

GrahamcOfBorg · 2018-07-31T16:05:11Z

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: bazel

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnfree = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnfree = true; }
to ~/.config/nixpkgs/config.nix.

GrahamcOfBorg · 2018-07-31T16:05:32Z

Success on x86_64-linux (full log)

Attempted: bazel

Partial log (click to expand)

these paths will be fetched (98.80 MiB download, 101.39 MiB unpacked):
  /nix/store/gpirdnr7yl9wadd6hfc9my6yc4v93qry-bazel-0.15.2
  /nix/store/xkfg1m8q8zn0yc3fw8ds2d3sr6gcvfb5-bash
copying path '/nix/store/xkfg1m8q8zn0yc3fw8ds2d3sr6gcvfb5-bash' from 'https://cache.nixos.org'...
copying path '/nix/store/gpirdnr7yl9wadd6hfc9my6yc4v93qry-bazel-0.15.2' from 'https://cache.nixos.org'...
/nix/store/gpirdnr7yl9wadd6hfc9my6yc4v93qry-bazel-0.15.2

GrahamcOfBorg · 2018-07-31T16:14:13Z

Success on x86_64-darwin (full log)

Attempted: bazel

Partial log (click to expand)

//examples/cpp:hello-success_test                                        PASSED in 0.1s
//examples/java-native/src/test/java/com/example/myproject:hello         PASSED in 0.3s

Executed 2 out of 2 tests: 2 tests pass.
There were tests whose specified size is too big. Use the --test_verbose_timeout_warnings command line option to see which ones these are.
INFO: Build completed successfully, 20 total actions
installing
post-installation fixup
patching script interpreter paths in /nix/store/dyzwhqwpz12kvyd5p7cff9zhynnjssyp-bazel-0.15.2
/nix/store/dyzwhqwpz12kvyd5p7cff9zhynnjssyp-bazel-0.15.2

Profpatsch · 2018-08-02T08:56:20Z

tbh I can’t say much about Darwin patches (no machine). But it looks like at least it builds on Darwin now. Adding a test might be a good idea.

Can you please add some comments to the bash code lines, explaining why the respective lines are needed? It seems to require some pretty arcane knowledge, so better document too much then too little.
Like basically everything in the comments here should go to the source code comments, otherwise it’s just lost information and time.

If you can, please shorten the lines a bit as well.

uri-canva · 2018-08-02T10:21:54Z

Will add comments.

Adding a test might be a good idea.

What kind of test? Doesn't the darwin build cover testing this?

Profpatsch · 2018-08-03T17:07:06Z

What kind of test? Doesn't the darwin build cover testing this?

Ah, I mean some test that runs a simple build with bazel to check if it actually runs, not just builds.

uri-canva · 2018-08-06T03:21:18Z

@Profpatsch Added comments.

Ah, I mean some test that runs a simple build with bazel to check if it actually runs, not just builds.

We already do that by building and running the example tests:

nixpkgs/pkgs/development/tools/build-managers/bazel/default.nix

Line 142 in 4c45016

./output/bazel test --test_output=errors \

Profpatsch · 2018-08-06T07:48:17Z

That looks awesome! @mboes, I think we can merge?

uri-canva · 2018-08-08T22:44:50Z

ping @mboes

uri-canva · 2018-08-11T03:12:16Z

ping @mboes

uri-canva · 2018-08-13T01:45:07Z

ping @mboes

Profpatsch · 2018-08-13T06:32:23Z

I will just merge now, feel free to revert if something’s not right.

GrahamcOfBorg added the 6.topic: darwin Running or building packages on Darwin label Jul 28, 2018

Profpatsch self-requested a review July 28, 2018 21:07

uri-canva commented Jul 29, 2018

View reviewed changes

uri-canva force-pushed the bazel-darwin-hydra branch from b91fad3 to 8938d9d Compare July 29, 2018 11:53

GrahamcOfBorg added 10.rebuild-darwin: 1-10 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Jul 29, 2018

bazel: fix darwin build on hydra

07254df

uri-canva force-pushed the bazel-darwin-hydra branch from 8938d9d to 07254df Compare August 6, 2018 03:10

Profpatsch approved these changes Aug 6, 2018

View reviewed changes

Profpatsch merged commit 8c802d4 into NixOS:master Aug 13, 2018

uri-canva deleted the bazel-darwin-hydra branch August 23, 2018 10:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bazel: fix darwin build on hydra #44181

bazel: fix darwin build on hydra #44181

uri-canva commented Jul 28, 2018 •

edited

Loading

uri-canva commented Jul 28, 2018

uri-canva commented Jul 28, 2018

matthewbauer commented Jul 28, 2018

Profpatsch commented Jul 28, 2018

uri-canva commented Jul 29, 2018

uri-canva Jul 29, 2018

LnL7 Jul 29, 2018

uri-canva commented Jul 29, 2018

uri-canva commented Jul 31, 2018

Profpatsch commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

Profpatsch commented Aug 2, 2018 •

edited

Loading

uri-canva commented Aug 2, 2018

Profpatsch commented Aug 3, 2018 •

edited

Loading

uri-canva commented Aug 6, 2018

Profpatsch commented Aug 6, 2018

uri-canva commented Aug 8, 2018

uri-canva commented Aug 11, 2018

uri-canva commented Aug 13, 2018

Profpatsch commented Aug 13, 2018

bazel: fix darwin build on hydra #44181

bazel: fix darwin build on hydra #44181

Conversation

uri-canva commented Jul 28, 2018 • edited Loading

Motivation for this change

Things done

uri-canva commented Jul 28, 2018

uri-canva commented Jul 28, 2018

matthewbauer commented Jul 28, 2018

Profpatsch commented Jul 28, 2018

uri-canva commented Jul 29, 2018

uri-canva Jul 29, 2018

Choose a reason for hiding this comment

LnL7 Jul 29, 2018

Choose a reason for hiding this comment

uri-canva commented Jul 29, 2018

uri-canva commented Jul 31, 2018

Profpatsch commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

Profpatsch commented Aug 2, 2018 • edited Loading

uri-canva commented Aug 2, 2018

Profpatsch commented Aug 3, 2018 • edited Loading

uri-canva commented Aug 6, 2018

Profpatsch commented Aug 6, 2018

uri-canva commented Aug 8, 2018

uri-canva commented Aug 11, 2018

uri-canva commented Aug 13, 2018

Profpatsch commented Aug 13, 2018

uri-canva commented Jul 28, 2018 •

edited

Loading

Profpatsch commented Aug 2, 2018 •

edited

Loading

Profpatsch commented Aug 3, 2018 •

edited

Loading