Old installed version of Cython is used

[jdemeyer]

On a machine which had an older version of Cython installed but which was upgraded recently after #20218 was merged:

$ ls -ld local/lib/python/site-packages/Cython*
drwxrwxr-x 12 jdemeyer jdemeyer 4096 Sep  6 21:23 local/lib/python/site-packages/Cython
drwxrwxr-x  5 jdemeyer jdemeyer 4096 Sep  6 21:22 local/lib/python/site-packages/Cython-0.23.3-py2.7-linux-i686.egg
drwxrwxr-x  2 jdemeyer jdemeyer 4096 Sep  6 21:14 local/lib/python/site-packages/Cython-0.24.1-py2.7.egg-info

Despite Cython-0.24.1 being available, the older version 0.23.3 is still used by default. This causes breakage when building the Sage library.

>>> import Cython
>>> Cython.__version__
'0.23.3'
>>> import sys
>>> [x for x in sys.path if 'Cython' in x]
['/home/jdemeyer/sage-git/local/lib/python2.7/site-packages/Cython-0.23.3-py2.7-linux-i686.egg']

The problem is not limited to Cython, the same happens for other packages too.

This is a blocker ticket since it breaks upgrades from older Sage versions.

Steps to reproduce:

1. ./sage --sh -c 'easy_install cython==0.23.3' # Install an older version of Cython.

2. ./sage -f cython # Rebuild the Cython in Sage. This should uninstall the old version but it doesn't.

Note to the release manager: ideally, this would be merged together with #21552 since both tickets trigger recompilation of all Python packages.

CC: @embray

Component: build

Author: Erik Bray, Jeroen Demeyer

Branch: 8fd8273

Reviewer: Jeroen Demeyer, Erik Bray, Matthias Koeppe

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

[embray]

comment:5

Wow, I see the issue. When you use pip with the --ignore-installed option it also does not uninstall existing versions of the package (at the very least if it's an old .egg). I would say that's undocumented behavior.

[embray]

comment:6

I think the easiest thing to do for now would be to make PIP_INSTALL point to a little wrapper function that also forcibly calls pip uninstall. That will also make ./sage -f work properly since it will force reinstall of the package even if the same version was already installed.

[jdemeyer]

comment:7

Replying to @embray:

I think the easiest thing to do for now would be to make PIP_INSTALL point to a little wrapper function that also forcibly calls pip uninstall.

That is not entirely trivial because you need to figure out which package to uninstall. You cannot do pip uninstall ., you need something like pip uninstall $PKGNAME.

Other than that, I like the idea. We could probably drop the --ignore-installed --no-deps options too.

[embray]

comment:8

Replying to @jdemeyer:

Replying to @embray:

I think the easiest thing to do for now would be to make PIP_INSTALL point to a little wrapper function that also forcibly calls pip uninstall.

That is not entirely trivial because you need to figure out which package to uninstall. You cannot do pip uninstall ., you need something like pip uninstall $PKGNAME.

Fair point. A broader idea I had is that as many of the spkg-installs (but not all!) have sections for uninstalling previous installations, that step should maybe be broken out to a separate spkg-uninstall script. Another step in making the packages a little more uniform in how they are managed. But my hope was to find a simple way to address this specific issue now.

I'm sort of of the mind that pip uninstall . should work. If there's a local setup.py it should use that to figure out the package's name, and then uninstall that.

Other than that, I like the idea. We could probably drop the --ignore-installed --no-deps options too.

I don't know what you mean here. If --ignore-installed is dropped then it's a moot point. pip will uninstall the previous version of the package without that flag. It seems that --ignore-installed is just very literal in its intent. It really means "pay no attention whatsoever to what I already have installed".

[embray]

comment:9

I think what pip really needs here is an option to explicitly ensure that conflicting packages are uninstalled, even if I said --ignore-installed.

[embray]

Branch: u/embray/sage-pip-install

[embray]

comment:10

Here's an initial stab at a fix. Adds a new sage-pip-install script to wrap pip, since otherwise this got too unwieldy. It just ensures that any previous installations of the package are thoroughly uninstalled before the actual pip install call.

In the process I found out that Cython's setup.py is badly behaved--./setup.py --name should really only print the package's name (if it prints anything else it should be to stderr). This is easy enough to work around, but I wonder what other packages make this mistake, and I'm testing for that now...

---

New commits:

015197a

Add a sage-pip-install wrapper for pip which handles uninstallation better.

[embray]

Commit: 015197a

[embray]

Author: Erik Bray

[sagetrac-git]

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

ea52ac9

Make sure to consume additional flags passed to the PIP_INSTALL command

[sagetrac-git]

Changed commit from 015197a to ea52ac9

[sagetrac-git]

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

a6e0538

Ensure stderr is captured for repeat uninstalls

[sagetrac-git]

Changed commit from ea52ac9 to a6e0538

[jdemeyer]

comment:13

#!/bin/bash should be #!/usr/bin/env bash

[jdemeyer]

comment:14

Replying to @embray:

I don't know what you mean here. If --ignore-installed is dropped then it's a moot point. pip will uninstall the previous version of the package without that flag.

The whole point of adding --ignored-installed in #20218 was to fix cases of existing but broken installations. Those would now be uninstalled anyway, so you no longer need the --ignore-installed flag. And once you do that, you can drop --no-deps to enable dependency checking.

[jdemeyer]

comment:15

Replying to @embray:

In the process I found out that Cython's setup.py is badly behaved--./setup.py --name should really only print the package's name

Is Cython the only misbehaving package? If so, I would rather add cython/cython#1455 in order to simplify sage-pip-install.

[embray]

comment:16

Replying to @jdemeyer:

#!/bin/bash should be #!/usr/bin/env bash

Sure, will do.

[embray]

comment:17

Replying to @jdemeyer:

Replying to @embray:

I don't know what you mean here. If --ignore-installed is dropped then it's a moot point. pip will uninstall the previous version of the package without that flag.

The whole point of adding --ignored-installed in #20218 was to fix cases of existing but broken installations. Those would now be uninstalled anyway, so you no longer need the --ignore-installed flag. And once you do that, you can drop --no-deps to enable dependency checking.

That makes sense. I think for the purposes of this ticket we shouldn't mess with it though--it's already working as is (minus this issue). I'd rather get the uninstallation fixed, and then worry about further tweaking the install options.

I'm also going to write up something about this experience for the pip mailing list. As pip is now the de-facto installer for Python packages it ought to be easier for system integrators to control it a bit (though I still maintain that as system integrators dependency management is our job, not pip's).

