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

cc-wrapper: gross hack for staging-next to fix scipy **revert this when merging staging-next to staging** #225273

Closed
wants to merge 3 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 8, 2023

#225220 plus an ugly hack to avoid a mass-rebuild on staging-next

@ghost ghost requested a review from Ericson2314 as a code owner April 8, 2023 08:06
@ghost ghost requested a review from vcunat April 8, 2023 08:06
@ghost
Copy link
Author

ghost commented Apr 8, 2023

@ofborg build python3Packages.scipy

Should build correctly.

@ghost
Copy link
Author

ghost commented Apr 8, 2023

@ofborg build python3Packages.scikit-learn

Copy link
Member

@vcunat vcunat left a comment

Choose a reason for hiding this comment

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

This does not fix the .scikit-learn build for me.

@vcunat
Copy link
Member

vcunat commented Apr 8, 2023

Speaking of temporary hacks this one worked for me:

--- a/pkgs/development/python-modules/scikit-learn/default.nix
+++ b/pkgs/development/python-modules/scikit-learn/default.nix
@@ -56,4 +56,6 @@ buildPythonPackage rec {
   '';
 
+  NIX_LDFLAGS = "-L${stdenv.cc.cc.lib}/lib";
+
   doCheck = !stdenv.isAarch64;
 

(It would probably need adding a condition on stdenv.cc.isGNU.)

@vcunat
Copy link
Member

vcunat commented Apr 8, 2023

Pushed as 46f29d4 as momentary unblocker.

@ghost

This comment was marked as outdated.

@ghost ghost closed this Apr 8, 2023
Adam Joseph added 3 commits April 8, 2023 11:30
As suggested by @trofi here:

  #209870 (comment)

This should fix failures among packages which use gfortran:

  #209870 (comment)
  https://hydra.nixos.org/build/215195834
In #209870 I tried to unify the
treatment of clang and gcc in cc-wrapper as much as possible.
However it appears that I went too far.

Clang requires -isystem flags in order to be able to find gcc's
libstdc++.  Gcc does not need these flags.  If they are added,
gfortran will get confused:

  #209870 (comment)

This commit deunifies the chunk of code that adds the -isystem
flags, and explains why this chunk applies only to clang.
@ghost ghost reopened this Apr 8, 2023
@ghost ghost marked this pull request as draft April 8, 2023 18:36
@github-actions github-actions bot added 6.topic: python 6.topic: stdenv Standard environment 6.topic: TeX Issues regarding texlive and TeX in general and removed 6.topic: stdenv Standard environment 6.topic: TeX Issues regarding texlive and TeX in general 6.topic: python labels Apr 8, 2023
@ofborg ofborg bot requested a review from vcunat April 8, 2023 18:57
@vcunat
Copy link
Member

vcunat commented Apr 8, 2023

Maybe we won't need this more complex temporary hack (i.e. do this smaller amount of rebuilds). So far I'm not aware of any package needing it.

@ghost
Copy link
Author

ghost commented Apr 8, 2023

Maybe we won't need this more complex temporary hack (i.e. do this smaller amount of rebuilds).

Meaning that you intend to merge #225220 and revert 46f29d4?

@vcunat
Copy link
Member

vcunat commented Apr 8, 2023

That's what I assumed so far.

@vcunat
Copy link
Member

vcunat commented Apr 8, 2023

It's certainly simpler and hopefully it will suffice.

@ghost
Copy link
Author

ghost commented Apr 8, 2023

That's what I assumed so far.

Sounds good.

@ghost ghost closed this Apr 8, 2023
vcunat added a commit that referenced this pull request Apr 10, 2023
This is a low-rebuild version of PR #225273
/cc the proper and hopefully complete fix in PR #225220
@ghost ghost deleted the pr/scipy/hack-for-staging-next branch January 23, 2024 06:50
This pull request was closed.
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.

1 participant