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

glibc, gcc: closure-size fixes for ARM targets #15867

Merged
merged 9 commits into from
Jun 22, 2016
Merged

Conversation

elitak
Copy link
Contributor

@elitak elitak commented May 31, 2016

Cross compiling to arm and most of every other target was broken by the closure-size merge.

Here're fixes for ARMv5 and 7, tested on a some packages from the small set that are actually cross-compilable to begin with.

I also disposed of builder.sh for glibc, as its only contents were a couple of hooks that I just moved into nix. I did this so that the cross/native differentiation was a bit cleaner.

@bjornfor
Copy link
Contributor

History is a bit unclean (for instance, commit "works but rebasethese changes"), but +1 for fixing cross-compilation.

@elitak
Copy link
Contributor Author

elitak commented May 31, 2016

Yes, sorry, I will recapitulate before it's merged, reorganizing the fixes to different gcc versions into parallel rather than serial commits. Since this includes changes to the core of nixpkgs, I wanted to leave lots of time for review.

I would like approval especially of removing glibc's builder.sh; it may have consequences I haven't been able to imagine.

@vcunat vcunat added the 1.severity: mass-rebuild This PR causes a large number of packages to rebuild label May 31, 2016
@elitak
Copy link
Contributor Author

elitak commented May 31, 2016

The CI build is failing because of logsize/timeout. Is there any way to up these limits or at least run the same tests locally?

@bjornfor
Copy link
Contributor

bjornfor commented Jun 1, 2016

"nox-review wip --against origin/master"

@dezgeg
Copy link
Contributor

dezgeg commented Jun 1, 2016

cc @edolstra for the glibc parts, I suppose.

Generally looks good to me; now test-building which will take some time...

Also what's our policy for dropping old GCC versions? 4.5 has no in-tree users, 4.6 has only one that could be replaced with a patch.

@edolstra
Copy link
Member

edolstra commented Jun 1, 2016

Removing the glibc builder is fine.

Removing GCC 4.5 would be fine, except that it does have a in-tree user: gnat45, which is the most recent version of gnat we have. Would be nice if gnat could be upgraded to a more recent version (@viric?). Or perhaps we should just drop gnat...

@dvc94ch
Copy link
Contributor

dvc94ch commented Jun 6, 2016

After checking out this branch and rebasing it on master I'm having an issue with:
nix-build pkgs/stdenv/linux/make-bootstrap-tools-cross.nix -A armv7l

/nix/store/g8wapfiw6y5237q3r1xwi38yqsmrvm50-binutils-2.26-armv7l-unknown-linux-gnueabi/armv7l-unknown-linux-gnueabi/bin/ld: cannot find -lcrypt
/nix/store/g8wapfiw6y5237q3r1xwi38yqsmrvm50-binutils-2.26-armv7l-unknown-linux-gnueabi/armv7l-unknown-linux-gnueabi/bin/ld: cannot find -lm
collect2: error: ld returned 1 exit status
Makefile:716: recipe for target 'busybox_unstripped' failed
make: *** [busybox_unstripped] Error 1
builder for ‘/nix/store/11kp4ysi6nxwjlvs1k0jdmg4djyczd98-busybox-1.23.2-armv7l-unknown-linux-gnueabi.drv’ failed with exit code 2
error: build of ‘/nix/store/11kp4ysi6nxwjlvs1k0jdmg4djyczd98-busybox-1.23.2-armv7l-unknown-linux-gnueabi.drv’ failed

@dezgeg
Copy link
Contributor

dezgeg commented Jun 6, 2016

Yes, that is still left broken. I have a branch (based on this) to make it work and will apply it at the same time as this (as both touch mass-rebuild packages) once I've cleaned it up a little.

@dvc94ch dvc94ch mentioned this pull request Jun 7, 2016
7 tasks
vcunat added a commit that referenced this pull request Jun 10, 2016
... needed after closure-size merge (#7701)
vcunat added a commit that referenced this pull request Jun 11, 2016
The evaluation problem happened in while checking find-tarballs.nix
http://hydra.nixos.org/build/36754203/nixlog/1/raw
(it didn't seem worth digging into why exactly)
@vcunat vcunat merged commit fa4fcaf into NixOS:master Jun 22, 2016
@cleverca22
Copy link
Contributor

i think this PR has caused glibc to depend on the bootstrap tools at runtime, testing further

@vcunat
Copy link
Member

vcunat commented Jun 23, 2016

In any case, the dependency is there now, which is rather bad. I'm sorry I haven't checked better.

vcunat added a commit that referenced this pull request Jun 23, 2016
The main output started to retain dependency on bootstrap-tools; see
#15867 (comment)

This reverts commit c05d829, reversing
changes made to f073df6.
@vcunat
Copy link
Member

vcunat commented Jun 23, 2016

Thanks for finding and reporting. I reverted the last staging merge to quickly avoid it for now; fixing can be done on staging later.

@viric
Copy link
Member

viric commented Jun 23, 2016

Sorry, I missed the question on gnat45. If we drop gnat, we would not be able to build any ghdl, which I do use a lot. I have not tried to update gnat45 because it has been working fine until now and I have very limited Ada knowledge.

@vcunat
Copy link
Member

vcunat commented Jun 23, 2016

e8ca9dc is the culprit. I don't yet understand why exactly.

@viric
Copy link
Member

viric commented Jun 23, 2016

fwiw, you need to call the strip specific for the objects architecture. Applying x86_64 strip to arm objects breaks them.

@vcunat
Copy link
Member

vcunat commented Jun 23, 2016

I meant I can't see why the auto-stripping doesn't strip those files; it's possible some fixes inside #15339 would make it apply. Anyway, I suppose for now I'd simply go for:

diff --git a/pkgs/development/libraries/glibc/default.nix b/pkgs/development/libraries/glibc/default.nix
index 0ea6b4b..2a1652a 100644
--- a/pkgs/development/libraries/glibc/default.nix
+++ b/pkgs/development/libraries/glibc/default.nix
@@ -72,6 +72,14 @@ in
       # Get rid of more unnecessary stuff.
       rm -rf $out/var $out/sbin/sln

+      # For some reason these aren't stripped otherwise and retain reference
+      # to bootstrap-tools; on cross-arm this stripping would break objects.
+      if [ -z "$crossConfig" ]; then
+        for i in "$out"/lib/*.a; do
+            strip -S "$i"
+        done
+      fi
+
       # Put libraries for static linking in a separate output.  Note
       # that libc_nonshared.a and libpthread_nonshared.a are required
       # for dynamically-linked applications.

@cleverca22
Copy link
Contributor

i was also surprised this was even possible, the stdenv had code to prevent this

https://github.com/NixOS/nixpkgs/blob/master/pkgs/stdenv/linux/default.nix#L290

but now i see it was disabled

@vcunat
Copy link
Member

vcunat commented Jun 23, 2016

The problem there is that allow* doesn't play well with multiple outputs, as typically some of the outputs do have a large closure and you can't specify these per-output.

@cleverca22
Copy link
Contributor

i see, and the comment explains why that safety is no longer doing its job

@elitak elitak deleted the xgcc-xglibc branch August 29, 2016 10:51
@Ericson2314 Ericson2314 added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label May 18, 2017
Ericson2314 pushed a commit to Ericson2314/nixpkgs that referenced this pull request Dec 29, 2018
matthewbauer added a commit that referenced this pull request Jan 4, 2019
gcc-4.8: fixup cross compilation after merging #15867 for 18.09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.severity: mass-rebuild This PR causes a large number of packages to rebuild 6.topic: cross-compilation Building packages on a different platform than they will be used on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants