bazel: add darwin support #40990

uri-canva · 2018-05-23T13:32:15Z

Motivation for this change

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-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/)
Fits CONTRIBUTING.md.

uri-canva · 2018-05-23T13:35:49Z

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

-  customBash = writeScriptBin "bash" ''
-    #!${stdenv.shell}
-    PATH="$PATH:${lib.makeBinPath [ coreutils ]}" exec ${bash}/bin/bash "$@"
+  customBash = writeCBin "bash" ''


This needs to be a native binary instead of a script because otherwise sandbox-exec will replace the interpreter with /bin/sh instead of using stdenv.shell. This might be a security feature, I could repro the behaviour even when using /bin/bash as the interpreter, in fact it will use /bin/sh as the interpreter even when the first two bytes aren't #!, as long as they're not any of the Mach-O byte signatures.

How does sandbox-exec come into play here?

Here's a simple repro:

mybash:

#!/bin/bash exec /bin/bash "$@"

u.sh:

#!/Users/uri/github/NixOS/nixpkgs/mybash echo $BASH_VERSION

$ which bash; bash --version /Users/uri/homebrew/bin/bash GNU bash, version 4.4.19(1)-release (x86_64-apple-darwin17.4.0) Copyright (C) 2016 Free Software Foundation, Inc. License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html> This is free software; you are free to change and redistribute it. There is NO WARRANTY, to the extent permitted by law. $ which sh; sh --version /bin/sh GNU bash, version 3.2.57(1)-release (x86_64-apple-darwin17) Copyright (C) 2007 Free Software Foundation, Inc. # executing u.sh directly uses mybash as expected $ ./u.sh 4.4.19(1)-release # executing u.sh through sandbox-exec uses /bin/sh $ sandbox-exec -p "(version 1) (allow default)" $PWD/u.sh 3.2.57(1)-release

See #40990 (comment) .

Ah yes, scripts are not allowed as a shebang. I wonder if we should generalise this a bit and expose it.

uri-canva · 2018-05-23T13:37:03Z

pkgs/development/tools/build-managers/bazel/darwin.patch

+     while [ -n "${next}" ]; do
+         previous="${next}"
+-        next=$(readlink "${previous}")
+        next=$(readlink "${previous}" | sed -e 's/\n//g')


Nix's readlink when running on macOS will output an additional newline. This does not happen with macOS' own readlink, or on Linux and NixOS.

This is very weird. We are using coreutils' readlink for both Linux & macOS so I'm not sure why it is different.

Yeah, that sounds very weird.

I tried to isolate the issue as I did before, but when I looked into it more I realised this is just a different symptom of the same issue as sandbox-exec: it's not that it's calling readlink differently, it's a different version of bash: see the comment about sandbox-exec above, and repro for env:

$ env ./u.sh 3.2.57(1)-release

See #40990 (comment) .

uri-canva · 2018-05-23T13:41:13Z

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

  '';

-  postPatch = ''
+  postPatch = stdenv.lib.optionalString stdenv.hostPlatform.isDarwin ''
+    export NIX_LDFLAGS="$NIX_LDFLAGS -F${CoreFoundation}/Library/Frameworks -F${CoreServices}/Library/Frameworks -F${Foundation}/Library/Frameworks"


Ideally we wouldn't need to specify these explicitly, as https://github.com/NixOS/nixpkgs/blob/bbcaf78350d87c4657b1a2451132535467a454eb/pkgs/build-support/bintools-wrapper/setup-hook.sh should add these automatically, like it does -L flags, and like how https://github.com/NixOS/nixpkgs/blob/bbcaf78350d87c4657b1a2451132535467a454eb/pkgs/build-support/cc-wrapper/setup-hook.sh adds -F flags, but when I tried to fix that I found out a bunch of packages are finicky on macOS so I gave up.