[embray]

comment:18

Replying to @jdemeyer:

Replying to @embray:

In the process I found out that Cython's setup.py is badly behaved--./setup.py --name should really only print the package's name

Is Cython the only misbehaving package? If so, I would rather add cython/cython#1455 in order to simplify sage-pip-install.

There are a few others I've found:

• pari_jupyter
• widgetsnbextension
• giacpy
• matplotlib

I've already opened an issue for matplotlib, though I don't have a patch. I think the check is good to leave in even if these are fixed because you never know when another package is going to be added, or updated in such a way that it breaks this.

[jdemeyer]

comment:19

I see. For pari_jupyter, I need Cython 0.25 to fix that: sagemath/pari-jupyter#1

[jdemeyer]

comment:47

Replying to @embray:

See above.

Can you give a little more detail?

[jdemeyer]

comment:48

In other words: what would you like me to do concretely?

[embray]

comment:49

Remove any changes to spkg-installs.

[sagetrac-git]

Changed commit from e351c98 to 8fd8273

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

29957fa	Various fixes and simplifications to sage-pip-install
783cd7d	Re-install all pip-installed packages
8fd8273	Fix various dependencies

[jdemeyer]

comment:51

Not fully tested yet, but this reverts all changes to spkg-install, except for the pip package.

[embray]

comment:52

Looks good to me.

[vbraun]

comment:53

Reviewer name...

[mkoeppe]

comment:55

The patch bumps the version numbers of dozens of packages without making changes to them. This seems unnecessary.

[mkoeppe]

Reviewer: Jeroen Demeyer, Erik Bray, Matthias Koeppe

[jdemeyer]

comment:57

Replying to @mkoeppe:

The patch bumps the version numbers of dozens of packages without making changes to them.

This patch does make implicit changes to all those spkg-install scripts because it changes $PIP_INSTALL. See [comment:21] and following.

In any case, if this is merged with #21552, then all Python packages need to be rebuilt anyway so it wouldn't matter.

[embray]

comment:58

I don't personally think it's terribly important to bump all these package versions since it doesn't actually change all of them--for example packages that did not use setuptools (and thus are not installed as .egg directories) really aren't changed at all. For those that are changed, this changes some small details about how they are installed, but nothing about how they run.

That said, strictly speaking, Jeroen is right that this does change runtime behavior since it will change what sys.path looks like. Also nice to bump the version numbers to force all the .egg installs to be removed.

[vbraun]

Changed branch from u/jdemeyer/sage-pip-install to 8fd8273

[mkoeppe]

comment:60

OK, thanks for the explanation.

[mkoeppe]

Changed commit from 8fd8273 to none