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

Sage python upgrade fixes #56781

Merged
merged 3 commits into from Mar 3, 2019
Merged

Conversation

timokau
Copy link
Member

@timokau timokau commented Mar 3, 2019

Motivation for this change

Fixes the sage build after it was broken by the pip upgrade & strictDeps in #55757.

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 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 nox --run "nox-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.

pip 19 introduced a deprecation warning for python 2.7. Since sage
internally uses pip to query for installed packages, this deprecation
warning will show up at unexpected times. That is ugly and messes with
the testsuite.

Upstream: https://trac.sagemath.org/ticket/27405
@timokau timokau requested a review from FRidh as a code owner March 3, 2019 12:27
];

buildInputs = [
gd
readline
iml
Copy link
Member Author

Choose a reason for hiding this comment

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

@FRidh I don't understand why I need to add iml to buildInputs to make the build succeed. Its already in nativeBuildInputs, yet the build fails without it also being in buildInputs.

@@ -58,6 +58,10 @@ buildPythonPackage rec {
tidyr
]) ++ extraRPackages ++ rWrapper.recommendedPackages;

nativeBuildInputs = [
R # needed at setup time to detect R_HOME (alternatively set R_HOME explicitly)
Copy link
Member Author

Choose a reason for hiding this comment

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

@matthewbauer what is the right way to do this? This is probably wrong right? It would pick up the build R and not be able to find it at runtime.

@timokau
Copy link
Member Author

timokau commented Mar 3, 2019

Merging this right now to fix the current build failure. After feedback I'll make another PR if necessary.

@timokau timokau merged commit 37182b2 into NixOS:master Mar 3, 2019
@timokau timokau deleted the sage-python-upgrade-fixes branch March 3, 2019 19:48
@Ma27
Copy link
Member

Ma27 commented Mar 10, 2019

@timokau sage builds for me on the nixpkgs master, but is still broken on release-19.03. As this seems to fix the build, it should be backported IMHO.

@timokau timokau mentioned this pull request Mar 10, 2019
10 tasks
@timokau
Copy link
Member Author

timokau commented Mar 10, 2019

Thanks for reminding me. #57253

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.

None yet

3 participants