Opened 6 years ago

Closed 5 years ago

#16759 closed enhancement (fixed)

install_package() is obsolete

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-6.9
Component: misc Keywords:
Cc: was Merged in:
Authors: Jeroen Demeyer Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: da03be1 (Commits) Commit: da03be13988e41fc483ee2001654a55d4e45ee95
Dependencies: Stopgaps: #16760

Description (last modified by jdemeyer)

Packages should be installed using sage -i from a shell, not with the install_package() Sage command.

For consistency in error messages, we introduce the exception PackageNotFoundError which should be used to signal that a package is not installed. This idea was taken from an existing but unused exception OptionalPackageNotFoundError (which is removed).

Change History (40)

comment:1 follow-up: Changed 6 years ago by jhpalmieri

It also looks like sage --standard is broken: it is using old package information. For example, it thinks that the current version of Sage is 5.13. Also, pillow is now a standard package, replacing pil, but pil shows up.

I'm not sure where to get a list of all of the current standard packages, but once we had that, we might be able to get the version numbers from http://www.sagemath.org/packages/upstream/.

comment:2 in reply to: ↑ 1 Changed 6 years ago by jdemeyer

Replying to jhpalmieri:

It also looks like sage --standard is broken: it is using old package information.

I would say that sage --standard is doing the right thing, but somehow http://www.sagemath.org/packages/standard/ is no longer updated. The right solution is updating the website script to take into account the new git-style packages (which is certainly a different issue than the one this ticket is about).

comment:3 Changed 6 years ago by jdemeyer

...that being said, it makes no sense to work on this ticket if the underlying sage --standard command isn't fixed.

comment:4 Changed 6 years ago by kcrisman

Along similar lines, does sage -i work properly with installing optional packages that have both "old" and "new" package versions?

But I agree with William that this is important. I'm working at a Sage Days prep right now with exactly the kind of people William was talking about in the thread that led to this - people who know quite a bit of math and might want to use some specialized optional package (say, polymake) but would need an awful lot of hand-holding to make it through some of this weird behavior.

comment:5 Changed 6 years ago by jhpalmieri

  • Stopgaps set to #16760

Here's an option: turn off install_package until we can fix this. See #16760.

comment:6 Changed 6 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

comment:7 Changed 5 years ago by charpent

This might be related to the case of the tarball : you use Pillow-2.2.2.tar.gz. it turns out that sage -sh sage-fix-pkg-checksums, that computes the checksumes, will treat only lower-case-named tarballs ... except on Macs, whose filesystems will find Pillow-2.2.2.tar.gz when asked for pillow-2.2.2.tar.gz. See #18229 for discussion.

This is documented in the Developer's guide, paragraph "Directory structure", that states :

"The build scripts and associated files are in a subdirectory SAGE_ROOT/build/pkgs/package, where you replace package with a lower-case version of the upstream project name."

I suppose that the original Pillow porter might have been a Mac user, and that nobody caught the problem.

HTH,

Last edited 5 years ago by charpent (previous) (diff)

comment:8 follow-up: Changed 5 years ago by jhpalmieri

From the sage-fix-pkg-checksums script, it looks like the case for the directory name (in SAGE_ROOT/build/pkgs/) must match the case for the tarball (except on OS X). Why not change the policy that currently requires the directory to be all lower-case? Would it cause upgrade problems on OS X if we renamed "pillow" to "Pillow", given the stupid way it deals with case?

comment:9 Changed 5 years ago by jhpalmieri

Or rewrite sage-fix-pkg-checksums to convert the tarball to lowercase to determine the directory name. That might be a better choice.

  • src/bin/sage-fix-pkg-checksums

    diff --git a/src/bin/sage-fix-pkg-checksums b/src/bin/sage-fix-pkg-checksums
    index 65542b9..b42463d 100755
    a b fi 
    1111for upstream in "$@"
    1212do
    1313    tarball=`basename "$upstream"`
    14     pkg_name=${tarball%%-*}
     14    pkg_name=`echo ${tarball%%-*} | tr '[:upper:]' '[:lower:]'`
    1515    pkg_compression=${tarball#*.tar}   # gz or bz2
    1616    if [ -d "$SAGE_ROOT/build/pkgs/$pkg_name" ]; then
    1717        sage_version=`cat "$SAGE_ROOT/build/pkgs/$pkg_name/package-version.txt" | sed 's/\.p[0-9][0-9]*$//'`

Maybe print a warning whenever the case is changed, just to be safe?

comment:10 Changed 5 years ago by jhpalmieri

Oops, there are more changes required: we need to keep track of pkg_name for the directory, and also the version with the original case for use in

            echo "tarball=$original_pkg_name-VERSION.tar$pkg_compression" > $checksums

comment:11 in reply to: ↑ 8 ; follow-up: Changed 5 years ago by charpent

Replying to jhpalmieri: Would you mind joining this thead on sage-devel ? There is not only a (small) technical problem, but also a policy problem, and this thread i probably not the right place to discuss it. In other words, we know that at least the sage-fix-spkg-checksums script won't work on mixedCase tarballs, and I wonder what will be broken if this script is fixed to accept mixed-case tarball names. Leif thinks that's only a convention, but I am not so sure...

Update : Just saw your patch. It fixes the technical problem, but what other holes does it open ?

From the sage-fix-pkg-checksums script, it looks like the case for the directory name (in SAGE_ROOT/build/pkgs/) must match the case for the tarball (except on OS X). Why not change the policy that currently requires the directory to be all lower-case? Would it cause upgrade problems on OS X if we renamed "pillow" to "Pillow", given the stupid way it deals with case?

comment:12 in reply to: ↑ 11 Changed 5 years ago by jhpalmieri

Replying to charpent:

Update : Just saw your patch. It fixes the technical problem, but what other holes does it open ?

Good question. Since we want to support OS X, we cannot allow different files whose names are the same except for case differences, so we cannot allow different directories build/Pillow and build/PiLlOW. So I hope it doesn't open any holes. As I suggested, maybe the script should print a warning if there are case differences between the tarball and the directory: "Note: case mismatch between upstream/Pillow-2.2.2.tar.gz and build/pkgs/pillow."

comment:13 Changed 5 years ago by jhpalmieri

See #18344.

comment:14 follow-up: Changed 5 years ago by leif

I'd keep all (Sage-internal) "package names" (i.e., folders) in build/pkgs/ lowercase.

We should IMHO simply be more flexible w.r.t. upstream tarballs (their names and also their top-level folders).

The easiest way to achieve this is to record the upstream tarball's name as well; currently we only have package-version.txt, which doesn't contain anything but what we consider the version of the package.

In the long run, we wouldn't have to rename or modify upstream tarballs (which was the goal anyway), and could even provide the original URL(s) to obtain them, at least as an alternative, and also for documentation purposes.

(There's more data we could move/replicate from SPKG.txt into more machine-readable form, finally creating SPKG.txt from, or augmenting it with data from other meta-data files, such as authors, copyright, upstream contact/address for bug reports, etc.)

comment:15 in reply to: ↑ 14 ; follow-up: Changed 5 years ago by jhpalmieri

Replying to leif:

I'd keep all (Sage-internal) "package names" (i.e., folders) in build/pkgs/ lowercase.

Yes.

We should IMHO simply be more flexible w.r.t. upstream tarballs (their names and also their top-level folders).

Yes.

The easiest way to achieve this is to record the upstream tarball's name as well; currently we only have package-version.txt, which doesn't contain anything but what we consider the version of the package.

The tarball name (without the version) is stored in checksums.ini already.

See the proposal at #18344. I think that the tarball and the directory name should agree except possibly for case differences. I guess we could allow completely different names for the two, but that would make various parts of the system more complicated: sage-fix-pkg-checksums, sage-spkg, etc., not to mention making everything more opaque: why should a file foo.tar.gz be the right tarball for build/pkgs/bar?

comment:16 in reply to: ↑ 15 Changed 5 years ago by leif

Replying to jhpalmieri:

The tarball name (without the version) is stored in checksums.ini already.

I disregarded M$ Windows files... ;-)

comment:17 Changed 5 years ago by leif

Related:

> I tried to install pyopenssl in order to run sagenb in sage 6.6 with
> secure=True. It fails with Error 404, see below.
> 
> Regards,
> 
> CH
> 
> I get:
> $ ./sage -i pyopenssl
> Attempting to download package pyopenssl
>>>> Checking online list of optional packages.
> [.]
>>>> Found pyopenssl-0.13.p0
>>>> Trying to download http://www.sagemath.org/spkg/optional/pyopenssl-0.13.p0.spkg
> ....
> IOError: [Errno 404] Not Found:
> '//www.sagemath.org/spkg/optional/pyopenssl-0.13.p0.spkg'
> Error: failed to download package pyopenssl-0.13.p0
> 
> $ LANG=C wget http://www.sagemath.org/spkg/optional/pyopenssl-0.13.p0.spkg
> --2015-05-03 16:02:09--
> http://www.sagemath.org/spkg/optional/pyopenssl-0.13.p0.spkg
> Resolving www.sagemath.org (www.sagemath.org)... 128.208.178.250
> Connecting to www.sagemath.org (www.sagemath.org)|128.208.178.250|:80... connected.
> HTTP request sent, awaiting response... 404 Not Found
> 2015-05-03 16:02:10 ERROR 404: Not Found.

Unfortunately setting SAGE_SERVER is borked as well (because the mirrors still
have 'spkg/', not 'packages/', which the scripts simply ignore), but you could
use a mirror "manually":


  wget http://mirror.switch.ch/mirror/sagemath/spkg/optional/pyopenssl-0.13.p0.spkg

  ./sage -i pyopenssl-0.13.p0.spkg   # Note the extension

Setting SAGE_SERVER leads into an endless loop: ;-)

SAGE_SERVER="http://sage.scipy.org/sage/" ./sage --optional
Using Sage Server http://sage.scipy.org/sage/packages
HTTP Error 404: Not Found



********************************************************************************



Error contacting http://sage.scipy.org/sage/packages/optional/list. Try using an alternative server.
For example, from the bash prompt try typing

   export SAGE_SERVER=http://sage.scipy.org/sage/

then try again.



********************************************************************************

The above fails for a different reason, but try yourself with some other mirror...

comment:18 Changed 5 years ago by leif

  • Dependencies set to #15642

comment:19 Changed 5 years ago by leif

  • Milestone changed from sage-6.4 to sage-6.7

comment:20 Changed 5 years ago by leif

  • Dependencies changed from #15642 to #15642, #18407

comment:21 Changed 5 years ago by leif

  • Description modified (diff)

comment:22 Changed 5 years ago by jdemeyer

  • Dependencies #15642, #18407 deleted
  • Description modified (diff)
  • Summary changed from Fix install_package() to install_package() is obsolete

I vote for removing the install_package() command. Installing packages from within Sage can never work properly anyway:

  1. for some packages, you need to rebuild the Sage library, which cannot be done in Sage.
  2. some packages might need a change in environment variables (e.g. ccache) and that can only be done if sage-env is re-run after installing the package.

comment:23 Changed 5 years ago by jdemeyer

  • Branch set to u/jdemeyer/install_package___is_obsolete

comment:24 Changed 5 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Commit set to ec6c23a895a72e46b4920ee08a5de73a3d269791
  • Milestone changed from sage-6.7 to sage-6.9
  • Status changed from new to needs_review
  • Type changed from defect to enhancement

New commits:

ec6c23aRemove install_package(), document sage -i

comment:25 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:26 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:27 Changed 5 years ago by jdemeyer

  • Description modified (diff)

comment:28 follow-ups: Changed 5 years ago by jhpalmieri

Channeling Nathann: regarding changes like sage -i cryptominisat && make, do we need to say something about being in SAGE_ROOT first?

This is no longer a stopgap, so in misc.package.install_package, should we be returning some other error message? Maybe NotImplementedError? I agree that install_package('blah') won't work much of the time for the reasons you give, so a deprecation isn't the right course of action, but a stopgap implies that the situation is temporary and is going to be fixed. This situation may be temporary, but only in the sense that install_package() may be completely deleted eventually. Also, with a stopgap, you only get the message the first time you run it. If we raise an error, you will get it every time, which might be better.

I'm not sure if the changes to trac.py belong here or on a separate ticket. I don't care that much, though.

Also, please make the following change (or something similar):

  • src/sage/misc/package.py

    diff --git a/src/sage/misc/package.py b/src/sage/misc/package.py
    index de4d206..9d727d9 100644
    a b components: 
    1919Use the :func:``optional_packages`` command to list all
    2020optional packages available on the central Sage server.
    2121
    22 Actually installing the packages should be done via the Sage command
     22Actually installing the packages should be done via the command
    2323line, using the following commands:
    2424
    2525- ``sage -i PACKAGE_NAME`` -- install the given package

Also, the sentences at the top of that file describing Sage packages are outdated and should probably just be replaced with a pointer to the Developer's Guide. If you are willing to do that, too, that would be nice.

comment:29 in reply to: ↑ 28 Changed 5 years ago by jdemeyer

Replying to jhpalmieri:

Channeling Nathann: regarding changes like sage -i cryptominisat && make, do we need to say something about being in SAGE_ROOT first?

Good point. I'll replace this by sage -i cryptominisat sagelib which doesn't have the SAGE_ROOT problem.

comment:30 Changed 5 years ago by git

  • Commit changed from ec6c23a895a72e46b4920ee08a5de73a3d269791 to cbf315d4d1623a2200937f4556388de5214566ce

Branch pushed to git repo; I updated commit sha1. New commits:

cbf315dReplace "make" by "sage -i sagelib"

comment:31 in reply to: ↑ 28 Changed 5 years ago by jdemeyer

Replying to jhpalmieri:

replaced with a pointer to the Developer's Guide.

I don't think it's technically possible to have an actual link to the Developer's Guide, but I will change the wording.

comment:32 in reply to: ↑ 28 Changed 5 years ago by jdemeyer

Replying to jhpalmieri:

I'm not sure if the changes to trac.py belong here or on a separate ticket.

It's really just a deprecation that I added. Pretty innocent.

comment:33 Changed 5 years ago by git

  • Commit changed from cbf315d4d1623a2200937f4556388de5214566ce to da03be13988e41fc483ee2001654a55d4e45ee95

Branch pushed to git repo; I updated commit sha1. New commits:

da03be1Replace install_package() by installed_packages()

comment:34 Changed 5 years ago by jdemeyer

John, I think I made all the changes you proposed.

comment:35 follow-up: Changed 5 years ago by jhpalmieri

Okay, looks good. What do we do about the files

src/doc/en/thematic_tutorials/numerical_sage/installation_linux.rst
src/doc/en/thematic_tutorials/numerical_sage/installation_osx.rst
src/doc/en/thematic_tutorials/numerical_sage/mpi4py.rst

Maybe I can handle the last one:

  • src/doc/en/thematic_tutorials/numerical_sage/mpi4py.rst

    diff --git a/src/doc/en/thematic_tutorials/numerical_sage/mpi4py.rst b/src/doc/en/thematic_tutorials/numerical_sage/mpi4py.rst
    index 93afc85..ef129b1 100644
    a b MPI which stands for message passing interface is a common library 
    55for parallel programming. There is a package mpi4py that builds on
    66the top of mpi, and lets arbitrary python objects be passed between
    77different processes. These packages are not part of the default
    8 sage install. To install them do
    9 
    10 .. skip
    11 
    12 ::
    13 
    14     sage: optional_packages()
    15 
    16 Find the package name openmpi-\* and mpi4py-\*and do
    17 
    18 .. skip
     8sage install. To install them, from a shell prompt, run
    199
    2010::
    2111
    22     sage: install_package('openmpi-*')
    23     sage: install_package('mpi4py-*')
     12    sage -i openmpi mpi4py
    2413
    2514Note that openmpi takes a while to compile (15-20 minutes or so).
    2615Openmpi can be run on a cluster, however this requires some set up

But the other two files refer to packages which are now archived (vtk) or are archived and almost certainly a bad idea to install (python-2.5.1-framework). We can change each instance of install_package to sage -i ... anyway; I'm not sure what else to suggest.

comment:36 in reply to: ↑ 35 Changed 5 years ago by jdemeyer

Replying to jhpalmieri:

Okay, looks good. What do we do about the files

src/doc/en/thematic_tutorials/numerical_sage/installation_linux.rst
src/doc/en/thematic_tutorials/numerical_sage/installation_osx.rst
src/doc/en/thematic_tutorials/numerical_sage/mpi4py.rst

I know.... it's difficult. Since I see no sensible way to fix those files, I just left them alone.

The problem is that those files have a laundry list of old-style packages which are no longer supported. I see no sensible way of updating them, there is no point to replace install_package("foo") by ./sage -i foo if the latter doesn't actually work.

comment:37 follow-up: Changed 5 years ago by jhpalmieri

How about this:

  • src/doc/en/thematic_tutorials/numerical_sage/installation_linux.rst

    diff --git a/src/doc/en/thematic_tutorials/numerical_sage/installation_linux.rst b/src/doc/en/thematic_tutorials/numerical_sage/installation_linux.rst
    index 16df05f..01ee37f 100644
    a b  
    11Installing Visualization Tools on Linux
    22=======================================
    33
     4.. warning::
     5
     6   The following instructions are outdated (2015-Sep).
     7
    48This section assumes you are running linux. You may need
    59administrator rights to complete this section. First try
    610
  • src/doc/en/thematic_tutorials/numerical_sage/installation_osx.rst

    diff --git a/src/doc/en/thematic_tutorials/numerical_sage/installation_osx.rst b/src/doc/en/thematic_tutorials/numerical_sage/installation_osx.rst
    index 05c27e8..2c6cec8 100644
    a b  
    11Installing Visualization Software on OS X
    22=========================================
    33
     4
     5.. warning::
     6
     7   The following instructions are outdated (2015-Sep).
     8
    49The first thing we need to do is rebuild Python to use OSX's
    510frameworks, so that it can create graphical windows. To do this
    611first from the terminal do
  • src/doc/en/thematic_tutorials/numerical_sage/mpi4py.rst

    diff --git a/src/doc/en/thematic_tutorials/numerical_sage/mpi4py.rst b/src/doc/en/thematic_tutorials/numerical_sage/mpi4py.rst
    index 93afc85..98d0edb 100644
    a b  
    11mpi4py
    22======
    33
     4
     5.. warning::
     6
     7   The following instructions are outdated (2015-Sep).
     8
    49MPI which stands for message passing interface is a common library
    510for parallel programming. There is a package mpi4py that builds on
    611the top of mpi, and lets arbitrary python objects be passed between

We can also add a note to contact sage-support for help. What do you think?

comment:38 in reply to: ↑ 37 Changed 5 years ago by jdemeyer

Replying to jhpalmieri:

We can also add a note to contact sage-support for help. What do you think?

In order to move forward with this ticket, I made it a separate ticket: #19198

comment:39 Changed 5 years ago by jhpalmieri

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

comment:40 Changed 5 years ago by vbraun

  • Branch changed from u/jdemeyer/install_package___is_obsolete to da03be13988e41fc483ee2001654a55d4e45ee95
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.