Odd, CoreFoundation should be the only special case here (since it's already in the stdenv). I would expect something like this to work.

{ preConfigure = '' export NIX_LDFLAGS="-F${CoreFoundation}/Library/Frameworks $NIX_LDFLAGS" ''; }

Nitpick: the configure phase is more appropriate for changing environment variables, etc.

Tried it:

ERROR: /private/tmp/nix-build-bazel-0.12.0.drv-0/third_party/protobuf/3.4.0/BUILD:399:1: Linking of rule '//third_party/protobuf/3.4.0:js_embed' failed (Exit 1): cc_wrapper.sh failed: error executing command (cd /tmp/bazel_FhkpSXM4/out/execroot/io_bazel && \ exec env - \ APPLE_SDK_PLATFORM='' \ APPLE_SDK_VERSION_OVERRIDE='' \ PATH=/nix/store/allybvvm28bybqrjpkdqvqi7d4r5x38q-zip-3.0/bin:/nix/store/10f4qxb00wk9j6cicd9h1qzwqgnc36ji-python-2.7.14/bin:/nix/store/dparbm2kq903rx6kp1m9zy1l2dcsxcmx-unzip-6.0/bin:/nix/store/gkzi1rn6qlqnalynjifmzgfzwh0x5aa2-which-2.21/bin:/nix/store/468wzkb2swaj4zdc1p6biiw1157jn673-bash/bin:/nix/store/8b0gj3bmcy5bh1zcnhn0146199b3m5kb-clang-wrapper-5.0.1/bin:/nix/store/p85m242dg1zd85ln3cg0agx0q1n0n6h0-clang-5.0.1/bin:/nix/store/5lkrw9dnsgy62qm1ampvww1c5n1pdm4b-coreutils-8.29/bin:/nix/store/jxmp27m26646f67faji9v94d5471ma8y-cctools-binutils-darwin-wrapper/bin:/nix/store/kdff2gim6417493yha769kh00n63lnrw-cctools-binutils-darwin/bin:/nix/store/5lkrw9dnsgy62qm1ampvww1c5n1pdm4b-coreutils-8.29/bin:/nix/store/bc5fk65ijws2938z3nlvkay7i83qjvp7-zulu1.8.0_121-8.20.0.5/bin:/nix/store/5lkrw9dnsgy62qm1ampvww1c5n1pdm4b-coreutils-8.29/bin:/nix/store/lzrwadp3l7np2734jxzqmxsgnjqfy4m7-findutils-4.6.0/bin:/nix/store/jrcykkx415grrjvrrhvs8hgl6pvj6x5b-diffutils-3.6/bin:/nix/store/llk6iyb3ppvyaflbkg5rbv960wlsxpkc-gnused-4.4/bin:/nix/store/c3sa761qjcaa5rg0s8h8qxj0mghkidv5-gnugrep-3.1/bin:/nix/store/gpp9x3ninvcw6m2n5m075q8nckzmcv09-gawk-4.2.1/bin:/nix/store/m88jhn3qgiz48zdllhpvgkz1ricmh89m-gnutar-1.30/bin:/nix/store/61bqs4awrycwdiaz7yb3bxvbyja1ghxr-gzip-1.9/bin:/nix/store/7vldi2061ga9phyhkr3sin8fk2kpc6sl-bzip2-1.0.6.0.1-bin/bin:/nix/store/y2nxw92is8125gdng3bsv23isray1g9m-gnumake-4.2.1/bin:/nix/store/r8bx3qf1bpncb14i9gzma4vr089pc3pv-bash-4.4-p19/bin:/nix/store/l1d4iaik29pkg7pdng7iw8ch75h71n36-patch-2.7.6/bin:/nix/store/03x40jq9ik0j4br0k2jmn288hz25589l-xz-5.2.3-bin/bin:/nix/store/5lkrw9dnsgy62qm1ampvww1c5n1pdm4b-coreutils-8.29/bin \ XCODE_VERSION_OVERRIDE=9.3.1 \ external/local_config_cc/cc_wrapper.sh -fobjc-link-runtime -Wl,-S -o bazel-out/host/bin/third_party/protobuf/3.4.0/js_embed bazel-out/host/bin/third_party/protobuf/3.4.0/_objs/js_embed/third_party/protobuf/3.4.0/src/google/protobuf/compiler/js/embed.o -headerpad_max_install_names -lc++ -no-canonical-prefixes -Wl,-L/nix/store/10f4qxb00wk9j6cicd9h1qzwqgnc36ji-python-2.7.14/lib -Wl,-L/nix/store/zl3vsrqfwx4xvxagdrdqav73yl53cxps-libc++-5.0.1/lib -Wl,-L/nix/store/anpyw2fy2f4qfz3d5yy0x9yxfxnx7sxy-objc4-osx-10.11.6/lib -Wl,-L/nix/store/zl3vsrqfwx4xvxagdrdqav73yl53cxps-libc++-5.0.1/lib -Wl,-L/nix/store/bc5fk65ijws2938z3nlvkay7i83qjvp7-zulu1.8.0_121-8.20.0.5/lib -Wl,-L/nix/store/10f4qxb00wk9j6cicd9h1qzwqgnc36ji-python-2.7.14/lib -Wl,-L/nix/store/zl3vsrqfwx4xvxagdrdqav73yl53cxps-libc++-5.0.1/lib -Wl,-L/nix/store/anpyw2fy2f4qfz3d5yy0x9yxfxnx7sxy-objc4-osx-10.11.6/lib -Wl,-L/nix/store/zl3vsrqfwx4xvxagdrdqav73yl53cxps-libc++-5.0.1/lib -Wl,-L/nix/store/bc5fk65ijws2938z3nlvkay7i83qjvp7-zulu1.8.0_121-8.20.0.5/lib) ld: framework not found Foundation clang: error: linker command failed with exit code 1 (use -v to see invocation)

Do you mean in normal operation cc_wrapper would take care of that?

I need to specify it in the postPatch so I can use the updated flags to patch the build commands.

uri-canva · 2018-05-23T13:42:47Z

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

+    sed -i -e "348 a --copt=\"$(echo $NIX_CFLAGS_COMPILE | sed -e 's/ /" --copt=\"/g')\" \\\\" scripts/bootstrap/compile.sh
+    sed -i -e "348 a --host_copt=\"$(echo $NIX_CFLAGS_COMPILE | sed -e 's/ /" --host_copt=\"/g')\" \\\\" scripts/bootstrap/compile.sh
+    sed -i -e "348 a --linkopt=\"-Wl,$(echo $NIX_LDFLAGS | sed -e 's/ /" --linkopt=\"-Wl,/g')\" \\\\" scripts/bootstrap/compile.sh
+    sed -i -e "348 a --host_linkopt=\"-Wl,$(echo $NIX_LDFLAGS | sed -e 's/ /" --host_linkopt=\"-Wl,/g')\" \\\\" scripts/bootstrap/compile.sh


Bazel will clear the environment when invoking cc and ld, both in the bootstrap build and in the proper build, so the Nix wrappers won't pick up the correct flags. I'm not sure why this didn't cause the build to fail on Linux and NixOS, but on macOS it means it won't be able to find the correct libraries and headers.

We may need to open an issue with Bazel on this. Does this just affect builds of bazel itself or also projects building with bazel?

It's intended to work like that: https://bazel.build/designs/2016/06/21/environment.html
Note that --action_env doesn't work with cc targets, which is why we have to pass them in using the other 4 variants: bazelbuild/bazel#4304.

It affects Bazel's bootstrap build, the build of Bazel built with the bootstrap Bazel, and anything built with the final Bazel.

uri-canva · 2018-05-23T13:47:12Z

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

 # Always assume all markers valid (don't redownload dependencies).
 # Also, don't clean up environment variables.
 , enableNixHacks ? false
+# Apple dependencies
+, libcxx, CoreFoundation, CoreServices, Foundation


We want the current version of CoreFoundation even though we have 10.10 CoreFoundation already because Bazel uses CFURL APIs that only exist in the newer versions:
https://github.com/bazelbuild/bazel/blob/8d782751aa34a1fbb8c3ad38047bb7de2baf1bdb/src/main/cpp/blaze_util_darwin.cc#L106
https://developer.apple.com/documentation/corefoundation/1542764-cfurlcopyresourcepropertyforkey

uri-canva · 2018-05-23T13:49:56Z

Tested by building Bazel on macOS 10.13, Ubuntu 18 and NixOS 18.

Also built a java codebase with it on macOS.

Couldn't test with nox-review as it wasn't working well inside the vagrant box.

uri-canva · 2018-05-23T13:50:16Z

cc @rdnetto @mboes

LnL7 · 2018-05-23T17:57:54Z

Whoa!

LnL7 · 2018-05-23T17:59:22Z

pkgs/development/tools/build-managers/bazel/darwin.patch

+     while [ -n "${next}" ]; do
+         previous="${next}"
+-        next=$(readlink "${previous}")
+        next=$(readlink "${previous}" | sed -e 's/\n//g')


Yeah, that sounds very weird.

LnL7 · 2018-05-23T18:00:30Z

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

-  customBash = writeScriptBin "bash" ''
-    #!${stdenv.shell}
-    PATH="$PATH:${lib.makeBinPath [ coreutils ]}" exec ${bash}/bin/bash "$@"
+  customBash = writeCBin "bash" ''


How does sandbox-exec come into play here?

LnL7 · 2018-05-23T18:03:44Z

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

  '';

-  postPatch = ''
+  postPatch = stdenv.lib.optionalString stdenv.hostPlatform.isDarwin ''
+    export NIX_LDFLAGS="$NIX_LDFLAGS -F${CoreFoundation}/Library/Frameworks -F${CoreServices}/Library/Frameworks -F${Foundation}/Library/Frameworks"


Odd, CoreFoundation should be the only special case here (since it's already in the stdenv). I would expect something like this to work.

{ preConfigure = '' export NIX_LDFLAGS="-F${CoreFoundation}/Library/Frameworks $NIX_LDFLAGS" ''; }

Nitpick: the configure phase is more appropriate for changing environment variables, etc.

LnL7 · 2018-05-23T18:04:58Z

pkgs/top-level/all-packages.nix

@@ -7637,7 +7637,10 @@ with pkgs;
  bam = callPackage ../development/tools/build-managers/bam {};

  bazel_0_4 = callPackage ../development/tools/build-managers/bazel/0.4.nix { };
-  bazel = callPackage ../development/tools/build-managers/bazel { };
+  bazel = callPackage ../development/tools/build-managers/bazel {
+    stdenv = if stdenv.isDarwin then clangStdenv else stdenv;


stdenv and clangStdenv are equivalent on darwin.

uri-canva · 2018-05-28T05:16:53Z

Sorry was away for the weekend, I looked into this more and found this:

https://stackoverflow.com/questions/9988125/shebang-pointing-to-script-also-having-shebang-is-effectively-ignored
https://en.wikipedia.org/wiki/Shebang_(Unix)

In Darwin under MacOS X, the file specified by interpreter must be an executable binary, and cannot be a script.

uri-canva · 2018-05-28T05:19:31Z

An alternative to having customBash be a binary would be to use #!/nix/.../bin/env /nix/.../bin/bash as the shebang instead, but that would require changing the summary replace of /bin/bash with customBash to be more specific, and replace differently depending on whether we want the path to bash, or the string to put in the shebang.

Note that the current find and replace also replaces instances of /bin/bash in code comments and error messages, so it's already overly generic, but I think fixing it that way would be a bigger change than strictly needed, and would probably uncover other issues.

uri-canva · 2018-05-28T05:54:37Z

cc @matthewbauer @LnL7 addressed review

uri-canva · 2018-06-04T04:05:18Z

@matthewbauer @LnL7 ping

LnL7 · 2018-06-04T18:45:12Z

pkgs/build-support/trivial-builders.nix

+    ''
+    n=$out/bin/${name}
+    mkdir -p "$(dirname "$n")"
+    echo -n '${code}' > code.c


it's probably better to use passAsFile for this.

uri-canva · 2018-06-05T02:56:10Z

Used passAsFile.

uri-canva · 2018-06-09T06:04:02Z

@matthewbauer @LnL7 ping

uri-canva · 2018-06-13T02:48:04Z

@matthewbauer @LnL7 ping

matthewbauer · 2018-06-13T03:25:24Z

Merged in 274bb96 with a few minor changes. The .bazelrc stuff is left out for now. Let's see if it's needed when building a few of our bazel packaages.

uri-canva · 2018-06-13T05:35:11Z

I think you misunderstood the point of the bazelrc additions, put up #41911 to add them, will explain them in detail there.

GrahamcOfBorg added 6.topic: darwin Running or building packages on Darwin 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100 labels May 23, 2018

uri-canva commented May 23, 2018

View reviewed changes

LnL7 reviewed May 23, 2018

View reviewed changes

uri-canva force-pushed the bazel-darwin branch from 23cfe1b to 3739b4f Compare May 28, 2018 05:52

LnL7 reviewed Jun 4, 2018

View reviewed changes

bazel: add darwin support

e413943

uri-canva force-pushed the bazel-darwin branch from 3739b4f to e413943 Compare June 5, 2018 02:54

matthewbauer closed this Jun 13, 2018

uri-canva mentioned this pull request Jun 13, 2018

bazel: fix darwin build #41911

Merged

8 tasks

uri-canva deleted the bazel-darwin branch June 13, 2018 08:33

bazel: add darwin support #40990

bazel: add darwin support #40990

Conversation

uri-canva commented May 23, 2018 • edited

Motivation for this change

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uri-canva commented May 23, 2018

uri-canva commented May 23, 2018

LnL7 commented May 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

uri-canva commented May 28, 2018

uri-canva commented May 28, 2018

uri-canva commented May 28, 2018

uri-canva commented Jun 4, 2018

Choose a reason for hiding this comment

uri-canva commented Jun 5, 2018

uri-canva commented Jun 9, 2018

uri-canva commented Jun 13, 2018

matthewbauer commented Jun 13, 2018

uri-canva commented Jun 13, 2018

uri-canva commented May 23, 2018 •

edited