Use pip to install Python dependencies

[embray]

New tarball: https://pypi.python.org/packages/26/34/0f3a5efac31f27fabce64645f8c609de9d925fe2915304d1a40f544cff0e/appnope-0.1.0.tar.gz

---

Work has already been done--for example in #19187--on using pip for sage -i where appropriate. However, a normal build of sage does not use pip for installing any Python spkgs; rather, they run python setup.py install which until recently has always been the way to install Python packages.

I can suggest several reasons why pip should be used instead of this method. To be clear--as many people think of pip as a tool for installing packages from the internet--this is not strictly the case. pip can be used to install an already downloaded tarball, or even straight from a source tree unpacked from a tarball or checked out from VCS. The typical way to do this is to change directories to the directory containing the setup.py script and running pip install .. This still ultimately calls setup.py install, among other setup.py commands, but with the right incantations to perform a pip-style flat install. Now, why is this preferable?

1. Future-compatibility. One of the goals that the Python packaging community has been working toward over the past several years has been to decouple Python package installation from build system. Traditionally setup.py provided both a build program and an installation program. At best it will still stick around as a build program, but its use for installation is being discouraged. There is now finally a PEP (http://legacy.python.org/dev/peps/pep-0516/), still very much in draft state, specifically concerning this decoupling. Under this new regime pip will not likely be the only software for installing Python packages, but the assumption can no longer be that there is a setup.py script at all.

2. pip avoids using easy_install. easy_install is an older installation program for Python packages that was bundled with setuptools--when a Python project uses setuptools in its setup.py, the setup.py install command invokes easy_install by default. The behavior of easy_install is to install the package as an egg archive/directory. easy_install has several disadvantages, of which this is just one, but there are a couple reasons that this alone is a problem:

   a. Every .egg requires a sys.path entry, hence difficult to debug paths that look like this:

>>> pprint(sys.path)
['',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/setuptools-18.1-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/Cython-0.23.3-py2.7-linux-x86_64.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/MarkupSafe-0.23-py2.7-linux-x86_64.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/Jinja2-2.7.3-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/six-1.10.0-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/Pygments-2.0.2-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/Sphinx-1.2.2-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/zope.interface-4.1.3-py2.7-linux-x86_64.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/Twisted-15.5.0-py2.7-linux-x86_64.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/pytz-2013b0-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/Babel-2.1.1-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/Werkzeug-0.11.2-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/speaklater-1.3-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/python_openid-2.2.5-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/itsdangerous-0.24-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/Flask-0.10.1-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/Flask_Silk-0.2-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/Flask_AutoIndex-0.5-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/Flask_Babel-0.9-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/Flask_OpenID-1.2.5-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/Flask_OldSessions-0.10-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/webassets-0.11.1-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/sagenb-0.11.6.1-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/backports.ssl_match_hostname-3.4.0.2-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/certifi-14.5.14-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/tornado-4.1-py2.7-linux-x86_64.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/setuptools_scm-1.7.0-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/path.py-7.1-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/pickleshare-0.5-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/simplegeneric-0.8.1-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/decorator-4.0.6-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/networkx-1.10-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/python_dateutil-2.2-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/cycler-0.9.0-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/matplotlib-1.5.0-py2.7-linux-x86_64.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/jsonschema-2.4.0-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/mistune-0.5.1-py2.7-linux-x86_64.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/pip-6.1.1-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/pkgconfig-1.1.0-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/singledispatch-3.4.0.3-py2.7.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages/rpy2-2.7.5-py2.7-linux-x86_64.egg',
 '/home/iguananaut/src/sagemath/sage/local/lib/python',
 '/home/iguananaut/src/sagemath/sage/local/lib/python/site_packages',
 '/home/iguananaut/src/sagemath/sage/local/lib/python27.zip',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/plat-linux2',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/lib-tk',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/lib-old',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/lib-dynload',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages']

 With the flat install style used by `pip`, and now more generally favored, the `sys.path`, with no eggs, would be simply:

>>> pprint(sys.path)
['',
 '/home/iguananaut/src/sagemath/sage/local/lib/python',
 '/home/iguananaut/src/sagemath/sage/local/lib/python27.zip',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/plat-linux2',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/lib-tk',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/lib-old',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/lib-dynload',
 '/home/iguananaut/src/sagemath/sage/local/lib/python2.7/site-packages']

 which is certainly easier to read and reason about.

b. The egg import mechanism requires writing to a common file during egg installation--easy-install.pth. This causes problems in parallel installations, requiring patches to setuptools. The flat install style used by pip has no such problems--there is a no single common file written to by pip during installation of packages (other than maybe the log file, which is only for debugging purposes).

As I explained in this comment, when a Python package is installed with pip, pip actually slips setuptools into the installation process, whether the package uses it explicitly or not. There are reasons for this not worth going into now. This is no problem for the vast majority of cases. The only exception I know of is that setuptools doesn't handle the data_files option well when --prefix is changed. As far as I can tell this is only a problem for sage itself and possibly for sagetex, but these don't even use setuptools in the first place, so until we have a workaround for that it does no harm if we keep using setup.py install for those packages, if nothing else works.

Most of the rest of the Python packages included by default in a Sage build (I haven't checked all of -optional yet) already use setuptools. I checked the setup.py of all the rest that don't explicitly use setuptools--these include docutils, mpmath, pexpect, ptyprocess, Pycrypto, pyparsing, pyzmq, scipy, and all packages from the Jupyter team (ip*, jupyter*, np*, notebook, traitlets).

Most of these packages are known by me (from personal experience) to work fine with pip. Looking at the setup.py scripts of the rest of them, most of them are fairly trivial and should work fine.

TL;DR, with the exceptions of sage and sagetex (pending work like #20108), as well as pip itself the spkg-install scripts for Python packages should change setup.py install commands to pip install .. Their dependencies would also have to be updated to require pip to be installed first.

At the same time the "uninstall" commands in those scripts can be changed to use pip uninstall.

Upstream: None of the above - read trac for reasoning.

CC: @jdemeyer @vbraun

Component: build

Author: Erik Bray, Jeroen Demeyer

Branch: f6859c3

Reviewer: Jeroen Demeyer, Volker Braun

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

[embray]

comment:3

To be clear, I'm working on a patch for this.

[sagetrac-tmonteil]

comment:4

IIRC, there was an issue with using pip when python was not compiled with openssl, even for building wihthout downloading. I am not sure that it is still the case, but this particular case should be taken into account since openssl (libssl-dev) is not necessarily available on the system (and cannot be made standard Sage package because of some licensing issues).

[jdemeyer]

Author: Erik Bray

[vbraun]

comment:6

Its exactly as Thierry says. Hopefully it won't be long until the announced switch of OpenSSL to a GPL-compatible license happens.

[embray]

comment:7

I've attached a branch containing the beginnings of this work. I've already put it through several rounds of testing locally including installing, reinstalling packages, uninstalling and reinstalling, etc.

But I'm interested to find out what happens in the patchbot.

I haven't addressed the SSL issue yet but I will. I wanted to start getting some test / review feedback on the main work in the meantime.

---

New commits:

9322696

Use pip to install most Python-based dependencies; see https://github.com/sagemath/sage/issues/20218

[embray]

Commit: 9322696

[embray]

Branch: u/embray/pip-install

[embray]

comment:8

Looks like I need to rebase--that's what I get for starting a patch and then not getting around to finishing it until weeks later :)

[sagetrac-git]

Changed commit from 9322696 to 7447ef2

[sagetrac-git]

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

7447ef2

Use pip to install most Python-based dependencies; see https://github.com/sagemath/sage/issues/20218

[jdemeyer]

comment:10

This isn't correct:

Sage's Python unconditionally removes . from sys.path
which can break several packages' installation

Sage's Python only removes . from sys.path if having . in sys.path might be insecure. If packages break because of this, those packages should be fixed instead of working around the security check.

[jdemeyer]

comment:11

In case you think that this whole security-thing is exaggerated: I can take over the Unix account of anybody who runs the Python test-suite on a computer where I have an account. When I reported this bug to Python, they didn't really care. When I reported this to IPython (which had a similar bug), they fixed their test-suite.

[embray]

comment:12

As someone who has actually surreptitiously taken over Unix accounts, I'm not too impressed by that particular issue. But that's beside the point--I'm not arguing with that right now.

I'll have to more closely look at what that patch does, but I'll change that comment to not supply inaccurate information. No packages are broken here though. The reason this is needed is that pip copies files packages to a tmp dir to build them. I'm actually surprised this hasn't come up for sage -pip, though it's only an issue for packages that import themselves in their setup.py so maybe it just coincidentally hasn't come up.

[jdemeyer]

comment:13

Replying to @embray:

The reason this is needed is that pip copies files packages to a tmp dir to build them.

It doesn't hurt if it's in a private directory inside /tmp, say /tmp/my-pip-directory where /tmp/my-pip-directory has reasonable permissions. So I don't fully understand the problem.

[embray]

comment:14

Indeed it should be doing that. It's not like it's putting the package directly under /tmp. It belongs to me and my user-specific group. So I'm not sure why there would be a problem and yet I am getting the warning about '.' being removed from sys.path and the import subsequently fails.

[jdemeyer]

comment:15

Replying to @embray:

Indeed it should be doing that. It's not like it's putting the package directly under /tmp. It belongs to me and my user-specific group. So I'm not sure why there would be a problem and yet I am getting the warning about '.' being removed from sys.path and the import subsequently fails.

What is the exact warning message?

[embray]

comment:16

Nevermind--I traced this to a bug in pip, combined with a possible minor defect in sage-uncompress-spkg (or Python's tarfile module depending on your perspective), along with strange packaging for pyparsing, which is the package where this came up originally.

1. It's true that pip creates a temp dir (under /tmp, typically) in which to build a package. This directory is created with mkdtemp and normally has safe permissions on it. However, when installing a package from a source directory (a la pip install .), pip then deletes that temp directory, and copies the source directory using shutil.copytree to the same path as the just-deleted temp dir. There are a couple problems with this:
   a) Deleting and recreating the temp dir of the same name involves a possible race condition.
   b) More importantly, while mkdtemp will create a directory with safe permissions, shutil.copytree copies the original permissions of the source files, which may be arbitrary. Instead, pip should be copying the contents of the source directory into the directory created with mkdtemp.

2. sage-uncompress-spkg just calls TarFile.extractall() which preserves the original permissions of all files/directories in the tar file, and doesn't provide a clear way to enable or disable this. This is different from the GNU tar command which by default (for normal users) applies the user's umask to all extracted files, which is generally safer depending on the user's umask. It would probably be best if sage-uncompress-spkg did this as well.

3. The top-level directory in pyparsing's source package has permissions set to 777. Of all the upstream source tarballs I have pyparsing is the only one like this. All the rest have 775 or 755.

Result: pyparsing's source is extracted with permissions 777 and is then copied into a directory in /tmp with unrestricted access, from which pip installs it. My previous comments aside these are all bad and all three should be addressed.

[jdemeyer]

comment:17

4. python setup.py sdist doesn't actually try to create a tarball with sane permissions in the first place.

[embray]

comment:18

python setup.py sdist doesn't actually try to create a tarball with sane permissions in the first place.

Agreed.... is what I was about to write. But I tested it and ./setup.py sdist does actually use sane permissions. So either the maintainer of pyparsing is not using ./setup.py sdist or is using a buggy version of Python or something.

[embray]

comment:19

See #20481 for an attempt to address point 2 above, as it was the lowest-hanging fruit I think. The issue in pip will be easy to fix too but will have to get it accepted upstream, etc, etc.

[jdemeyer]

comment:20

Replying to @embray:

python setup.py sdist doesn't actually try to create a tarball with sane permissions in the first place.

Agreed.... is what I was about to write. But I tested it and ./setup.py sdist does actually use sane permissions.

I also tested it and ./setup.py sdist did not use sane permissions.

[embray]

comment:21

Huh. How did you test it? I just ran chmod 777 on the parent directory and all subdirectories, then ran ./setup.py sdist. The permissions in the resulting tarball at least had my umask applied:

$ umask -S
u=rwx,g=rwx,o=rx

It's the 'w' I'm mostly concerned about. How are you defining "sane"?

[jdemeyer]

comment:22

I made a project with the following files:

zzz.py:

print("zzz")

setup.py:

from setuptools import setup   # setuptools or distutils does not matter

setup(
    name="zzz",
    py_modules=["zzz"],
)

I changed the permissions of both these files to 0666 and then ran python setup.py sdist. The result:

drwxr-xr-x jdemeyer/jdemeyer 0 2016-04-21 17:38 zzz-0.0.0/
drwxr-xr-x jdemeyer/jdemeyer 0 2016-04-21 17:38 zzz-0.0.0/zzz.egg-info/
-rw-r--r-- jdemeyer/jdemeyer 1 2016-04-21 17:38 zzz-0.0.0/zzz.egg-info/dependency_links.txt
-rw-r--r-- jdemeyer/jdemeyer 177 2016-04-21 17:38 zzz-0.0.0/zzz.egg-info/PKG-INFO
-rw-r--r-- jdemeyer/jdemeyer 123 2016-04-21 17:38 zzz-0.0.0/zzz.egg-info/SOURCES.txt
-rw-r--r-- jdemeyer/jdemeyer   4 2016-04-21 17:38 zzz-0.0.0/zzz.egg-info/top_level.txt
-rw-r--r-- jdemeyer/jdemeyer 177 2016-04-21 17:38 zzz-0.0.0/PKG-INFO
-rw-rw-rw- jdemeyer/jdemeyer 113 2016-04-21 17:36 zzz-0.0.0/setup.py
-rw-r--r-- jdemeyer/jdemeyer  59 2016-04-21 17:38 zzz-0.0.0/setup.cfg
-rw-rw-rw- jdemeyer/jdemeyer  13 2016-04-21 17:28 zzz-0.0.0/zzz.py

[sagetrac-git]

Changed commit from f5c0b16 to 55f9e9d

[vbraun]

comment:147

sage -t --long src/sage/misc/package.py
**********************************************************************
File "src/sage/misc/package.py", line 24, in sage.misc.package
Failed example:
    sorted(pkgs.keys())
Expected:
    ['4ti2',
     'alabaster',
     'arb',
     ...
     'zlib',
     'zn_poly',
     'zope_interface']
Got:
    ['4ti2',
     'alabaster',
     'appnope',
     'arb',

[embray]

comment:148

Heh. What an insidious test :)

[jdemeyer]

comment:149

That test doesn't exist in this branch... all tests pass on my machine. It must have been added recently.

[sagetrac-git]

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

7f365e3

Merge tag '7.4.beta2' into t/20218/pip-install

[sagetrac-git]

Changed commit from 55f9e9d to 7f365e3

[sagetrac-git]

Changed commit from 7f365e3 to f6859c3

[sagetrac-git]

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

f6859c3

Mark package tests as random

[jdemeyer]

comment:152

That test is only used for documentation purposes, it makes no sense to hardcode the list of packages in a test. Therefore, I mark the test # random.

[jdemeyer]

comment:153

Please review this trivial fix...

[vbraun]

Changed branch from u/jdemeyer/pip-install to f6859c3

[jdemeyer]

comment:156

Replying to @embray:

pip will uninstall them, mostly.

This doesn't happen, leading to breakage like #21441.

[jdemeyer]

Changed commit from f6859c3 to none

[embray]

comment:157

It does happen, normally. I don't know why it didn't for you. You might have already had 2 Cythons installed and maybe it only uninstalled one.