Opened 12 years ago

Closed 12 years ago

Last modified 12 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:

Status badges

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 12 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 12 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 12 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 12 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 12 years ago.
Replaces exit() calls with sys.exit() calls.
trac_7355-sage-i-no-version.6.patch (13.9 KB) - added by timdumol 12 years ago.
License changed.

Download all attachments as: .zip

Change History (23)

Changed 12 years ago by timdumol

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

comment:1 Changed 12 years ago by timdumol

  • Status changed from new to needs_review

Apply patch to scripts repo.

comment:2 Changed 12 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 12 years ago by timdumol

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

comment:3 Changed 12 years ago by timdumol

The fix you stated is up. Thanks!

comment:4 Changed 12 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 12 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 12 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 12 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 12 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 12 years ago by timdumol

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

comment:9 follow-up: Changed 12 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 12 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 12 years ago by timdumol

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

comment:11 Changed 12 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 12 years ago by timdumol

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

comment:12 Changed 12 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 12 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 12 years ago by timdumol

License changed.

comment:14 Changed 12 years ago by timdumol

  • Status changed from needs_work to needs_review

Done.

comment:15 Changed 12 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 12 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 12 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.