Opened 11 years ago

Closed 11 years ago

#9528 closed defect (fixed)

#8306 completely breaks "sage -upgrade"

Reported by: was Owned by: GeorgSWeber
Priority: blocker Milestone: sage-4.5.1
Component: build Keywords:
Cc: Merged in: sage-4.5.1
Authors: Leif Leonhardy, William Stein Reviewers: Mitesh Patel
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

If you have any version of sage < version 4.5, and you try to upgrade to sage-4.5, the addition of a file pipestatus in #8306 means that your upgrade will instantly and totally break.

Attachments (5)

create-pipestatus.sh (1.4 KB) - added by leif 11 years ago.
Bash script to create specific version of "pipestatus" in $SAGE_ROOT/spkg
install (7.4 KB) - added by was 11 years ago.
this should be put as SAGE_ROOT/spkg/install
create-pipestatus-v2.sh (1.4 KB) - added by leif 11 years ago.
Creates a slightly improved version of pipestatus, no changes to the script code itself
install.2 (7.4 KB) - added by mpatel 11 years ago.
Remove blank line, add chmod. Updated spkg/install based on "4.5"
install.diff (1.7 KB) - added by mpatel 11 years ago.
Diff of spkg/install vs "4.5".

Download all attachments as: .zip

Change History (26)

comment:1 Changed 11 years ago by was

My first idea to fix this is to modify the script spkg/install, which *does* get updated on "sage -upgrade" so that it checks for the file pipestatus, and if it isn't there, then it downloads it.

Unfortunately, install is a shell script, not a python script, so grabbing a file is harder. But it could call python.

This is going to be a little ugly though.

comment:2 follow-up: Changed 11 years ago by leif

Or just create pipestatus in spkg-install; it's an almost trivial script, unlikely to change, and we can check for the bash version number once inside spkg-install and write only the appropriate branch to pipestatus.

comment:3 Changed 11 years ago by leif

The only reason for pipestatus was that we did not want to rely on bash version >=3.0.

comment:4 in reply to: ↑ 2 Changed 11 years ago by was

Replying to leif:

Or just create pipestatus in spkg-install;

This will not work. The problem is that spkg-install isn't run until after pipestatus is needed.

it's an almost trivial script,

I disagree -- It's 33 lines long, and I read it for 2 minutes and didn't fully understand it.

Changed 11 years ago by leif

Bash script to create specific version of "pipestatus" in $SAGE_ROOT/spkg

comment:5 Changed 11 years ago by leif

William, perhaps you can paste the attached shell script (i.e. part of it, removing the first line) into some other script that is run at the right moment in the upgrade process.

(The "harder" part of pipestatus is obviously non-trivial to understand, therefore it contains a reference to its explanation, shortcut. It's actually suited for many Bourne shells, not just bash <3.0.)

comment:6 Changed 11 years ago by was

Lief -- many thanks for posting this, which I *greatly* appreciate.

comment:7 Changed 11 years ago by leif

Btw, you can drop the parentheses in the >=3.0 version, since we don't have to set pipefail in a subshell (it's a stand-alone script).

comment:8 Changed 11 years ago by leif

And pipestatus's

if [ -z "$1" ]; then
  # usage error ...

should be

if [ $# -ne 2 ]; then
  # usage error ...

Changed 11 years ago by was

this should be put as SAGE_ROOT/spkg/install

Changed 11 years ago by leif

Creates a slightly improved version of pipestatus, no changes to the script code itself

comment:9 follow-ups: Changed 11 years ago by was

  • Status changed from new to needs_review

Hi,

I rewrote a script based on your idea (but not using your code). I tested it by:

(1) taking sage-4.5 and move spkg/pipestatus to spkg/pipestatus.orig (2) typed "make", then control-c in a few seconds (3) Do diff spkg/pipestatus spkg/pipestatus.orig and observe that the diff is just a single blank line.

Please review. Since spkg/install is pulled in by SAGE_ROOT/local/bin/spkg-update, this should fix the problem.

comment:10 in reply to: ↑ 9 Changed 11 years ago by leif

Replying to was:

Please review. Since spkg/install is pulled in by SAGE_ROOT/local/bin/spkg-update, this should fix the problem.

I just noticed I had written spkg-install instead of spkg/install... :/

Of course I prefer generating a version-specific pipestatus, but if it is a temporary solution, I'm ok with omitting it.

I'd though at least fix pipestatus's parameter checking as I did in my second version:

...
  cat > pipestatus <<EOF
#!/usr/bin/env bash

if [ \$# -ne 2 -o -z "\$1" -o -z "\$2" ]; then
    echo "Run two commands in a pipeline 'CMD1 | CMD2' and exit"
    echo "with the exit status of CMD1, *not* that of CMD2."
    echo "\$0 cmd1 cmd2"
    exit
fi
...

Dropping the parentheses around (set -o pipefail; eval "\$1 | \$2") is optional, but you should remove -n from the echo in your install.

We cannot yet test upgrading from e.g. 4.4.4 though, can we?

(I've tried your install there, it's ok when deps etc. get updated, too.)

In any case, add a

    chmod +x pipestatus

after the cat...

comment:11 in reply to: ↑ 9 Changed 11 years ago by leif

Replying to was:

(3) Do diff spkg/pipestatus spkg/pipestatus.orig and observe that the diff is just a single blank line.

Unfortunately, the blank line is in the wrong place. #! must be the first characters in the file, otherwise strange things happen...

Changed 11 years ago by mpatel

Remove blank line, add chmod. Updated spkg/install based on "4.5"

Changed 11 years ago by mpatel

Diff of spkg/install vs "4.5".

comment:12 Changed 11 years ago by mpatel

  • Authors set to Leif Leonhardy, William Stein

I apologize for not testing sage -upgrade (and other problems not yet found!). Thanks very much for working on a fix.

Following Leif's suggestions, I've attached a slightly updated install that I'm testing now. Since we've already tested pipestatus on several platforms, I suggest making changes to it in a separate ticket. Perhaps we could remove it altogether, in favor of always auto-generating it?

comment:13 follow-up: Changed 11 years ago by mpatel

Upgrading from 4.4.4 to 4.5 works for me on sage.math with install.2. The long doctests pass. This is with MAKE="-j12" and SAGE_PARALLEL_SPKG_BUILD="yes". A separate, completely serial upgrade with MAKE unset is still running.

Starting with "4.5" on sage.math, I copied install.2 to spkg/ and made a new source distribution with sage -sdist 4.5.1. This builds with MAKE="-j20" and SAGE_PARALLEL_SPKG_BUILD="yes". The long doctests pass. Another build with just MAKE="-j16" is still running.

Thanks again for fixing this.

comment:14 in reply to: ↑ 13 Changed 11 years ago by mpatel

Replying to mpatel:

Upgrading from 4.4.4 to 4.5 works for me on sage.math with install.2. The long doctests pass. This is with MAKE="-j12" and SAGE_PARALLEL_SPKG_BUILD="yes". A separate, completely serial upgrade with MAKE unset is still running.

Starting with "4.5" on sage.math, I copied install.2 to spkg/ and made a new source distribution with sage -sdist 4.5.1. This builds with MAKE="-j20" and SAGE_PARALLEL_SPKG_BUILD="yes". The long doctests pass. Another build with just MAKE="-j16" is still running.

Those builds' long doctests also pass, as do those for a completely serial build of "4.5.1" from scratch on sage.math.

I'm attempting to upgrade from a 4.3.0.1 binary on t2. I'm also building 4.4.4 on bsd.math so that I can test sage -upgrade.

But so far, my review is positive.

comment:15 follow-ups: Changed 11 years ago by mpatel

  • Reviewers set to Mitesh Patel

Upgrading from 4.4.4 also works for me on bsd.math.

On t2: It seems the 4.3.0.1 binary is too old to upgrade. (LinBox doesn't build, possibly because part of the toolchain has changed since 4.3.0.1 was built.) But upgrading from "4.5" to "4.5.1" works, after deleting the former's pipestatus.

I'm still building 4.4.4 on t2, but I think we're ready for the real 4.5.

Can someone check that the small changes made from install to install.2 are OK?

comment:16 in reply to: ↑ 15 Changed 11 years ago by mpatel

Replying to mpatel:

I'm still building 4.4.4 on t2, but I think we're ready for the real 4.5.

The upgrade from 4.4.4 to "4.5.1" on t2 is now working on Singular --- the Sage and Gap packages remain. No problems so far. I need to sleep soon; I'll report again as soon as possible.

Also, "4.5.1" also builds from scratch on bsd.math. The long doctests pass.

comment:17 Changed 11 years ago by was

  • Status changed from needs_review to positive_review

Looks very good. Thanks guys!!

comment:18 in reply to: ↑ 15 Changed 11 years ago by leif

Replying to mpatel:

Can someone check that the small changes made from install to install.2 are OK?

Yes. Positive review, too.

comment:19 Changed 11 years ago by leif

In the long run, we should use something like pipestatus in $SAGE_ROOT/makefile, too.

comment:20 in reply to: ↑ 15 Changed 11 years ago by mpatel

Replying to mpatel:

I'm still building 4.4.4 on t2, but I think we're ready for the real 4.5.

Upgrading from 4.4.4 to "4.5.1" works on t2. The long doctests pass.

comment:21 Changed 11 years ago by rlm

  • Merged in set to sage-4.5.1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.