Opened 10 years ago

Closed 10 years ago

#12602 closed enhancement (fixed)

Rework download/extract code in sage-spkg

Reported by: jdemeyer Owned by: GeorgSWeber
Priority: major Milestone: sage-5.0
Component: build Keywords:
Cc: jhpalmieri Merged in: sage-5.0.beta10
Authors: Jeroen Demeyer Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #12479 Stopgaps:

Status badges

Description (last modified by jdemeyer)

The current code in sage-spkg to determine which package to install and whether to download it is buggy and too complicated. Also get rid of calls to the newest_version script, get rid of the "download again" part (why was it needed?).

Changes in behaviour:

  1. When specifying a download URL, it will always download the file, even if a corresponding spkg exists locally. This allows for easier development of spkgs, because people often post updated spkgs with the same version number.
  2. "sage -f package" without a version number will use the most recent (by modification time) local spkg in spkg/standard or spkg/optional. Only if no such package exists, it will try to download it. This makes it consistent with "sage -f package-x.y.z" with a version number.
  3. Allow gzip compression (in addition to bzip2 and no compression) of spkgs.

Apply 12602_spkg_download.patch to the SAGE_ROOT repository.

Attachments (1)

12602_spkg_download.patch (13.5 KB) - added by jdemeyer 10 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 10 years ago by jhpalmieri

Questions: if you type sage -i pkg, should it first check spkg/standard (and spkg/optional, spkg/experimental) to see if there is a matching spkg, before trying to download something? Or should it check version numbers to see if the on-line file is more recent? If it checks first in spkg/..., then should there be an option to force downloading, or should we just assume that people will figure out what's going on and delete the local version if they want to download the spkg?

comment:2 Changed 10 years ago by jdemeyer

I still think the default should be to give priority to local packages.

We could support the following (-u for "upgrade"):

./sage -i -u mpfr

which would check the latest online version of mpfr, download it if needed. What do you think?

comment:3 Changed 10 years ago by jdemeyer

  • Description modified (diff)

comment:4 Changed 10 years ago by jdemeyer

  • Dependencies changed from #10479, #10579 to #12479

comment:5 Changed 10 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Status changed from new to needs_review

comment:6 Changed 10 years ago by jdemeyer

  • Description modified (diff)

comment:7 Changed 10 years ago by jdemeyer

  • Description modified (diff)

comment:8 Changed 10 years ago by jdemeyer

  • Description modified (diff)

comment:9 Changed 10 years ago by SimonKing

To what repository is your patch supposed to be added? It apparently does not work with devel/sage.

comment:10 Changed 10 years ago by jdemeyer

  • Description modified (diff)

comment:11 follow-up: Changed 10 years ago by jhpalmieri

On line 268, why cd /? The working directory remains / for a while after that, which makes me a little uncomfortable.

comment:12 in reply to: ↑ 11 Changed 10 years ago by jdemeyer

Replying to jhpalmieri:

On line 268, why cd /?

No real reason. I just wanted to make sure that PKG_SRC is an absolute path. So doing "cd /" would give a failure if PKG_SRC had a relative path.

I changed the relevant code to the safer

# Do a final check that PKG_SRC is a file with an absolute path
cd /
if [ ! -f "$PKG_SRC" ]; then
    echo >&2 "Error: spkg file '$PKG_SRC' not found."
    echo >&2 "This shouldn't happen, it is a bug in the sage-spkg script."
    exit 1

# Go back to SAGE_ROOT where we have less chance of completely messing
# up the system if we do something wrong.
cd "$SAGE_ROOT" || exit

Changed 10 years ago by jdemeyer

comment:13 Changed 10 years ago by jhpalmieri

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

Okay, this latest change looks good. The code as a whole also looks good. I've been using this for a while (as part of the sage-gcc betas), and it seems to work well as part of the Sage build, when downloading optional packages (with and without version numbers specified), and when installing local packages.

comment:14 Changed 10 years ago by jdemeyer

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