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

idrisPackages: Fix linking to gmp library and cc #58319

Merged
merged 2 commits into from Apr 28, 2019

Conversation

infinisil
Copy link
Member

@infinisil infinisil commented Mar 26, 2019

Fixes #54579 and probably #52796

Ping @ar1a @marsam @shmish111 @mpickering

This has been broken since 5d18129,
which updated idris from 1.3.0 to 1.3.1, which included
idris-lang/Idris-dev#4472 as the cause of the
error. I'm still not entirely sure why this broke it though.

This way should be rather future proof, it uses NIX_CFLAGS to pass
gpm link flags to our CC wrapper directly. The
NIX_CC_WRAPPER_${stdenv.cc.infixSalt}_TARGET_HOST part I'm pretty sure
is needed for the CC wrapper to know that those CFLAGS are meant for the
cc running on the HOST.

The second commit just switches to stdenv's cc.

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 compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-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.

This has been broken since 5d18129,
which updated idris from 1.3.0 to 1.3.1, which included
idris-lang/Idris-dev#4472 as the cause of the
error. I'm still not entirely sure why this broke it though.

This now way should be rather future proof, it uses NIX_CFLAGS to pass
gpm link flags to our CC wrapper directly. The
`NIX_CC_WRAPPER_${stdenv.cc.infixSalt}_TARGET_HOST` part I'm pretty sure
is needed for the CC wrapper to know that those CFLAGS are meant for the
cc running on the HOST.
This should make it work on Darwin with clang.
@infinisil infinisil changed the title idrisPackages: Fix linking to gmp library idrisPackages: Fix linking to gmp library and cc Mar 26, 2019
@infinisil
Copy link
Member Author

@GrahamcOfBorg build idrisPackages.contrib

@infinisil
Copy link
Member Author

Okay well that's weird. I tried building idrisPackages.contrib locally on my mac, which failed with an error that some Idris typeclass wasn't known (DecEq specifically). I am running a very outdated macOS and nix version though, so not sure. @GrahamcOfBorg seems to not have had any problems. Let's try a couple more libraries

@GrahamcOfBorg build idrisPackages.effects idrisPackages.glfw idrisPackages.html idrisPackages.http idrisPackages.idrisscript idrisPackages.pipes idrisPackages.webgl idrisPackages.yaml

And also some of the currently failing ones on Darwin:

@GrahamcOfBorg build idrisPackages.bi idrisPackages.config idrisPackages.electron idrisPackages.eternal idrisPackages.ipkgparser idrisPackages.mhd

@infinisil
Copy link
Member Author

infinisil commented Mar 29, 2019

State of builds:

  • On x86_64, everything builds
  • On aarch, there's lots of timeouts, but no failures
  • For the usually succeeding effects, glfw, ... packages, on darwin the contrib dependency now failed, even though it succeeded when I told it to build it earlier.. It failed with the same error I encountered regarding DecEq, maybe an impurity? This is the check with idrisPackages.contrib failing as a dependency: https://github.com/NixOS/nixpkgs/pull/58319/checks?check_run_id=87664056, and this is the one where it succeeded standalone: https://github.com/NixOS/nixpkgs/pull/58319/checks?check_run_id=87621971. I doubt that the different master version at these different times would've had an influence. @grahamc Can you perhaps check what versions of macOS these two builds used?
  • For the usually failing bi, config, ..., on darwin only two of them failed, soo success?

@infinisil
Copy link
Member Author

@GrahamcOfBorg build idrisPackages.contrib

1 similar comment
@infinisil
Copy link
Member Author

@GrahamcOfBorg build idrisPackages.contrib

@ar1a
Copy link
Contributor

ar1a commented Mar 29, 2019

This closes #54579

@infinisil
Copy link
Member Author

@GrahamcOfBorg build idrisPackages.contrib

@infinisil
Copy link
Member Author

infinisil commented Mar 29, 2019

Doesn't solve the darwin issue.. I have no idea what's up with that tbh. Maybe something during the compilation of Prelude failed, making DecEq unavailable, dunno

This is the error for future reference:

Type checking ./Control/Isomorphism/Extra.idr
./Control/Isomorphism/Extra.idr:14:9:
   |
14 | swapped : DecEq a => (l : a) -> (r : a) -> Iso a a
   |         ^
When checking type of Control.Isomorphism.Extra.swapped:
No such variable DecEq

builder for '/nix/store/qib47cxnxl50vak43a0vk07rzmcvj57f-idris-contrib-1.3.1.drv' failed with exit code 1

I think I'll just merge this for now, idris currently is in an almost unusable state, so anything is an improvement.

@infinisil
Copy link
Member Author

Not sure what to do about this right now. There's clearly a regression here, on my mac idrisPackages.contrib doesn't build anymore, but no idea where it comes from.

@ar1a
Copy link
Contributor

ar1a commented Apr 1, 2019

you may get some insight from #idris or perhaps create an issue

@infinisil
Copy link
Member Author

I'll just merge this now, idris is almost unusable without this

@infinisil infinisil merged commit 3857a8b into NixOS:master Apr 28, 2019
@infinisil
Copy link
Member Author

Seems to build just fine on macOS Mojave, but not on High Sierra. So this problem should automatically disappear as hydra and people update their version

@rycee
Copy link
Member

rycee commented Jul 27, 2019

Been playing around with Idris and notice that this issue seem to affect 19.03, would any mind if I backport this? Seems to work well for me.

@infinisil
Copy link
Member Author

@rycee Sounds good

@rycee
Copy link
Member

rycee commented Jul 27, 2019

Cool, done 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

idris package missing gmp.h, can't compile hello world.
4 participants