#26353 closed enhancement (fixed)

MR1: Remove special case for Cygwin for BLAS detection when installing fflas_ffpack

Reported by: galois Owned by:
Priority: minor Milestone: sage-8.4
Component: porting: Cygwin Keywords:
Cc: Merged in:
Authors: Erik Bray Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: 09e289a (Commits) Commit: 09e289abfa50503b27fa35592cd466ec3906c6d2
Dependencies: Stopgaps:

Description (last modified by embray)

https://secure.gravatar.com/avatar/c7d341e2b002754f2b21fbab3a3fd072 Erik Bray (@embray) opened a merge request at https://gitlab.com/sagemath/sage/merge_requests/1:

We don't need this special case for Cygwin anymore. openblas and atlas packages should work without special-casing use of the netlib BLAS from Cygwin.






Change History (10)

comment:1 Changed 11 months ago by embray

  • Component changed from PLEASE CHANGE to porting: Cygwin
  • Description modified (diff)
  • Priority changed from major to minor
  • Status set to new
  • Type changed from PLEASE CHANGE to enhancement

comment:2 Changed 11 months ago by embray

  • Status changed from new to needs_review

comment:3 Changed 11 months ago by embray

First attempt at merge request synced from gitlab. A few bugs that happened (which for some reason didn't happen in testing):

1) The post back to gitlab with the link to the ticket did not succeed. Somehow a typo I didn't have in testing crept in. I have already fixed that.

2) For reasons I'm not sure, the ticket was created in an invalid state instead of "new". In fact, the ticket should probably created and immediately set to "needs_review".

The rest seemed to work OK. It will be good to have a mechanism to set other fields such as component and type, but for now I don't have a mechanism for that and instead assume it should be left up to project maintainers to set those fields.

comment:4 Changed 11 months ago by galois

  • Commit changed from baa351bed94e2ac5d1673e9371338e579d5d4094 to 09e289abfa50503b27fa35592cd466ec3906c6d2

New commits added to merge request. I updated the commit SHA-1. This was a forced push. New commits:

09e289awe don't need this special case for Cygwin anymore

comment:5 follow-up: Changed 11 months ago by tscrim

You probably want a mechanism to set the real name in the authors field too as otherwise this ticket will get reverted back from a positive review. Should I do the review from the gitlab side or from the Sage side?

comment:6 in reply to: ↑ 5 Changed 11 months ago by embray

Replying to tscrim:

You probably want a mechanism to set the real name in the authors field too as otherwise this ticket will get reverted back from a positive review.

That's a good point. The names would have to match up though (or, I suppose, the release manager can always update the "trusted names" list.

Should I do the review from the gitlab side or from the Sage side?

Whichever you prefer.

comment:7 Changed 11 months ago by embray

  • Authors set to Erik Bray

comment:8 Changed 11 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:9 Changed 11 months ago by embray

Just deployed an update to the bot which should address all the issues raised in this initial test ticket :)

comment:10 Changed 11 months ago by vbraun

  • Branch changed from u/galois/mrs/1/embray/cygwin/fflas_ffpack/simplify-blas to 09e289abfa50503b27fa35592cd466ec3906c6d2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.