Skip to content

cython: 0.29.21 -> 0.29.22, backport Cython 3.0 trashcan support#115200

Merged
SuperSandro2000 merged 2 commits intoNixOS:stagingfrom
collares:cython-trashcan
Mar 6, 2021
Merged

cython: 0.29.21 -> 0.29.22, backport Cython 3.0 trashcan support#115200
SuperSandro2000 merged 2 commits intoNixOS:stagingfrom
collares:cython-trashcan

Conversation

@collares
Copy link
Member

@collares collares commented Mar 5, 2021

Users of Sage may get random stack overflows on large object deallocations without this patch. It backports Cython 3.0 trashcan support to Cython 0.X.

This patch does not affect Python code unless the code explicitly uses the feature, which is both an argument for patching the global Cython (it's fairly safe) and for making sage use a modified Cython, since only a modified cypari2 will use this change. Just let me know if applying the patch globally is a bad idea, I won't be offended :) In fact, I didn't run nixpkgs-review because it rebuilds everything that uses Python.

(I can also just work around the Sage test failure instead of applying the patch at all, but users can still sporadically hit the problem during normal use.)

cc @omasanori @timokau This is causing the totallyreal.pyx aarch64 failure in the dependency update PR, I believe.

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

@collares collares requested review from FRidh and jonringer as code owners March 5, 2021 15:48
@ofborg ofborg bot added 6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Mar 5, 2021
@collares collares changed the title cython: backport Cython 3.0 trashcan support cython: 0.29.21 -> 0.29.22, backport Cython 3.0 trashcan support Mar 5, 2021
@collares
Copy link
Member Author

collares commented Mar 5, 2021

Updated the package version while I'm at it.

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

I build up to home-assistant which failed due to an unrelated error. If @jonringer or @FRidh approve it this can be merged.

Copy link
Member

@FRidh FRidh left a comment

Choose a reason for hiding this comment

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

This is fine, but you will be responsible then for keeping the patch up to date. When it does not apply, and blocks cython, it may get removed.

@collares
Copy link
Member Author

collares commented Mar 6, 2021

Deal! This patch is two years old, was made for Cython 0.29.5 and hasn't bitrotted yet, so I think it's unlikely to ever block Cython (well, until Cython 3.0 gets released, at which point the patch should be removed). But if it happens I'll update the patch.

@SuperSandro2000 SuperSandro2000 merged commit b122c50 into NixOS:staging Mar 6, 2021
@collares
Copy link
Member Author

collares commented Mar 6, 2021

@FRidh I just noticed #114452 while rebasing patches that depended on this. I am afraid the cypari2 upgrade there could cause Sage test failures, so can this land on staging-next directly? If not, I would be grateful if you could drop the cypari2 update from #114452 (7a3db26) so I could land it on staging along with Sage test fixes. Thanks!

@FRidh
Copy link
Member

FRidh commented Mar 6, 2021

@FRidh I just noticed #114452 while rebasing patches that depended on this. I am afraid the cypari2 upgrade there could cause Sage test failures, so can this land on staging-next directly?

No, its too big a rebuild and staging-next has been stuck already too long.

If not, I would be grateful if you could drop the cypari2 update from #114452 (7a3db26) so I could land it on staging along with Sage test fixes. Thanks!

Feel free to open a PR with a revert if it is needed. I see now it isn't the first time.

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

Labels

6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants