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

install_package() is obsolete #16759

Closed
jdemeyer opened this issue Aug 4, 2014 · 45 comments
Closed

install_package() is obsolete #16759

jdemeyer opened this issue Aug 4, 2014 · 45 comments

Comments

@jdemeyer
Copy link

jdemeyer commented Aug 4, 2014

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).

CC: @williamstein

Component: misc

Stopgaps: #16760

Author: Jeroen Demeyer

Branch/Commit: da03be1

Reviewer: John Palmieri

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

@jdemeyer jdemeyer added this to the sage-6.3 milestone Aug 4, 2014
@jhpalmieri
Copy link
Member

comment:1

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/.

@jdemeyer
Copy link
Author

jdemeyer commented Aug 4, 2014

comment:2

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).

@jdemeyer
Copy link
Author

jdemeyer commented Aug 4, 2014

comment:3

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

@kcrisman
Copy link
Member

kcrisman commented Aug 4, 2014

comment:4

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.

@jhpalmieri
Copy link
Member

comment:5

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

@jhpalmieri
Copy link
Member

Stopgaps: #16760

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Apr 30, 2015

comment:7

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,

@jhpalmieri
Copy link
Member

comment:8

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?

@jhpalmieri
Copy link
Member

comment:9

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

diff --git a/src/bin/sage-fix-pkg-checksums b/src/bin/sage-fix-pkg-checksums
index 65542b9..b42463d 100755
--- a/src/bin/sage-fix-pkg-checksums
+++ b/src/bin/sage-fix-pkg-checksums
@@ -11,7 +11,7 @@ fi
 for upstream in "$@"
 do
     tarball=`basename "$upstream"`
-    pkg_name=${tarball%%-*}
+    pkg_name=`echo ${tarball%%-*} | tr '[:upper:]' '[:lower:]'`
     pkg_compression=${tarball#*.tar}   # gz or bz2
     if [ -d "$SAGE_ROOT/build/pkgs/$pkg_name" ]; then
         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?

@jhpalmieri
Copy link
Member

comment:10

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

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Apr 30, 2015

comment:11

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?

@jhpalmieri
Copy link
Member

comment:12

Replying to @EmmanuelCharpentier:

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."

@jhpalmieri
Copy link
Member

comment:13

See #18344.

@nexttime
Copy link
Mannequin

nexttime mannequin commented May 1, 2015

comment:14

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.)

@jhpalmieri
Copy link
Member

comment:15

Replying to @nexttime:

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?

@nexttime
Copy link
Mannequin

nexttime mannequin commented May 1, 2015

comment:16

Replying to @jhpalmieri:

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

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

@nexttime
Copy link
Mannequin

nexttime mannequin commented May 4, 2015

comment:17

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...

@nexttime
Copy link
Mannequin

nexttime mannequin commented May 9, 2015

Dependencies: #15642

@nexttime nexttime mannequin modified the milestones: sage-6.4, sage-6.7 May 9, 2015
@nexttime
Copy link
Mannequin

nexttime mannequin commented May 12, 2015

Changed dependencies from #15642 to #15642, #18407

@nexttime

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link
Author

jdemeyer commented Sep 9, 2015

comment:22

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.

@jdemeyer
Copy link
Author

jdemeyer commented Sep 9, 2015

Changed dependencies from #15642, #18407 to none

@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jdemeyer

This comment has been minimized.

@jhpalmieri
Copy link
Member

comment:28

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):

diff --git a/src/sage/misc/package.py b/src/sage/misc/package.py
index de4d206..9d727d9 100644
--- a/src/sage/misc/package.py
+++ b/src/sage/misc/package.py
@@ -19,7 +19,7 @@ components:
 Use the :func:``optional_packages`` command to list all
 optional packages available on the central Sage server.
 
-Actually installing the packages should be done via the Sage command
+Actually installing the packages should be done via the command
 line, using the following commands:
 
 - ``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.

@jdemeyer
Copy link
Author

comment:29

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2015

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

cbf315dReplace "make" by "sage -i sagelib"

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2015

Changed commit from ec6c23a to cbf315d

@jdemeyer
Copy link
Author

comment:31

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.

@jdemeyer
Copy link
Author

comment:32

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.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2015

Changed commit from cbf315d to da03be1

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Sep 12, 2015

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

da03be1Replace install_package() by installed_packages()

@jdemeyer
Copy link
Author

comment:34

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

@jhpalmieri
Copy link
Member

comment:35

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:

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/src/doc/en/thematic_tutorials/numerical_sage/mpi4py.rst
+++ b/src/doc/en/thematic_tutorials/numerical_sage/mpi4py.rst
@@ -5,22 +5,11 @@ MPI which stands for message passing interface is a common library
 for parallel programming. There is a package mpi4py that builds on
 the top of mpi, and lets arbitrary python objects be passed between
 different processes. These packages are not part of the default
-sage install. To install them do
-
-.. skip
-
-::
-
-    sage: optional_packages()
-
-Find the package name openmpi-\* and mpi4py-\*and do
-
-.. skip
+sage install. To install them, from a shell prompt, run
 
 ::
 
-    sage: install_package('openmpi-*')
-    sage: install_package('mpi4py-*')
+    sage -i openmpi mpi4py
 
 Note that openmpi takes a while to compile (15-20 minutes or so).
 Openmpi 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.

@jdemeyer
Copy link
Author

comment:36

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.

@jhpalmieri
Copy link
Member

comment:37

How about this:

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/src/doc/en/thematic_tutorials/numerical_sage/installation_linux.rst
+++ b/src/doc/en/thematic_tutorials/numerical_sage/installation_linux.rst
@@ -1,6 +1,10 @@
 Installing Visualization Tools on Linux
 =======================================
 
+.. warning::
+
+   The following instructions are outdated (2015-Sep).
+
 This section assumes you are running linux. You may need
 administrator rights to complete this section. First try
 
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/src/doc/en/thematic_tutorials/numerical_sage/installation_osx.rst
+++ b/src/doc/en/thematic_tutorials/numerical_sage/installation_osx.rst
@@ -1,6 +1,11 @@
 Installing Visualization Software on OS X
 =========================================
 
+
+.. warning::
+
+   The following instructions are outdated (2015-Sep).
+
 The first thing we need to do is rebuild Python to use OSX's
 frameworks, so that it can create graphical windows. To do this
 first from the terminal do
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/src/doc/en/thematic_tutorials/numerical_sage/mpi4py.rst
+++ b/src/doc/en/thematic_tutorials/numerical_sage/mpi4py.rst
@@ -1,6 +1,11 @@
 mpi4py
 ======
 
+
+.. warning::
+
+   The following instructions are outdated (2015-Sep).
+
 MPI which stands for message passing interface is a common library
 for parallel programming. There is a package mpi4py that builds on
 the 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?

@jdemeyer
Copy link
Author

comment:38

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

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@vbraun
Copy link
Member

vbraun commented Sep 14, 2015

Changed branch from u/jdemeyer/install_package___is_obsolete to da03be1

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

4 participants