Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sage -i http://.../foo.spkg work immediately #13373

Closed
jdemeyer opened this issue Aug 15, 2012 · 28 comments
Closed

Make sage -i http://.../foo.spkg work immediately #13373

jdemeyer opened this issue Aug 15, 2012 · 28 comments

Comments

@jdemeyer
Copy link

The following works right after extracting the Sage source tarball:

./sage -i /path/to/foo.spkg

But this doesn't work for remote packages:

./sage -i http://example.com/foo.spkg

or

./sage -i foo

Using either wget, curl or system Python, we should also make this work.

Apply:

  1. attachment: 13373_sage-i.patch to SAGE_ROOT.
  2. attachment: 13373_remove_scripts.patch to SAGE_LOCAL/bin.
  3. attachment: 13373_doc.patch to the Sage library.

Component: scripts

Author: Leif Leonhardy, Jeroen Demeyer

Reviewer: John Palmieri

Merged: sage-5.3.rc0

Issue created by migration from https://trac.sagemath.org/ticket/13373

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 15, 2012

comment:1

So we need some sage-latest-online-package and sage-download_package [replacement / wrapper] scripts in spkg/bin/?

(In case there's some capable system-wide Python in the user's PATH, the original scripts [copied to spkg/bin/] should already work, right?)

To not have different files with the same name (in local/bin/ and spkg/bin/), we might rename the current [Python] versions to *.py and make sage-{latest-online-package,download_package} plain shell scripts (then moved to spkg/bin/), which just call the original Python implementation if appropriate (Sage's or some other Python available), or otherwise mimic the behavior using wget or curl.

We could probably drop the Python implementations completely in favor of Bash versions (although in theory neither wget nor curl [nor perl], but some python may be available), but as the features of these scripts are IMHO unlikely to change, we can keep parallel implementations without hassle as well.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 16, 2012

Tentative / preliminary "stand-alone" shell script to replace sage-latest-online-package; doesn't use/support Python (even if available) [yet].

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 16, 2012

comment:2

Attachment: latest-online-spkg.sh.gz

Ok, couldn't resist to write some shell script to replace sage-latest-online-package, using curl, wget or $URL_GRABBER instead of Python (which the shell script currently doesn't support at all), and not requiring any "Sage environment".

Do what you want with it... ;-)

Replacing sage-download_package like that should be even easier, I just haven't [yet] done so...

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 16, 2012

comment:3

P.S.: Also hardly tested (especially not using it from sage-spkg / sage -i ...; would need it to be renamed anyway).

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 16, 2012

comment:4

P.P.S.:

Since the scripts are / will be very similar, I'd write a single script which operates depending on basename $0, i.e., on the name it is invoked with, sage-latest-online-package or sage-download_package (one being just a link to the other). Don't know whether Mercurial would like hard links, so one could be a symbolic link to the other.

@jdemeyer
Copy link
Author

comment:5

Replying to @nexttime:

Since the scripts are / will be very similar, I'd write a single script which operates depending on basename $0, i.e., on the name it is invoked with, sage-latest-online-package or sage-download_package (one being just a link to the other). Don't know whether Mercurial would like hard links, so one could be a symbolic link to the other.

Or better: why not create a separate script download_file.sh which simply downloads a given URL and nothing more and which is called by the other scripts?

@jdemeyer
Copy link
Author

comment:6

Concerning verbosity of these scripts: The current Python scripts also show something when downloading, so I wouldn't mind having wget or curl work in its full verbosity.

@jdemeyer
Copy link
Author

Author: Leif Leonhardy, Jeroen Demeyer

@jdemeyer
Copy link
Author

comment:7

I expanded on Leif's idea and made a self-contained patch. This no longer uses neither sage-latest-online-package nor sage-download_package.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

Attachment: 13373_remove_scripts.patch.gz

Attachment: 13373_doc.patch.gz

@jdemeyer

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:10

It mostly seems to work, although I would like other people to review this as well. One problem: if curl, wget, and python are not available, running sage -i without a full URL won't print the relevant error message, because stderr is redirected to /dev/null when sage-download-file is called in sage-spkg:

pkg=`sage-download-file 2>/dev/null "$URL_BASE/$repo/list" |grep "^${PKG_NAME}\(\$\|-\)"`

@jdemeyer
Copy link
Author

comment:11

I was already doubting whether to use 2>/dev/null there, so I removed that.

@jdemeyer
Copy link
Author

comment:12

This doesn't work with Solaris grep :-(

@jhpalmieri
Copy link
Member

comment:13

Now if the programs are not available, sage-download-file is executed four times (for repo in optional experimental standard archive; do ...), so the error message prints four times.

