Ticket #10157 (closed defect: fixed)

Opened 3 years ago

Last modified 3 years ago

#9799 breaks bdisted binaries (lacking pipestatus)

Reported by: jdemeyer Owned by: jdemeyer
Priority: blocker Milestone: sage-4.6.1
Component: distribution Keywords: scripts
Cc: jhpalmieri Work issues:
Report Upstream: N/A Reviewers: John Palmieri
Authors: Jeroen Demeyer Merged in: sage-4.6.1.alpha0
Dependencies: Stopgaps:

Description (last modified by leif) (diff)

spkg/pipestatus does not appear in binary distributions, but #9799 introduces the use of pipestatus in the top-level Makefile as well.

Also, ./sage -i ... etc. (i.e., sage-sage) should make use of pipestatus, cf. #9811.

Dependency: #9799

Attachments

10157_pipestatus.patch Download (817 bytes) - added by jdemeyer 3 years ago.
sage_scripts patch

Change History

comment:1 Changed 3 years ago by jdemeyer

  • Milestone changed from sage-4.6 to sage-4.6.1

comment:2 Changed 3 years ago by jdemeyer

  • Description modified (diff)

Changed 3 years ago by jdemeyer

sage_scripts patch

comment:3 Changed 3 years ago by jdemeyer

  • Owner changed from tbd to jdemeyer
  • Description modified (diff)
  • Authors set to Jeroen Demeyer

comment:4 follow-up: ↓ 8 Changed 3 years ago by leif

  • Status changed from new to needs_review

Good catch. I didn't know the bdists didn't have spkg/pipestatus.

comment:5 Changed 3 years ago by leif

  • Description modified (diff)

comment:6 Changed 3 years ago by leif

  • Cc jhpalmieri added

In principle positive review; should test this of course (later).

comment:7 Changed 3 years ago by jdemeyer

If you thrust the author of the patch to do the test, I am doing the following:

  • building Sage from source with #9799, #10156, #10157 applied and makefile renamed to Makefile
  • making an sdist and a bdist from this built Sage
  • extract the sdist, make, make ptestlong
  • extract the bdist, make ptestlong

If all this works, I think that #10156 and #10157 are proven to work.

comment:8 in reply to: ↑ 4 Changed 3 years ago by jdemeyer

Replying to leif:

Good catch. I didn't know the bdists didn't have spkg/pipestatus.

Well, the "catch" was made because the above recipe failed.

comment:9 Changed 3 years ago by leif

  • Summary changed from #9799 breaks bdisted binaries to #9799 breaks bdisted binaries (lacking pipestatus)

comment:10 Changed 3 years ago by jdemeyer

All builds and tests mentioned above worked with #9799, #10156, #10157 applied.

comment:11 Changed 3 years ago by jdemeyer

  • Merged in set to sage-4.6.1.alpha0

comment:12 Changed 3 years ago by jdemeyer

  • Merged in changed from sage-4.6.1.alpha0 to sage-4.6.1.alpha1

comment:13 Changed 3 years ago by jhpalmieri

  • Status changed from needs_review to positive_review
  • Reviewers set to John Palmieri

The patch looks completely sensible, and it's worked for me when I tested it out.

comment:14 Changed 3 years ago by jdemeyer

  • Status changed from positive_review to closed
  • Resolution set to fixed
  • Merged in changed from sage-4.6.1.alpha1 to sage-4.6.1.alpha0

Set "merged" to sage-4.6.1.alpha1 by mistake (this patch didn't change).

Note: See TracTickets for help on using tickets.