Navigation Menu

Skip to content
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

Jump to bottom

buildBazelPackage: autodetect nix toolchain instead of Xcode one on macOS #56033

Merged
merged 1 commit into from Feb 19, 2019
Merged

buildBazelPackage: autodetect nix toolchain instead of Xcode one on macOS #56033

merged 1 commit into from Feb 19, 2019

Conversation

uri-canva
Copy link
Contributor

@uri-canva uri-canva commented Feb 19, 2019 •
edited by kalbasit

Motivation for this change

Certain derivations fail to build with sandboxing enabled, depending on how Xcode / CLT are configured and installed. See #55991.

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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

fixes #55991

This was referenced Feb 19, 2019
Closed
Closed
@uri-canva
Copy link
Contributor Author

Please build bazel-watcher, tensorflow and bazel-deps.

@kalbasit
Copy link
Member

@uri-canva does this work for you now for building bazel-watcher on Darwin?
@LnL7 ofborg built the previous one correctly, but it failed on Hydra. Is there a way to get this tested on hydra before we merge?

@GrahamcOfBorg build bazel-watcher tensorflow bazel-deps

@uri-canva
Copy link
Contributor Author

bazel-watcher built correctly for me on macOS before, assuming Xcode was installed and set up correctly. I broke my Xcode installation to repro the issue where headers couldn't be found, then made bazel-watcher build correctly without requiring Xcode.

@kalbasit
Copy link
Member

looks like bazel-deps has failed. Can you try it locally if it fails for you for this PR? What about on master?

I would try it myself, but I'm currently at a hotel with a very bad connection :(

ERROR: /build/output/external/bazel_tools/tools/jdk/BUILD:188:1: SkylarkAction external/bazel_tools/tools/jdk/platformclasspath_classes/DumpPlatformClassPath.class failed (Exit 1) javac failed: error executing command external/remotejdk_linux/bin/javac -source 8 -target 8 -Xlint:-options -cp external/remotejdk_linux/lib/tools.jar -d bazel-out/host/bin/external/bazel_tools/tools/jdk/platformclasspath_classes ... (remaining 1 argument(s) skipped)
Use --sandbox_debug to see verbose messages from the sandbox
src/main/tools/linux-sandbox-pid1.cc:427: "execvp(external/remotejdk_linux/bin/javac, 0x4f5c10)": No such file or directory
Target //src/scala/com/github/johnynek/bazel_deps:parseproject_deploy.jar failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 24.515s, Critical Path: 0.09s
INFO: 1 process: 1 linux-sandbox.
FAILED: Build did NOT complete successfully
FAILED: Build did NOT complete successfully
builder for '/nix/store/dnjxdqnm32yzjsamxmmhz1yy3qszib9m-bazel-deps-2019-02-01.drv' failed with exit code 1
error: build of '/nix/store/dnjxdqnm32yzjsamxmmhz1yy3qszib9m-bazel-deps-2019-02-01.drv' failed

@uri-canva
Copy link
Contributor Author

uri-canva commented Feb 19, 2019 •
edited

Yeah I noticed, that's broken by the Bazel update on master: https://hydra.nixos.org/job/nixpkgs/trunk/bazel-deps.x86_64-linux, we can't test this PR on that.

@uri-canva
Copy link
Contributor Author

Oh sorry you're right, the failure is actually different, I will have a look.

@uri-canva
Copy link
Contributor Author

Ok, tested it locally, the failure I get is the same on this branch as it is on master, so I think it's ok. We can't verify on hydra because it doesn't have a log.

@kalbasit
Copy link
Member

@uri-canva Fair enough, we'll let Hydra test it out once we merge it down. Can you please address my comment I left inline and I'll wait for ofborg to test it out on Darwin before I merge.

@uri-canva
Copy link
Contributor Author

Added comments.

kalbasit
kalbasit approved these changes Feb 19, 2019
View reviewed changes
Copy link
Member

@kalbasit kalbasit left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kalbasit
Copy link
Member

@GrahamcOfBorg build bazel-watcher

Profpatsch
Profpatsch reviewed Feb 19, 2019
View reviewed changes
pkgs/build-support/build-bazel-package/default.nix
host_linkopts+=( "--host_linkopt=$flag" )
done

BAZEL_USE_CPP_ONLY_TOOLCHAIN=1 \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work on Linux?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a no-op on Linux. See it in the source code here.

@zimbatm
Copy link
Member

zimbatm commented Feb 19, 2019

nix-review pr 56033 produces the following output:

builder for '/nix/store/c8n9njq6jmdiy4abjwz1rxizfidhmg9x-bazel-deps-2019-02-01.drv' failed with exit code 1; last 10 log lines:
  ERROR: /build/output/external/bazel_tools/tools/jdk/BUILD:188:1: SkylarkAction external/bazel_tools/tools/jdk/platformclasspath_classes/DumpPlatformClassPath.class failed (Exit 1) javac failed: error executing command external/remotejdk_linux/bin/javac -source 8 -target 8 -Xlint:-options -cp external/remotejdk_linux/lib/tools.jar -d bazel-out/host/bin/external/bazel_tools/tools/jdk/platformclasspath_classes ... (remaining 1 argument(s) skipped)
  
  Use --sandbox_debug to see verbose messages from the sandbox
  src/main/tools/linux-sandbox-pid1.cc:427: "execvp(external/remotejdk_linux/bin/javac, 0x4f5c10)": No such file or directory
  Target //src/scala/com/github/johnynek/bazel_deps:parseproject_deploy.jar failed to build
  Use --verbose_failures to see the command lines of failed build steps.
  INFO: Elapsed time: 18.244s, Critical Path: 0.41s
  INFO: 7 processes: 7 linux-sandbox.
  FAILED: Build did NOT complete successfully
  FAILED: Build did NOT complete successfully
cannot build derivation '/nix/store/0si8yncz9l72d2dja62ryf451pdcb4wk-env.drv': 1 dependencies couldn't be built

ghost
ghost approved these changes Feb 19, 2019
View reviewed changes
Copy link

@ghost ghost left a comment •
edited by ghost

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that bazel-deps is both broken for Linux on master and 18.09, I think it's safe to merge this as an incremental improvement. If anyone wants a working Linux build they will have to do it themselves.

@zimbatm zimbatm merged commit 718a82b into NixOS:master Feb 19, 2019
@dgarzon dgarzon mentioned this pull request Feb 25, 2019
Closed
@uri-canva uri-canva deleted the bazel-use-cpp-only-toolchain branch February 26, 2019 01:09
@abbradar
Copy link
Member

@uri-canva While meddling with Tensorflow I noticed that the code that adds flags from environment variables isn't really needed - for example NIX_LDFLAGS just works (and is isn't even handled there). Maybe it makes a difference for different derivations? What did it fix for you?

@abbradar
Copy link
Member

abbradar commented Jul 13, 2019 •
edited

Nevermind, I found out this is because Nix hacks are disabled for some packages. Instead of this I propose:

  1. Drop one part of Nix hacks (disabling timestamp checking) because somehow it just works without it for Tensorflow now, and I assume for other packages too;
  2. Always use Nix hacks for building Nix packages because that's much easier w.r.t. various environment variables that Nix uses.

@uri-canva
Copy link
Contributor Author

The nix hacks disable the environment sanitisation logic in Bazel, which means Bazel will reuse cached results regardless of whether they used environment variables that changed in the meantime or not. That should be ok as long as we never use Bazel to cache outputs within nix, which is a reasonable assumption since we would be using the nix store to cache outputs anyway.

@uri-canva uri-canva mentioned this pull request Jul 16, 2019
Merged
10 tasks
@kalbasit kalbasit mentioned this pull request Jul 23, 2019
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kalbasit kalbasit kalbasit approved these changes

@Profpatsch Profpatsch Profpatsch left review comments

Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bazel-watcher 0.9.0 broken in macOS
6 participants
@uri-canva @kalbasit @zimbatm @abbradar @Profpatsch @GrahamcOfBorg