@jdemeyer
Copy link
Author

comment:14

Portability problems fixed by using slightly contrived sed commands instead of grep.

@jdemeyer
Copy link
Author

comment:15

New version, fails immediately if downloading one of the package lists failed.

@jhpalmieri
Copy link
Member

comment:16

I was wondering if you could use egrep instead of grep. sed is fine, though...

Why do you use >&2 on line 280 in echo >&2 "Found $PKG_NAME"?

I think it might be clearer with some extra output, say like this:

diff --git a/spkg/bin/sage-spkg b/spkg/bin/sage-spkg
--- a/spkg/bin/sage-spkg
+++ b/spkg/bin/sage-spkg
@@ -258,6 +258,8 @@ if [ ! -f "$PKG_SRC" ]; then
         URL_BASE="${SAGE_SERVER}packages"
         for repo in optional experimental standard archive; do
             # Download the list of packages.
+            echo
+            echo "Checking list of $repo packages."
             repolist="${repo}.list"
             sage-download-file "$URL_BASE/$repo/list" >$repolist
             if [ $? -ne 0 ]; then
@@ -282,10 +284,12 @@ if [ ! -f "$PKG_SRC" ]; then
             echo >&2 "Error: Could not find a package matching $PKG_NAME in $URL_BASE"
             exit 1
         fi
-        echo >&2 "Found $PKG_NAME"
+        echo
+        echo "Found $PKG_NAME."
     fi
 
     PKG_SRC="`pwd`/$PKG_NAME.spkg"
+    echo "Downloading $PKG_NAME.spkg."
     sage-download-file "$PKG_URL" >"$PKG_SRC"
     if [ $? -ne 0 ]; then
         # Delete failed download

@jdemeyer
Copy link
Author

Attachment: 13373_sage-i.patch.gz

@jdemeyer
Copy link
Author

comment:17

Some further cosmetic changes.

Also one important functional change: first download to a temporary file, such that we don't get a corrupted download.

@jdemeyer
Copy link
Author

comment:18

I am also using curl by default instead of wget because I like the output while downloading better.

@jhpalmieri
Copy link
Member

comment:19

I just ran into a tangentially related problem: if you unpack the Sage tarball and run ./sage -i openssl..... On a system like mark on skynet (Solaris), the system version of patch is stupid, so it can't apply the patches in the spkg, and the build fails. Is it worth trying to address this somehow (on another ticket, not here)? Or should we not worry about it? I'm inclined to not worry about it, personally.

@jhpalmieri
Copy link
Member

comment:20

With Sage's Python, hitting ctrl-c not only interrupts the download but also causes the file to be deleted: the if block in the following gets executed:

    sage-download-file "$PKG_URL" >"$PKG_TMP"
    if [ $? -ne 0 ]; then
        # Delete failed download
        rm -f "$PKG_TMP"
        echo >&2 "Error: failed to download package $PKG_NAME"
        exit 1
    fi

But with curl, and I think wget, hitting ctrl-c dumps me out of the script entirely, so I still end up with the temporary file. What do you think about something like this:

diff --git a/spkg/bin/sage-spkg b/spkg/bin/sage-spkg
--- a/spkg/bin/sage-spkg
+++ b/spkg/bin/sage-spkg
@@ -284,6 +284,7 @@ if [ ! -f "$PKG_SRC" ]; then
     # corrupted .spkg file).
     PKG_TMP="$PKG_NAME.tmp"
     echo ">>> Downloading $PKG_NAME.spkg."
+    trap 'echo' INT
     sage-download-file "$PKG_URL" >"$PKG_TMP"
     if [ $? -ne 0 ]; then
         # Delete failed download
@@ -291,6 +292,7 @@ if [ ! -f "$PKG_SRC" ]; then
         echo >&2 "Error: failed to download package $PKG_NAME"
         exit 1
     fi
+    trap - INT
 
     PKG_SRC="`pwd`/$PKG_NAME.spkg"
     mv -f "$PKG_TMP" "$PKG_SRC"

(This seems to work. Is it the right way to catch ctrl-c?)

@jdemeyer
Copy link
Author

comment:21

Personally, I wouldn't worry about the non-deleting of that temporary file and just leave things as they are. That temporary file will be overwritten anyway if the user tries to download that package again.

Concerning patch: one can always do

./sage -i patch
./sage -i openssl

That should work, so I wouldn't worry too much either.

@jhpalmieri
Copy link
Member

comment:22

Okay, looks good. Works on linux, OS X 10.7, Solaris, OpenSolaris.

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@jdemeyer
Copy link
Author

Merged: sage-5.3.rc0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants