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

Jump to bottom

glibc: fix cross compilation build failure #76972

Merged
merged 2 commits into from Jan 11, 2020
Merged

glibc: fix cross compilation build failure #76972

merged 2 commits into from Jan 11, 2020

Conversation

thefloweringash
Copy link
Member

Motivation for this change

Fix cross compiled glibc which fails due to warnings. I don't know why these warnings only show up in cross compilation, but it lets us avoid a mass rebuild of glibc.

Tested on an x86_64 builder

  • pkgsCross.riscv64.hello
  • pkgsCross.aarch64-multiplatform.hello
  • pkgsCross.armv7l-hf-multiplatform.hello
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.
Notify maintainers

cc @

@thefloweringash thefloweringash changed the title glibc: fix warnings during cross complation glibc: fix cross compilation build failure Jan 5, 2020
@thefloweringash
Copy link
Member Author

thefloweringash commented Jan 5, 2020 •
edited

This change fixes the symptoms, but I'm really not sure of the cause. Why is the gcc 8 warning only showing up now? There's also some overlap with the gdCflags.

@FRidh FRidh mentioned this pull request Jan 5, 2020
Closed
10 tasks
lopsided98
lopsided98 approved these changes Jan 10, 2020
View reviewed changes
Copy link
Contributor

@lopsided98 lopsided98 left a comment

Choose a reason for hiding this comment

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

Fixes the build for me.

@thefloweringash thefloweringash mentioned this pull request Jan 11, 2020
Closed
@Ma27 Ma27 self-requested a review January 11, 2020 13:23
@Ma27
Copy link
Member

Ma27 commented Jan 11, 2020

This change fixes the symptoms, but I'm really not sure of the cause

I assume that this is due to the fact that we still have glibc 2.27 which was released >2 years ago (current version is 2.30) and it's just not "compatible" with newer compilers (the glibc 2.30 update is currently worked on in #66528).

Why is the gcc 8 warning only showing up now?

Just a wild guess, but perhaps nobody noticed? (it seems to occur only when cross-compiling and we had gcc7 by default for a long time).

Ma27
Ma27 approved these changes Jan 11, 2020
View reviewed changes
Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

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

As I didn't see that one before, I basically tried the same after getting notified in #77504 and this seems fine. But before merging, I'd prefer to wait for some more reviews :)

flokli
flokli reviewed Jan 11, 2020
View reviewed changes
Copy link
Contributor

@flokli flokli left a comment

Choose a reason for hiding this comment

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

LGTM - however, it might be wiser to apply it for non-cross too - either here (and then target staging), or as a follow-up PR to staging once this PR is merged.

@Mindavi
Copy link
Contributor

Mindavi commented Jan 11, 2020

Wouldn't it be better to apply the patch for the OOB access? It seems like a fix that's easy to apply.

@flokli
Copy link
Contributor

flokli commented Jan 11, 2020

@Mindavi indeed - so this should fetchpatch https://sourceware.org/git/gitweb.cgi?p=glibc.git;a=patch;h=9c79cec8cd2a6996a73aa83d79b360ffd4bebde6;hp=969c3355069215f1c1cad800a822d0b303fdc1fa unconditionally.

@thefloweringash
glibc: remove TODO that is now done
569f052 
This was preventing a mass-rebuild by returning null. As of
5f2d96b it always returns a string.
@thefloweringash thefloweringash force-pushed the cross-glibc-warnings branch 2 times, most recently from ae4eeb9 to 53a5b0c Compare January 11, 2020 18:29
@thefloweringash thefloweringash changed the base branch from master to staging January 11, 2020 18:29
@thefloweringash
Copy link
Member Author

Updating but patch doesn't apply. Hold on a moment while I sort this out.

@ofborg ofborg bot requested a review from edolstra January 11, 2020 18:38
@flokli flokli merged commit 5e51524 into NixOS:staging Jan 11, 2020
@thefloweringash
Copy link
Member Author

thefloweringash commented Jan 11, 2020 •
edited

@flokli sorry I got the backport wrong. Please revert this or see fixup PR.

@thefloweringash thefloweringash mentioned this pull request Jan 11, 2020
Merged
10 tasks
@flokli
Copy link
Contributor

flokli commented Jan 11, 2020

Reverted 5101476 in cd827f2.

@thefloweringash thefloweringash deleted the cross-glibc-warnings branch January 19, 2020 17:43
@golddranks golddranks mentioned this pull request Jan 27, 2020
Closed
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/january-2020-in-nixos/5771/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flokli flokli flokli approved these changes

@lopsided98 lopsided98 lopsided98 approved these changes

@Ma27 Ma27 Ma27 approved these changes

@edolstra edolstra Awaiting requested review from edolstra

Assignees
No one assigned
Labels
10.rebuild-darwin: 11-100 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+ 10.rebuild-linux-stdenv
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@thefloweringash @Ma27 @Mindavi @flokli @nixos-discourse @lopsided98