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

libcxx: fix build on linux musl #70244

Merged
merged 2 commits into from
Nov 7, 2019
Merged

libcxx: fix build on linux musl #70244

merged 2 commits into from
Nov 7, 2019
+13 −7

Conversation

nmattia
Copy link
Contributor

@nmattia nmattia commented Oct 2, 2019

Motivation for this change

Before this, libunwind, libcxxabi and libcxx failed to build on x86_64-linux-musl.

On current master with

for attr in libunwind libcxx libcxxabi; do for platform in linux; do for pkgs in pkgs pkgsStatic; do echo -n "$attr $platform $pkgs "; nix-build -A $pkgs.$attr --argstr system x86_64-$platform &> /dev/null && echo success || echo failure ; done; done; done | column -t

libunwind  linux  pkgs        success
libunwind  linux  pkgsStatic  failure
libcxx     linux  pkgs        success
libcxx     linux  pkgsStatic  failure
libcxxabi  linux  pkgs        success
libcxxabi  linux  pkgsStatic  failure

on this branch:

libunwind  linux  pkgs        success
libunwind  linux  pkgsStatic  success
libcxx     linux  pkgs        success
libcxx     linux  pkgsStatic  success
libcxxabi  linux  pkgs        success
libcxxabi  linux  pkgsStatic  success

I have not, I repeat not, built the pkgsStatic versions on darwin, although the pkgs versions do still work.

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 nix-review --run "nix-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)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @basvandijk

Sorry, something went wrong.

@nmattia nmattia requested a review from matthewbauer as a code owner October 2, 2019 11:47
@nmattia nmattia changed the title Nm fix libcxx libcxx: fix build on linux musl Oct 2, 2019
@basvandijk
Copy link
Member

@GrahamcOfBorg build libunwind libcxx libcxxabi

@basvandijk
Copy link
Member

@GrahamcOfBorg build pkgsStatic.libunwind pkgsStatic.libcxx pkgsStatic.libcxxabi

@domenkozar
Copy link
Member

This should go to staging as it's darwin stdenv rebuild. Good work :)

@nmattia
Copy link
Contributor Author

nmattia commented Oct 2, 2019

Yeah unfortunately due to https://github.com/NixOS/nixpkgs/pull/70244/files#diff-94997ad820e0a722e115eb7b6b93a404R17 I couldn't avoid the rebuild on non-static

matthewbauer
matthewbauer reviewed Oct 2, 2019
View reviewed changes
pkgs/development/libraries/libunwind/default.nix Outdated
@@ -9,6 +9,11 @@ stdenv.mkDerivation rec {
sha256 = "1y0l08k6ak1mqbfj6accf9s5686kljwgsl4vcqpxzk5n74wpm6a3";
};

# There's no "gcc_s" (shared gcc lib) on musl.
Copy link
Member

Choose a reason for hiding this comment

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

According to this, libunwind is broken on musl:

https://github.com/libunwind/libunwind#libc-requirements

We should probably switch to using LLVM's libunwind. I've added it for llvm/8/, but it may make sense to add it here as well:

https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/compilers/llvm/8/libunwind.nix

@nmattia
Copy link
Contributor Author

nmattia commented Oct 3, 2019 •
edited
Loading

We can probably drop the first commit once #70321 lands

nmattia added 2 commits October 4, 2019 10:51
@nmattia
libcxxapi: don't produce shared libs on musl
e494de2
@nmattia
libcxx: don't produce shared libs on musl

Verified

This commit was signed with the committer’s verified signature. The key has expired.
dtzWill Will Dietz
GPG key ID: EBB0EA4124809D02
Expired
Verified
Learn about vigilant mode
fadd37b
@nmattia
Copy link
Contributor Author

nmattia commented Oct 4, 2019

Dropped the first commit since libunwind isn't needed anymore (#70321 ). Good to merge?

@dtzWill
Copy link
Member

dtzWill commented Oct 15, 2019

(do we have a musl team (@nixos/musl? no?)? Regardless, and no worries for new folks, but please cc me on musl issues :). I don't always have time, maybe less time than I'd like, but I would definitely fix something like this should it break again in the future! Sorry folks!)

@matthewbauer matthewbauer changed the base branch from master to staging November 7, 2019 22:12
@matthewbauer matthewbauer merged commit c100983 into NixOS:staging Nov 7, 2019
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux and removed 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-linux: 11-100 labels Nov 7, 2019
@nmattia nmattia deleted the nm-fix-libcxx branch November 8, 2019 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewbauer matthewbauer

@jwiegley jwiegley

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@nmattia @basvandijk @domenkozar @dtzWill @jwiegley @matthewbauer