Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#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 embray)

This adds a version of the patch from 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 embray

  • Authors set to Erik Bray
  • 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

New commits:

f95dcffTrac #27384: Patch cysignals with fix for Cygwin's sigaltstack exception

comment:2 follow-up: Changed 3 years ago by jdemeyer

Don't you need 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 embray

Replying to jdemeyer:

Don't you need 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 jdemeyer

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

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 embray

  • Resolution changed from duplicate to wontfix
Note: See TracTickets for help on using tickets.