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

binutils: unbreak w/musl via upstream commits #54464

Merged
merged 3 commits into from
Jan 27, 2019

Conversation

dtzWill
Copy link
Member

@dtzWill dtzWill commented Jan 22, 2019

Motivation for this change

53e1db9#commitcomment-32020416

Try building 'busybox-sandbox-shell' without these fixes,
observe crash :(.

Try building 'busybox-sandbox-shell' with these fixes,
observe no crash! \o/

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.

@dtzWill
Copy link
Member Author

dtzWill commented Jan 22, 2019

cc @vcunat @NeQuissimus (mentioning the commit didn't seem to 'ping' that discussion? dunno)

@vcunat
Copy link
Member

vcunat commented Jan 22, 2019

Yes, GitHub doesn't backlink mentions of commits.

@vcunat
Copy link
Member

vcunat commented Jan 22, 2019

I just hope we won't need many iterations to make binutils usable. Otherwise I can't really see why switching the default version. Have you considered taking the whole 2.31 maintenance branch they have in git?

@NeQuissimus
Copy link
Member

@GrahamcOfBorg build busybox-sandbox-shell

@dtzWill
Copy link
Member Author

dtzWill commented Jan 22, 2019 via email

@NeQuissimus
Copy link
Member

Ubuntu bundles a ton of patches: https://packages.ubuntu.com/cosmic/all/binutils-source/filelist

@vcunat
Copy link
Member

vcunat commented Jan 22, 2019

I believe we can easily afford a stdenv rebuild every month. Human time is more of a problem for me here – just deciding whether patches are mutually independent (and I don't mean failures of applying them) and which of them matter for us.

@dtzWill
Copy link
Member Author

dtzWill commented Jan 24, 2019

Okay to merge as-is to get this fixed (things are broken currently!), and we can "upgrade" binutils further as a separate PR/issue? I'd say RFC but that seems a bit heavy for this sort of discussion :).

# un-break features so linking against musl doesn't produce crash-only binaries
./0001-x86-Add-a-GNU_PROPERTY_X86_ISA_1_USED-note-if-needed.patch
./0001-x86-Properly-merge-GNU_PROPERTY_X86_ISA_1_USED.patch
./0001-x86-Properly-add-X86_ISA_1_NEEDED-property.patch
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to use fetchurl for any of these or did you patch them locally?

@dtzWill
Copy link
Member Author

dtzWill commented Jan 25, 2019

Wasn't sure fetchpatch would work re:bootstrap, can probably grab via fetchurlBoot but would need some fetchpatch (filterdiff) alternative to drop the changelog portions of the commits (I kept the commits separate in this PR to help make that clear, esp looking back later on).

Anyway maybe we can, but not sure how reasonable it'd be. Maybe there's a nice way to do it, in which case I agree that's be preferable.

@vcunat
Copy link
Member

vcunat commented Jan 25, 2019

I think I tried hard to find a way when patching glibc with upstream commits, some time ago, and I failed. I ended up putting large gzipped diffs into nixpkgs.

@Mic92
Copy link
Member

Mic92 commented Jan 25, 2019

ok then.

@dtzWill dtzWill merged commit 8b1e530 into NixOS:staging Jan 27, 2019
@dtzWill dtzWill mentioned this pull request Feb 4, 2019
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants