#27384 closed defect (wontfix)
Apply Cysignals fix for Cygwin exception handler on sigaltstack
| Reported by: | embray | Owned by: | |
|---|---|---|---|
| Priority: | blocker | Milestone: | sage-duplicate/invalid/wontfix |
| Component: | porting: Cygwin | Keywords: | |
| Cc: | jdemeyer | Merged in: | |
| Authors: | Erik Bray | Reviewers: | |
| Report Upstream: | N/A | Work issues: | |
| Branch: | u/embray/cygwin/patch-cysignals-altstack-exception-handler (Commits, GitHub, GitLab) | Commit: | f95dcff22000f525b3a114d89dd10b2e134589fa |
| Dependencies: | Stopgaps: |
Status badges
Description (last modified by )
This adds a version of the patch from https://github.com/sagemath/cysignals/pull/108 modified to apply to cysignals 1.8.1.
Since the issue only affects Cygwin I thought for now it would be easier (as in #27267) to apply the patch locally, rather than wait on Jeroen to approve the upstream PR, make a new release of cysignals, and make Sage depend on that release.
This fix certainly resolves the blocker issue well-enough for me for the time being. Fixing this is a prerequisite for fixing #27214.
Change History (6)
comment:1 Changed 3 years ago by
- Branch set to u/embray/cygwin/patch-cysignals-altstack-exception-handler
- Cc jdemeyer added
- Commit set to f95dcff22000f525b3a114d89dd10b2e134589fa
- Description modified (diff)
- Status changed from new to needs_review
comment:2 follow-up: ↓ 3 Changed 3 years ago by
Don't you need https://github.com/sagemath/cysignals/commit/8913e0b66a9acea11a5971868d748cde368ae29b also?
It's not a big deal to make a new cysignals release, just not right now.
comment:3 in reply to: ↑ 2 Changed 3 years ago by
Replying to jdemeyer:
Don't you need https://github.com/sagemath/cysignals/commit/8913e0b66a9acea11a5971868d748cde368ae29b also?
I thought so at first too, but the change that that was fixing was after 1.8.1 so it's not relevant right now (I will need it for any future cysignals update in Sage)
It's not a big deal to make a new cysignals release, just not right now.
I know it's not a big deal. But as I explained (and I think you're agreeing) it's easier to just include the patch in the Sage SPKG for now.
comment:4 Changed 3 years ago by
- Milestone changed from sage-8.7 to sage-duplicate/invalid/wontfix
- Resolution set to duplicate
- Status changed from needs_review to closed
Can this be closed in favour of #27070?
comment:5 Changed 3 years ago by
Sure, as long as Cysignals 1.10 includes the fix (and doesn't break anything else). I will review the changelog to make sure there isn't anything else suspect.
comment:6 Changed 3 years ago by
- Resolution changed from duplicate to wontfix
New commits:
Trac #27384: Patch cysignals with fix for Cygwin's sigaltstack exception