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.

Sign up for GitHub

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

clang: fix install on Darwin #39038

Merged
merged 2 commits into from Apr 17, 2018
Merged

clang: fix install on Darwin #39038

merged 2 commits into from Apr 17, 2018

Conversation

strager
Copy link
Contributor

@strager strager commented Apr 17, 2018 •
edited

Motivation for this change

As of Nix 2.0, building the user-environment package on macOS (Darwin) fails because LLVMgold.so is a broken symlink. Fix the issue by not creating the symlink in the first place, since it wouldn't be used on Darwin anyway.

Requests for comments

It seems that Nix 2.0's new buildenv implementation breaks with broken symlinks. Where can I find the code for the new buildenv implementation? The buildenv source in this repository (NixOS/nixpkgs) looks like a Perl script, not a compiled binary which I have in /nix/store/xmi4fylvx4qc79ji9v5q3zfy9vfdy4sv-nix-2.0/libexec/nix/buildenv.

I think these changes will cause full rebuilds. Should they go on master or staging?

Should I bother with other versions of Clang (e.g. 4.0.1 in pkgs/development/compilers/llvm/4/default.nix)? How can I install those with nix-env -i?

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platforms: nix-env -i --no-build-output --max-jobs 3 --cores 8 -f . clang
    • 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
Things to do
  • Test more thoroughly
  • Update other versions of Clang
  • Figure out if LLVM's Gold LTO plugin is supposed to be built on Darwin
  • Find the change in Nix 2.0 or buildenv which caused the broken symlink to break the build

@strager
clang: refactor post-install comments
d3d0c01 
Throughout the evolution of the Clang packages, some comments have
become misplaced. Put some of Clang's postInstall comments next to the
lines they refer to.
@strager
clang: fix install on Darwin
2505aa5 
As of Nix 2.0, building the `user-environment` package on macOS (Darwin)
fails because LLVMgold.so is a broken symlink. Fix the issue by not
creating the symlink in the first place, since it wouldn't be used on
Darwin anyway.
@matthewbauer
Copy link
Member

matthewbauer commented Apr 17, 2018 •
edited

Looks good! LLVM gold is only provided on Linux I think. LTO on Darwin is still broken anyway (#19098).

Note that the base for this should be "staging". I also want to cherry-pick 84088930985e6d8dd805661596690453d4ee3f03 (probably will create a PR) so please ping me after merging this.

@matthewbauer matthewbauer changed the base branch from master to staging April 17, 2018 03:31
@matthewbauer matthewbauer merged commit 2505aa5 into NixOS:staging Apr 17, 2018
matthewbauer added a commit that referenced this pull request Apr 17, 2018
@matthewbauer
clang: only link if LLVMgold exists
7b73c7f 
buildEnv now errors with broken symlinks. Lots of things like this
still exist.

Discussion in #39038.
@strager
Copy link
Contributor Author

strager commented Apr 17, 2018

I'm unfamiliar with nixpkgs' process.

  • Why did you merge this into staging before I addressed my own to-do items (like testing on Linux)?
  • Is there any work I need to do now?
  • What is commit 7b73c7f and how is it different from 2505aa5?

@matthewbauer
Copy link
Member

matthewbauer commented Apr 17, 2018 •
edited

  • This is a pretty straightforward change and I also wanted to apply 9ee69c2 which modifies clang.
  • We should be good. Staging will pick up the commit soon, and we'll verify that no regressions are introduced in Hydra: https://hydra.nixos.org/jobset/nixpkgs/staging. I'll follow the status there and we can always revert it if there are problems.
  • 7b73c7f is just applying the change to older LLVMs.

@LnL7
Copy link
Member

LnL7 commented Apr 17, 2018

@strager The checkboxes are there as information for reviewers, you're not required to complete all of those.

Ericson2314 pushed a commit that referenced this pull request May 23, 2018
@matthewbauer @Ericson2314
clang: only link if LLVMgold exists
d417ada 
buildEnv now errors with broken symlinks. Lots of things like this
still exist.

Discussion in #39038.

(cherry picked from commit 7b73c7f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 501+ 10.rebuild-darwin-stdenv 10.rebuild-linux: 101-500 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@strager @matthewbauer @LnL7 @GrahamcOfBorg @Janik-Haag