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

pynac: use python3 instead of python2 #104476

Merged
merged 1 commit into from Nov 23, 2020
Merged

pynac: use python3 instead of python2 #104476

merged 1 commit into from Nov 23, 2020

Conversation

refnil
Copy link
Contributor

@refnil refnil commented Nov 21, 2020

The use of python2 was a problem with the future sage 9.2. Python3 is supported upstream since sage (the major software using this library) moved itself to python3.
I don't really know about this project so I can't test in depth.

Motivation for this change

Make sage 9.2 run. See #101447 (comment) for details about the original problem.

Things done

Change python2 to python3.
Add dependencies to ncurses since it was used in the configure step.

  • 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.

@FRidh
Copy link
Member

FRidh commented Nov 21, 2020

@GrahamcOfBorg eval

@FRidh FRidh requested a review from timokau November 21, 2020 09:19
@timokau
Copy link
Member

timokau commented Nov 21, 2020

CC @omasanori
Seems like I can't request a review due to NixOS/rfc39#3, that is going to make #104083 less useful as well :/

@omasanori
Copy link
Contributor

OK, I will take a look at it this weekend. Thank you for letting me know!

Copy link
Contributor

@omasanori omasanori left a comment

Choose a reason for hiding this comment

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

There seems no SageMath Trac tickets related to this change. After the current discussion on dependencies is concluded, no objections from my side. Thank you for your PR!

@timokau
Copy link
Member

timokau commented Nov 23, 2020

Thank you for your work @refnil!

@timokau timokau merged commit e4af85f into NixOS:master Nov 23, 2020
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

4 participants