Skip to content

binutils: 2.31.1 -> 2.34#86954

Merged
flokli merged 3 commits intoNixOS:stagingfrom
lovesegfault:binutils-2.34
May 11, 2020
Merged

binutils: 2.31.1 -> 2.34#86954
flokli merged 3 commits intoNixOS:stagingfrom
lovesegfault:binutils-2.34

Conversation

@lovesegfault
Copy link
Member

Motivation for this change

Trying #85951 again as it had to be reverted.

cc. @flokli @pbogdan @thefloweringash

Closes #78204

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.

@lovesegfault lovesegfault requested review from cleverca22, flokli and vcunat and removed request for flokli May 5, 2020 15:59
@lovesegfault
Copy link
Member Author

Please see #85951 (comment) and #85951 (comment).

Still unclear on how to solve the (apparent) bootstrapping issue.

@lovesegfault lovesegfault mentioned this pull request May 5, 2020
10 tasks
@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch. labels May 5, 2020
@ofborg ofborg bot requested review from Ericson2314 and edolstra May 5, 2020 16:29
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels May 5, 2020
@pbogdan
Copy link
Member

pbogdan commented May 5, 2020

I don't feel confident enough to say if it's a good / bad idea but this patch:

diff --git a/pkgs/development/libraries/glibc/common.nix b/pkgs/development/libraries/glibc/common.nix
index 0429c7295fb..52fa7191cb7 100644
--- a/pkgs/development/libraries/glibc/common.nix
+++ b/pkgs/development/libraries/glibc/common.nix
@@ -203,7 +203,7 @@ stdenv.mkDerivation ({
     configureScript="`pwd`/../$sourceRoot/configure"
 
     ${lib.optionalString (stdenv.cc.libc != null)
-      ''makeFlags="$makeFlags BUILD_LDFLAGS=-Wl,-rpath,${stdenv.cc.libc}/lib"''
+      ''makeFlags="$makeFlags BUILD_LDFLAGS=-Wl,-rpath,${stdenv.cc.libc}/lib OBJDUMP=${stdenv.cc.bintools.bintools}/bin/objdump"''
     }

does fix the issue with the generation of the stub functions and at least confirms what @thefloweringash found. I don't know if there's a better way to somehow line up the binutils tools that are included in the stdenv with other tools like objdump which at that stage get pulled from the bootstrap tools as I understand it.

@lovesegfault
Copy link
Member Author

Alright, @pbogdan's patch solved the issue and the full bootstrap seems to work now. Managed to build opencv, linux_latest, and twelf, all of which failed previously.

@flokli
Copy link
Member

flokli commented May 10, 2020

@lovesegfault just to confirm - with "bootstrap", you meant building opencv, linux_latest and twelf, not creating new bootstrap binaries, right?

@lovesegfault
Copy link
Member Author

@flokli That's correct.

@flokli
Copy link
Member

flokli commented May 11, 2020

I'd say let's cook this in staging :-)

@flokli flokli merged commit 629fa8a into NixOS:staging May 11, 2020
@flokli flokli mentioned this pull request May 14, 2020
10 tasks
@FRidh
Copy link
Member

FRidh commented May 17, 2020

@flokli
Copy link
Member

flokli commented May 17, 2020

@FRidh I'm not sure I really understand some of these test failures:

______________________________ test_load_library _______________________________

    def test_load_library():
>       x = find_and_load_library('c')

c/test_c.py:73: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

name = 'c', flags = 2

    def find_and_load_library(name, flags=RTLD_NOW):
        import ctypes.util
        if name is None:
            path = None
        else:
            path = ctypes.util.find_library(name)
            if path is None and name == 'c':
>               assert sys.platform == 'win32'
E               AssertionError: assert 'linux' == 'win32'
E                 - win32
E                 + linux

Why does this code think the platform should be win32? 😕

@FRidh
Copy link
Member

FRidh commented May 18, 2020

Most likely because find_library returned None, and that is something that absolutely should not happen.

$ nix-shell -p python3 --pure --run "python3 -c 'import ctypes.util; print(ctypes.util.find_library(\"c\"))'"

functions fine though.

FRidh added a commit to FRidh/nixpkgs that referenced this pull request May 23, 2020
Pythons find_library is broken with binutils 2.34, and numpy could not import libraries because of not properly aligned ELF's.

This is the second time binutils 2.34 got reverted. Next time, we should have a dedicated Hydra job for it.

This reverts commit 629fa8a, reversing
changes made to 4ddd080.
FRidh added a commit to FRidh/nixpkgs that referenced this pull request May 23, 2020
Reverting this change again because we're going back to binutils 2.31.

NixOS#86954 (comment)

This reverts commit ade7fae.
@vcunat
Copy link
Member

vcunat commented May 23, 2020

Hmm, CVEs are reported for 2.31.1: #63061 so I wonder if there's a version in-between that fixes them but doesn't break python yet.

EDIT: well, we haven't dealt with these in over a year, so perhaps we can just delay a couple weeks longer 😈

@lovesegfault
Copy link
Member Author

The battle to bump binutils rages on! See you guys in the next episode of the binutils Iliad

@luc65r luc65r mentioned this pull request Jun 8, 2020
29 tasks
luc65r pushed a commit to luc65r/nixpkgs that referenced this pull request Dec 27, 2020
FRidh pushed a commit that referenced this pull request Dec 28, 2020
alyssais added a commit to alyssais/nixpkgs that referenced this pull request Oct 21, 2024
This hasn't made it past patchPhase since dde943e ("Revert
"Revert "Merge pull request NixOS#86954 from
lovesegfault/binutils-2.34"""), more than four ago.  It's therefore
safe to say that nobody depends on this continuing to work in recent
Nixpkgs, and all these targetPlatform conditionals are making new
development (like adding cross binutils packages), so let's just
remove it.

It can be brought back if somebody wants to make it work in future,
but given that upstream binutils will continue to diverge from the
stagnant vc4 fork, a better way of doing this would be to upstream vc4
support to binutils, or at the very least use a different expression
for vc4 binutils.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 10.rebuild-linux-stdenv This PR causes stdenv to rebuild on Linux and must target a staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants