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 dependency updates #114881

Merged
merged 10 commits into from
Mar 11, 2021
Merged

sage dependency updates #114881

merged 10 commits into from
Mar 11, 2021

Conversation

collares
Copy link
Member

@collares collares commented Mar 2, 2021

I will slowly work on this.

@collares collares marked this pull request as draft March 2, 2021 19:58
@collares collares changed the title [WIP] sage dependency updates sage dependency updates Mar 2, 2021
@collares
Copy link
Member Author

collares commented Mar 2, 2021

@ofborg build sage

@AndersonTorres
Copy link
Member

@collares ping :)

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.

Thank you so much for your excellent work as usual!
Just a few minor comments for now; I will review several times for a better result.

pkgs/development/libraries/arb/default.nix Outdated Show resolved Hide resolved
pkgs/applications/science/math/palp/default.nix Outdated Show resolved Hide resolved
@collares
Copy link
Member Author

collares commented Mar 3, 2021

Many thanks for all the reviews everyone :) I value your comments but I also value your time, so (while I will quickly apply any suggested changes) I should clarify that nothing in this draft PR passes tests yet, and it might take a long time to debug the failures and get this to a mergeable state. Thanks for your patience!

@omasanori
Copy link
Contributor

FYI: https://trac.sagemath.org/ticket/30537

@symphorien
Copy link
Member

xcas still works ☑

@collares
Copy link
Member Author

collares commented Mar 4, 2021

@symphorien Thanks for checking! In a sense giac is the most annoying part, because the nofltk patch was not updated so we cannot run tests on fltk-less giac anymore (but we can on giac-with-xcas, which should be enough). I will look into updating the patch, which is why I replaced the fetchpatch by including the file itself.

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.

Heads up: with a single user installation, the store is actually readwrite, and python likes to corrupt the store by compiling pyc files inside it. This can cause strange side effects.

Thank you @symphorien, I did not know this.

Anyway, the failures I am facing occur with or without changes on this branch, so it does not introduce regression on my machine, at least. I have no objections and will approve after rebasing to resolve conflicts.

@collares
Copy link
Member Author

@omasanori Rebased. I was going to wait for the cysignals update but I don't think it matters much since I had already imported the relevant patch. So I think this is ready if it passes tests.

@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 114881 run on x86_64-linux 1

14 packages built:
  • arb
  • giac
  • giac-with-xcas
  • palp
  • python38Packages.cypari2
  • python38Packages.cysignals
  • python38Packages.fpylll
  • python38Packages.pplpy
  • python39Packages.cypari2
  • python39Packages.cysignals
  • python39Packages.fpylll
  • python39Packages.pplpy
  • rankwidth
  • sympow

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.

Thank you all so much for your invaluable help!

@collares
Copy link
Member Author

collares commented Mar 11, 2021

@omasanori You're welcome! Thanks for the review. I'll make another PR updating cypari2, eclib pari (all of which will require adding some sort of patching) and cysignals (which will allow us to remove a patch) as soon as upstream finishes up the pari upgrade.

@symphorien
Copy link
Member

So I think this is ready if it passes tests.

it passes tests so let me merge.

@symphorien symphorien merged commit a72148f into NixOS:master Mar 11, 2021
@collares collares deleted the sage-dependencies branch March 11, 2021 18:37
@timokau
Copy link
Member

timokau commented Mar 11, 2021

Thank you for your efforts to keep the sage package running and up to date :)

@omasanori omasanori mentioned this pull request Mar 11, 2021
@collares collares mentioned this pull request Mar 21, 2021
10 tasks
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.

7 participants