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

gmp: fix cross compilation to windows #132188

Closed
wants to merge 1 commit into from
Closed

gmp: fix cross compilation to windows #132188

wants to merge 1 commit into from

Conversation

alexfmpe
Copy link
Member

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/)
  • 21.11 Release Notes (or backporting 21.05 Relase notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

alexfmpe
alexfmpe commented Jul 31, 2021
View reviewed changes
pkgs/development/libraries/gmp/6.x.nix
@@ -48,6 +48,10 @@ let self = stdenv.mkDerivation rec {
++ optional (!withStatic && stdenv.hostPlatform.isWindows) "--disable-static --enable-shared"
++ optional (stdenv.hostPlatform.isDarwin && stdenv.hostPlatform.isAarch64) "--disable-assembly";

# Required for "undefined reference to __memset_chk" error
# see https://github.com/msys2/MINGW-packages-dev/pull/6
NIX_LDFLAGS = lib.optionalString stdenv.targetPlatform.isWindows "-lssp";
Copy link
Member Author

@alexfmpe alexfmpe Jul 31, 2021 •
edited

Choose a reason for hiding this comment

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

CPPFLAGS=-fstack-protector also gets through the __memset_chk error, but later hits undefined reference to `__stack_chk_fail'`

@alexfmpe
Copy link
Member Author

Not sure if this should be backported to 4.3.2.nix / 5.1.x.nix

@veprbl veprbl added the 6.topic: windows Running, or buiding, packages on Windows label Jul 31, 2021
@ofborg ofborg bot requested review from vrthra and peti July 31, 2021 05:34
@Mindavi Mindavi added the 6.topic: cross-compilation Building packages on a different sort platform than than they will be run on label Jul 31, 2021
@Mindavi
Copy link
Contributor

Mindavi commented Jul 31, 2021

Diff LGTM

@alexfmpe
Copy link
Member Author

I just ran into the same issue when trying to build pkgsCross.mingw32.hello on master. Same fix. Maybe this should go somewhere more general?
This seems to be a somewhat recent issue, I do not run into it on nixpkgs-20.09.
cc @Ericson2314

@alexfmpe alexfmpe mentioned this pull request Jul 31, 2021
Merged
11 tasks
@alyssais
Copy link
Member

This one also needs to target staging, due to the number of rebuilds.

@alexfmpe alexfmpe changed the base branch from master to staging July 31, 2021 10:42
@alexfmpe
Copy link
Member Author

This one also needs to target staging, due to the number of rebuilds.

Actually, I'm confused.
I see @NixOS/ofborg tagged this as linux/darwin rebuilds, but I'd expect lib.optionalString stdenv.targetPlatform.isWindows to prevent those from being affected? And AFAICT the mingw build was broken, so maybe no actual rebuild is triggered?

@FRidh
Copy link
Member

FRidh commented Jul 31, 2021

I see @NixOS/ofborg tagged this as linux/darwin rebuilds, but I'd expect lib.optionalString stdenv.targetPlatform.isWindows to prevent those from being affected? And AFAICT the mingw build was broken, so maybe no actual rebuild is triggered?

That's because optionalString still needs to return something, so NIX_LDFLAGS is still set. Don't worry about it.

@symphorien
Copy link
Member

I just ran into the same issue when trying to build pkgsCross.mingw32.hello on master. Same fix. Maybe this should go somewhere more general?

adding -lssp could probably go in pkgs/build-support/cc-wrapper/add-hardening.sh maybe?

@alexfmpe
Copy link
Member Author

alexfmpe commented Aug 1, 2021

Turns out this isn't quite right.
While it fixes the build issue, and makes pkgsCross.mingwW64.hello run fine (via wine) on top of master, that's not the case for staging - there it still builds but we get a runtime crash from not finding dlls.

adding -lssp could probably go in pkgs/build-support/cc-wrapper/add-hardening.sh maybe?

This ended up breaking other things, including a dependency of gmp: mcfgthreads-x86_64-w64-mingw32-git, because it can't find libssp

configure:2726: $? = 1
configure:2746: checking whether the C compiler works
configure:2768: x86_64-w64-mingw32-gcc    conftest.c  >&5
/nix/store/d8j3pwvdgqpm06qsg37d0w8j0w8vcsza-x86_64-w64-mingw32-binutils-2.35.1/bin/x86_64-w64-mingw32-ld: cannot find -lssp
collect2: error: ld returned 1 exit status

It seems actually fixing this requires #38632, which in turn requires #132340, so I'm marking this as a draft for the time being.

@alexfmpe alexfmpe marked this pull request as draft August 1, 2021 18:33
sternenseemann added a commit to sternenseemann/nixpkgs that referenced this pull request Aug 7, 2021
@sternenseemann
mpdecimal: support cross compilation to windows
7a5f5d2 
* The __memcpy_chk fix is similar to what is proposed here
  NixOS#132188

* We also need pthreads

Didn't test running, but compiles fine.
@alexfmpe alexfmpe mentioned this pull request Aug 11, 2021
Merged
11 tasks
@alexfmpe
Copy link
Member Author

It seems actually fixing this requires #38632, which in turn requires #132340, so I'm marking this as a draft for the time being.

pkgsCross.mingwW64.gmp builds again on staging (likely since #133479).
I don't think this PR per-se is useful anymore, as the next improvement will come from the above linked PRs.

@alexfmpe alexfmpe closed this Aug 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vrthra vrthra Awaiting requested review from vrthra

@peti peti Awaiting requested review from peti

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@FRidh FRidh Awaiting requested review from FRidh

@jonringer jonringer Awaiting requested review from jonringer

@LnL7 LnL7 Awaiting requested review from LnL7

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

@Mic92 Mic92 Awaiting requested review from Mic92

@mmahut mmahut Awaiting requested review from mmahut

@RaghavSood RaghavSood Awaiting requested review from RaghavSood

@ttuegel ttuegel Awaiting requested review from ttuegel

@zowoq zowoq Awaiting requested review from zowoq

Assignees
No one assigned
Labels
6.topic: cross-compilation Building packages on a different sort platform than than they will be run on 6.topic: windows Running, or buiding, packages on Windows 10.rebuild-darwin: 501+ 10.rebuild-darwin: 5001+ 10.rebuild-darwin-stdenv 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
@alexfmpe @Mindavi @alyssais @FRidh @symphorien @veprbl