Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#7355 closed enhancement (fixed)

Allow sage -i/-f to install packages without stating version number

Reported by: timdumol Owned by: tbd
Priority: minor Milestone: sage-4.3
Component: packages: standard Keywords:
Cc: Merged in: sage-4.3.alpha0
Authors: Tim Dumol Reviewers: Dan Drake
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Having to state the version number of a package all the time is annoying. This fixes that.

Attachments (6)

trac_7355-sage-i-no-version.patch (14.5 KB) - added by timdumol 5 years ago.
Adds sage-latest-online-package and changes sage-spkg to work without version number.
trac_7355-sage-i-no-version.2.patch (14.5 KB) - added by timdumol 5 years ago.
Removed ".spkg" suffix from return value of sage-latest-online-version
trac_7355-sage-i-no-version.3.patch (830 bytes) - added by timdumol 5 years ago.
Added Dan Drake's suggestions. Also strips package versions and matches packages exactly.
trac_7355-sage-i-no-version.4.patch (14.0 KB) - added by timdumol 5 years ago.
Added Dan Drake's suggestions. Also strips package versions and matches packages exactly.
trac_7355-sage-i-no-version.5.patch (13.9 KB) - added by timdumol 5 years ago.
Replaces exit() calls with sys.exit() calls.
trac_7355-sage-i-no-version.6.patch (13.9 KB) - added by timdumol 5 years ago.
License changed.

Download all attachments as: .zip

Change History (23)

Changed 5 years ago by timdumol

Adds sage-latest-online-package and changes sage-spkg to work without version number.

comment:1 Changed 5 years ago by timdumol

  • Status changed from new to needs_review

Apply patch to scripts repo.

comment:2 Changed 5 years ago by ddrake

Right now this doesn't quite work: your sage-latest-online-version prints things with ".spkg" on the end, and that confuses sage-spkg. One fix is to change the line

v = list(set([x.strip() for x in r if x.endswith('.spkg')]))

to

v = list(set([x.strip()[:-5] for x in r if x.endswith('.spkg')]))

to strip off the .spkg at the end.

I'll work on a review this weekend, I hope.

Changed 5 years ago by timdumol

Removed ".spkg" suffix from return value of sage-latest-online-version

comment:3 Changed 5 years ago by timdumol

The fix you stated is up. Thanks!

comment:4 Changed 5 years ago by ddrake

  • Status changed from needs_review to needs_work

I'm looking at the proposed sage-latest-online-package script and have some comments.

I don't really like the 'list.tmp' temporary file business. It's nice to check if the user has the permissions to upgrade, but I think doing so should be the job of whatever actually does the upgrade. The name of the script implies that it tells you the latest online package name, and I'd like to see it do only that.

I see that urlretrieve always uses a temporary file automatically (which gets deleted automatically), so perhaps we could do the first part of spkg_list like so:

web_url = "%s/%s"%(PKG_SERVER, url)
fn = urllib.urlretrieve(web_url)[0]
r = open(fn).read()
[etc]

Also, when you build the list of spkgs, why do list(set(..)), then sort the list? The listings are alphabetized anyway, and if we run into a duplicate, the script will exit on the first occurrence anyway. It looks like just this will be fine:

return [x.strip()[:-5] for x in r if x.endswith('.spkg')]

instead of

v = list(set(...))
v.sort
return v

With the above changes, the script seems to work fine. I'd like to know if anyone uses a nonstandard SAGE_SERVER so we can make sure the script still works for such cases.

comment:5 Changed 5 years ago by ddrake

Harald Schilly says that each subdirectory of packages has a "list" file; for example, http://sagemath.org/packages/standard/list which simplifies the script even more (presuming we can depend on the "list" file being present). Something like this should be okay:

fn, hdrs = urllib.urlretrieve(url + '/list')
spkgs = open(fn).read().splitlines()
if spkgs[0].endswith('.spkg'):
    return spkgs

If you get a 404, the first line will be some html and won't be a string ending in ".spkg".

comment:6 Changed 5 years ago by ddrake

Another idea: use urllib2.urlopen (http://docs.python.org/library/urllib2.html#urllib2.urlopen) which throws an HTTPError when we don't find the list, and doesn't use any temporary files at all.

try:
  data = urllib2.urlopen(some_url)
except HTTPError:
  # can explicitly check for a 404 if we want
  print "couldn't find it"
  sys.exit(whatever)
return data.read().splitlines()

comment:7 Changed 5 years ago by timdumol

I agree with all of your comments. I actually just copied everything from sage-update, so I didn't inspect the code. I think that using the list file is also applicable to sage-update, so should we also update sage-update?

comment:8 Changed 5 years ago by timdumol

  • Status changed from needs_work to needs_review

This new patch should address all the issues you pointed out. This also strips versions and matches exactly on the package name, instead of using pkg_name in spkg.

Changed 5 years ago by timdumol

Added Dan Drake's suggestions. Also strips package versions and matches packages exactly.

comment:9 follow-up: Changed 5 years ago by ddrake

Hrm, I think attachment:rac_7355-sage-i-no-version.3.patch was supposed to be uploaded to another ticket -- I see a one-line patch to sagenb/data/sage/html/worksheet/worksheet.html.

comment:10 in reply to: ↑ 9 Changed 5 years ago by timdumol

Replying to ddrake:

Hrm, I think attachment:rac_7355-sage-i-no-version.3.patch was supposed to be uploaded to another ticket -- I see a one-line patch to sagenb/data/sage/html/worksheet/worksheet.html.

Sorry about that. I have posted the actual patch now.

Changed 5 years ago by timdumol

Added Dan Drake's suggestions. Also strips package versions and matches packages exactly.

comment:11 Changed 5 years ago by ddrake

  • Status changed from needs_review to needs_work

I have one more suggestion / question, and then I think this will be ready to go: in sage-latest-spkg, you have a couple of bare "exit()" statements...do you want sys.exit()? I see that "exit(1)" works in Python, but I've always used sys.exit(). I can't find any documentation for the use of exit(), although it seems to work. Any ideas?

Changed 5 years ago by timdumol

Replaces exit() calls with sys.exit() calls.

comment:12 Changed 5 years ago by timdumol

  • Status changed from needs_work to needs_review

I have no idea why exit() works. Here's the new patch.

comment:13 Changed 5 years ago by was

  • Status changed from needs_review to needs_work

You have to change the license from

 	6	# License: GPLv2 

to

 	6	# License: GPLv2+ = GPLv2 or any later version at the user's option

Changed 5 years ago by timdumol

License changed.

comment:14 Changed 5 years ago by timdumol

  • Status changed from needs_work to needs_review

Done.

comment:15 Changed 5 years ago by ddrake

  • Status changed from needs_review to positive_review

This now looks good. Positive review! Let's get this into 4.3.

comment:16 Changed 5 years ago by mhansen

  • Authors set to Tim Dumol
  • Merged in set to sage-4.3.alpha0
  • Resolution set to fixed
  • Reviewers set to Dan Drake
  • Status changed from positive_review to closed

comment:17 Changed 5 years ago by mvngu

  • Milestone set to sage-4.3
  • Report Upstream set to N/A
  • Summary changed from Allow sage -i/-f to install packages without stating version number. to Allow sage -i/-f to install packages without stating version number

This introduced a bug as fixed at #7544.

Note: See TracTickets for help on using tickets.