Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#20218 closed enhancement (fixed)

Use pip to install Python dependencies

Reported by: embray Owned by: embray
Priority: minor Milestone: sage-7.4
Component: build Keywords:
Cc: jdemeyer, vbraun Merged in:
Authors: Erik Bray, Jeroen Demeyer Reviewers: Jeroen Demeyer, Volker Braun
Report Upstream: None of the above - read trac for reasoning. Work issues:
Branch: f6859c3 (Commits) Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

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

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

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

Change History (157)

comment:1 Changed 4 years ago by jdemeyer

  • Cc jdemeyer added
  • Milestone set to sage-7.2

comment:2 Changed 4 years ago by jdemeyer

  • Summary changed from Use pip to install Python depedencies to Use pip to install Python dependencies

comment:3 Changed 4 years ago by embray

  • Owner changed from (none) to embray

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

comment:4 Changed 4 years ago by tmonteil

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

comment:5 Changed 4 years ago by jdemeyer

  • Authors set to Erik Bray

comment:6 Changed 4 years ago by vbraun

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

comment:7 Changed 4 years ago by embray

  • Branch set to u/embray/pip-install
  • Commit set to 93226961bf040faf37f080af04589c8d31d904a9
  • Status changed from new to needs_review

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:

9322696Use pip to install most Python-based dependencies; see http://trac.sagemath.org/ticket/20218

comment:8 Changed 4 years ago by embray

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

comment:9 Changed 4 years ago by git

  • Commit changed from 93226961bf040faf37f080af04589c8d31d904a9 to 7447ef236941c521fb730b0baa1a760d7d4aa15d

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

7447ef2Use pip to install most Python-based dependencies; see http://trac.sagemath.org/ticket/20218

comment:10 Changed 4 years ago by jdemeyer

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.

comment:11 Changed 4 years ago by jdemeyer

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.

comment:12 follow-up: Changed 4 years ago by embray

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.

Last edited 4 years ago by embray (previous) (diff)

comment:13 in reply to: ↑ 12 Changed 4 years ago by jdemeyer

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.

comment:14 follow-up: Changed 4 years ago by 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.

comment:15 in reply to: ↑ 14 Changed 4 years ago by jdemeyer

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?

comment:16 Changed 4 years ago by embray

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.

comment:17 Changed 4 years ago by jdemeyer

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

comment:18 follow-up: Changed 4 years ago by 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. So either the maintainer of pyparsing is not using ./setup.py sdist or is using a buggy version of Python or something.

comment:19 Changed 4 years ago by embray

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.

comment:20 in reply to: ↑ 18 Changed 4 years ago by jdemeyer

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.

comment:21 Changed 4 years ago by embray

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

comment:22 Changed 4 years ago by jdemeyer

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

comment:23 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Merge conflict.

comment:24 follow-up: Changed 4 years ago by embray

But what about directories? I'm more concerned with what permissions it sets for directories than for files.

comment:25 in reply to: ↑ 24 Changed 4 years ago by jdemeyer

Replying to embray:

But what about directories? I'm more concerned with what permissions it sets for directories than for files.

I propose to use the umask for everything which is in the tarfile: files, directories, ...

comment:26 Changed 4 years ago by git

  • Commit changed from 7447ef236941c521fb730b0baa1a760d7d4aa15d to 95dd93867b40cff33492190d08ebd22c7486dc88

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

95dd938Use pip to install most Python-based dependencies; see http://trac.sagemath.org/ticket/20218

comment:27 Changed 4 years ago by jdemeyer

Also note that #14840 added lots of new Python packages, which might be installed using pip.

comment:28 Changed 4 years ago by jdemeyer

Typo: insalled

comment:29 Changed 4 years ago by embray

Ah, that explains why I had to rebase, and why pytz started showing up. Thanks, I'll update those as well.

comment:30 Changed 4 years ago by jdemeyer

Something else to potentially worry about: how is the upgrade path from non-pip-installed packages to pip-installed packages? What if a package was installed before using either distutils or setuptools and it's now re-installed using pip?

comment:31 follow-up: Changed 4 years ago by embray

pip will uninstall them, mostly. If a package was installed without pip, it can't uninstall and scripts or data files that were not installed directly under site-packages/package_name(.egg-info)? because there's no install log like there is with pip.

That's no worse, and slightly better than the current situation where some packages were uninstalled first, but most weren't, and of those that were only did rm -rf site-packages/packagename. Some were clever enough to do rm -rf site-packages/packagename* but that could actually have unintended consequences. The former case is no good either because it leaves .egg-info directories, making it appear to tools that know how to process those like the package is "installed".

comment:32 Changed 4 years ago by git

  • Commit changed from 95dd93867b40cff33492190d08ebd22c7486dc88 to 1c30cbe51dd8ec2812198d34b914809b400b2efb

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

d2d0d74Ignore directory permissions when extracting tarballs.
4fe154aModified the custom TarFile to simply apply the user's umask to all files and directories, regardless what it may be--consistent with the default behavior of 'tar'.
1c30cbeUse pip to install most Python-based dependencies; see http://trac.sagemath.org/ticket/20218

comment:33 Changed 4 years ago by embray

  • Dependencies set to #20481
  • Report Upstream changed from N/A to Not yet reported upstream; Will do shortly.

Rebased to include latest from develop, and to include #20481. This allowed me to remove the env PYTHONPATH=. from all pip calls. The only package that currently was affected by this issue was pyparsing, and #20481 effectively works around it. There is a larger problem with pip that should be addressed, but it is no longer an immediate blocker for this issue.

comment:34 Changed 4 years ago by git

  • Commit changed from 1c30cbe51dd8ec2812198d34b914809b400b2efb to 31b0f5f7a6803c8545b73251d3515adda9531827

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

31b0f5fThis is no longer needed.

comment:35 Changed 4 years ago by git

  • Commit changed from 31b0f5f7a6803c8545b73251d3515adda9531827 to 60dfae1b8caf6ff2c300afb8a56068cc3b6c1ea3

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

f3b3cdfAdds a new sage-apply-patches script.
ba037a8Fix accidental duplication of patch_subdir.
b17fb08Forgot to change this to loop over the full list when I made $patches into a list.
cedf80aMathjax doesn't have a source directory to cd into, so uses a different default patch path from most packages.
2ce575eNone of these packages currently have patches to apply.
29bd199Add license notice.
dff74e5Use pip to install most Python-based dependencies; see http://trac.sagemath.org/ticket/20218
07accc7This is no longer needed.
60dfae1Adds patch to pip to allow it to be used without SSL support from Python

comment:36 Changed 4 years ago by embray

  • Dependencies changed from #20481 to #20481 #20692
  • Status changed from needs_work to needs_review

I rebased this on top of my branch for #20692 (which itself is based on a recent version of develop) so that I could use it in applying a new patch to pip. Other than that the dependency is loose, and can be extracted if #20692 is rejected for some reason or requires significant more work.

I have also applied the necessary patch to pip so that it no longer requires SSL to work (at least for installing local packages).

Last edited 4 years ago by embray (previous) (diff)

comment:37 Changed 4 years ago by git

  • Commit changed from 60dfae1b8caf6ff2c300afb8a56068cc3b6c1ea3 to 2fe1d357600e60d3422141ba5a7fb16efb3a691b

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

2fe1d35Update some packages that were added since the start of this branch to use pip.

comment:38 Changed 4 years ago by vbraun

  • Report Upstream changed from Not yet reported upstream; Will do shortly. to Fixed upstream, in a later stable release.

comment:39 Changed 4 years ago by embray

  • Report Upstream changed from Fixed upstream, in a later stable release. to None of the above - read trac for reasoning.

The SSL issue was already known upstream before I made this ticket. However, there is a different issue with pip that I described here and that should still be reported upstream. I haven't gotten around to it yet, but I will.

comment:40 Changed 4 years ago by jdemeyer

  • Dependencies changed from #20481 #20692 to #20692
  • Status changed from needs_review to needs_work

You need to increase the patchlevel of pip, otherwise the patch will not be applied.

comment:41 follow-up: Changed 4 years ago by jdemeyer

This branch does a lot of stuff. It's not a disaster, but I would preferred to see this as 3 tickets:

  1. Add the SSL patch to pip.
  2. Use pip install to install Python packages.
  3. Add dependencies to optional packages without dependency files.
Version 0, edited 4 years ago by jdemeyer (next)

comment:42 in reply to: ↑ 41 Changed 4 years ago by embray

Replying to jdemeyer:

This branch does a lot of stuff. It's not a disaster, but I would have preferred to see this as 3 tickets:

  1. Add the SSL patch to pip.
  2. Use pip install to install Python packages.
  3. Add dependencies to optional packages without dependency files.

Okay, that's fair. I can break this up.

Not exactly clear how to break up 2 and 3 though. If optional packages are going to be installed with pip, they have to have dependency files listing at least pip first. But if I do that before making them installed with pip then they would have an unnecessary dependency on pip.

Unless I made a separate ticket that only adds the dependency files with no pip, and then later adds the pip dependency as part of 2). But that would be a little more annoying.

comment:43 Changed 4 years ago by git

  • Commit changed from 2fe1d357600e60d3422141ba5a7fb16efb3a691b to a30f722df2a57f741222cb78b8f0f1086e7aa1e7

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

e3ddeddSome of the packages changed to use pip in 2fe1d357600e60d3422141ba5a7fb16efb3a691b needed pip added to their build deps.
a30f722Bump pip package patch level

comment:44 Changed 4 years ago by jdemeyer

Over at #20739, people are suggesting to use --no-binary :all:. I don't understand why, but maybe you do...

comment:45 Changed 4 years ago by embray

That shouldn't be relevant here since this has nothing to do with installing packages from PyPI (or any other PyPI-like Python package index). That's an instruction for controlling what type of package to download if there are multiple options.

comment:46 Changed 4 years ago by jdemeyer

Right, makes sense.

comment:47 Changed 4 years ago by git

  • Commit changed from a30f722df2a57f741222cb78b8f0f1086e7aa1e7 to 708244570a2053395a1213acc862022e61e8f25a

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

2fd9cf6Use pip to install most Python-based dependencies; see http://trac.sagemath.org/ticket/20218
985790aThis is no longer needed.
0a55518Update some packages that were added since the start of this branch to use pip.
7082445Some of the packages changed to use pip in 2fe1d357600e60d3422141ba5a7fb16efb3a691b needed pip added to their build deps.

comment:48 Changed 4 years ago by embray

  • Dependencies changed from #20692 to #20913
  • Status changed from needs_work to needs_review

Rebased, and simplified to no longer depend on #20692, and also removed the patch to pip for SSL, which has been broken out to #20913.

This doesn't have a strict dependence on #20913, but #20913 should still be merged first since otherwise this patch will break the sage build on platforms with no OpenSSL support.

comment:49 Changed 4 years ago by git

  • Commit changed from 708244570a2053395a1213acc862022e61e8f25a to f028f6fcd96f74b9836a2a88558386d4eaa3cbb9

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

756d94aUse pip to install most Python-based dependencies; see http://trac.sagemath.org/ticket/20218
70135d4This is no longer needed.
bd0e2caUpdate some packages that were added since the start of this branch to use pip.
f028f6fSome of the packages changed to use pip in 2fe1d357600e60d3422141ba5a7fb16efb3a691b needed pip added to their build deps.

comment:50 Changed 4 years ago by embray

Rebased on recent develop branch.

It would be great to get this moving--I just a build fail on trying to install flask_babel (though it could have been any number of packages) because they all use setuptools, and hence running ./setup.py install makes them use easy_install by default and check online for the package's dependencies even though it shouldn't have to.

This patch ensures that Python packages are installed without dependency on PyPI.

comment:51 Changed 4 years ago by jdemeyer

  • Milestone changed from sage-7.2 to sage-7.3
  • Status changed from needs_review to needs_work

Merge conflict.

comment:52 Changed 4 years ago by git

  • Commit changed from f028f6fcd96f74b9836a2a88558386d4eaa3cbb9 to 7880aca0575e38126b2e6f510c6b8202d10d768b

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

0676708Use pip to install most Python-based dependencies; see http://trac.sagemath.org/ticket/20218
cadba88This is no longer needed.
3e014d9Update some packages that were added since the start of this branch to use pip.
7880acaSome of the packages changed to use pip in 2fe1d357600e60d3422141ba5a7fb16efb3a691b needed pip added to their build deps.

comment:53 Changed 4 years ago by embray

  • Status changed from needs_work to needs_review

Rebased again.


New commits:

0676708Use pip to install most Python-based dependencies; see http://trac.sagemath.org/ticket/20218
cadba88This is no longer needed.
3e014d9Update some packages that were added since the start of this branch to use pip.
7880acaSome of the packages changed to use pip in 2fe1d357600e60d3422141ba5a7fb16efb3a691b needed pip added to their build deps.

comment:54 Changed 4 years ago by jdemeyer

  • Dependencies #20913 deleted

comment:55 Changed 4 years ago by jdemeyer

  • Cc vbraun added

Volker, it would be good if you could test on the buildbot that this ticket works when building from scratch.

comment:56 Changed 4 years ago by jdemeyer

  • Dependencies set to #19295
  • Status changed from needs_review to needs_work

Sorry for the trouble again, but this conflicts with #19295. I guess you should rebase it to that ticket. Alternatively, wait for the new Sage beta release.

Apart from that, I see no obvious issue with this ticket. It mostly needs testing. I will try to do that in the following days.

comment:57 follow-up: Changed 4 years ago by embray

I mean, so long as #19295 is merged I can rebase this again on develop right?

comment:58 in reply to: ↑ 57 Changed 4 years ago by jdemeyer

Replying to embray:

I mean, so long as #19295 is merged I can rebase this again on develop right?

...when the next beta/rc release comes out which includes #19295, yes.

Or you just rebase right now on the branch of #19295.

comment:59 Changed 4 years ago by jdemeyer

As far as I can tell, this ticket works fine. So the only obstruction is #19295.

comment:60 Changed 4 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer

comment:61 Changed 4 years ago by git

  • Commit changed from 7880aca0575e38126b2e6f510c6b8202d10d768b to 0c899a4e207e59e437406f71016fbffe47dbe9a3

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

3a90e69Use pip to install most Python-based dependencies; see http://trac.sagemath.org/ticket/20218
c028d41This is no longer needed.
83061e1Update some packages that were added since the start of this branch to use pip.
0c899a4Some of the packages changed to use pip in 2fe1d357600e60d3422141ba5a7fb16efb3a691b needed pip added to their build deps.

comment:62 Changed 4 years ago by embray

  • Status changed from needs_work to needs_review

Rebased

comment:63 Changed 4 years ago by vbraun

  • Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Volker Braun
  • Status changed from needs_review to positive_review

comment:64 Changed 4 years ago by embray

  • Status changed from positive_review to needs_work

I actually need to modify this slightly. The $PIP_INSTALL variable that's set in build/make/install is now needed for running the spkg-install for all the Python packages that use it. This means spkg-install can't be run directly and *must* go through the build/make/install script.

Instead, $PIP_INSTALL should probably be set in sage-env.

comment:65 Changed 4 years ago by git

  • Commit changed from 0c899a4e207e59e437406f71016fbffe47dbe9a3 to 8df4b5f444557ad9c98c8ba0bdeb5ccc08211302

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

8df4b5fMove PIP_INSTALL into sage-env so that it's available so long as the Sage environment is activated

comment:66 Changed 4 years ago by embray

  • Status changed from needs_work to needs_review

Done. Seems to work fine.


New commits:

8df4b5fMove PIP_INSTALL into sage-env so that it's available so long as the Sage environment is activated

comment:67 Changed 4 years ago by vbraun

  • Status changed from needs_review to positive_review

comment:68 Changed 4 years ago by vbraun

  • Status changed from positive_review to needs_work

Fails for me when building after "make distclean" with

sagenb-export 2.0 is already the active version in easy-install.pth
Installing sagenb-export script to /mnt/disk/home/release/Sage/local/bin

Installed /mnt/disk/home/release/Sage/local/lib/python2.7/site-packages/sagenb_export-2.0-py2.7.egg
Processing dependencies for sagenb-export==2.0
Searching for entrypoints
Reading https://pypi.python.org/simple/entrypoints/

The buildbot is not allowed to download packages.

The nbconvert package lists entrypoints in local/lib/python2.7/site-packages/nbconvert-4.2.0-py2.7.egg-info/requires.txt, but only with this ticket. Without this ticket, the depenency is not picked up.

Perhaps the easiest solution would be to package entrypoints...

comment:69 Changed 4 years ago by embray

sagenb_export isn't even a sage package in this branch. It must have been added since I last rebased. I mean, until the commit I just pushed it wasn't even being installed with pip so I don't know what the relation would be.

Except that whoever added sagenb_export as an spkg did not also add all its dependencies which unfortunately you have to do.

Last edited 4 years ago by embray (previous) (diff)

comment:70 Changed 4 years ago by git

  • Commit changed from 8df4b5f444557ad9c98c8ba0bdeb5ccc08211302 to de5cc9d73614faa62e52e6204266f6a38254c18f

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

c268094Use pip to install most Python-based dependencies; see http://trac.sagemath.org/ticket/20218
a5cfec2This is no longer needed.
aaf7a8cUpdate some packages that were added since the start of this branch to use pip.
ec6ca6cSome of the packages changed to use pip in 2fe1d357600e60d3422141ba5a7fb16efb3a691b needed pip added to their build deps.
820314eMove PIP_INSTALL into sage-env so that it's available so long as the Sage environment is activated
de5cc9dpip install sagenb_exports

comment:71 Changed 4 years ago by jdemeyer

  • Dependencies #19295 deleted
  • Milestone changed from sage-7.3 to sage-7.4

This is proving to be difficult to do all at once. Maybe this should be split up to "provide the base functionality" like the PIP_INSTALL stuff and then we can fix individual packages or batches of packages in follow-up tickets.

Then a comment about the implementation: making PIP_INSTALL an environment variable looks strange. Maybe a small script build/bin/pip-install would make more sense?

comment:72 follow-up: Changed 4 years ago by embray

I don't think making PIP_INSTALL an environment variable "looks strange" any more than CC does.

How is it proving to be too difficult? It works for me.

Last edited 4 years ago by embray (previous) (diff)

comment:73 Changed 4 years ago by embray

Ah, looks like it needs to be rebased again. No problem.

comment:74 in reply to: ↑ 72 Changed 4 years ago by jdemeyer

Replying to embray:

How is it proving to be too difficult?

Difficult to merge in the sense that it constantly conflicts with other tickets.

comment:75 Changed 4 years ago by git

  • Commit changed from de5cc9d73614faa62e52e6204266f6a38254c18f to d7218a077e4c2bffe8a8cd6a496e62313191a36b

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

6c9dffeUse pip to install most Python-based dependencies; see http://trac.sagemath.org/ticket/20218
63cade4This is no longer needed.
a692a30Update some packages that were added since the start of this branch to use pip.
3e16e00Some of the packages changed to use pip in 2fe1d357600e60d3422141ba5a7fb16efb3a691b needed pip added to their build deps.
332d915Move PIP_INSTALL into sage-env so that it's available so long as the Sage environment is activated
d6121b2pip install sagenb_exports
d7218a0Three new Python packages were added that should use pip for installation

comment:76 Changed 4 years ago by embray

Another question that comes to mind: Where would be the best place in the documentation to document how an spkg for a Python package should be installed? That should be updated ASAP to demonstrate using pip to install the package instead of the old-fashioned python setup.py install?

Anyways the sooner we get this merged, and update the documentation, the less "difficult" this will be.


New commits:

6c9dffeUse pip to install most Python-based dependencies; see http://trac.sagemath.org/ticket/20218
63cade4This is no longer needed.
a692a30Update some packages that were added since the start of this branch to use pip.
3e16e00Some of the packages changed to use pip in 2fe1d357600e60d3422141ba5a7fb16efb3a691b needed pip added to their build deps.
332d915Move PIP_INSTALL into sage-env so that it's available so long as the Sage environment is activated
d6121b2pip install sagenb_exports
d7218a0Three new Python packages were added that should use pip for installation

comment:77 follow-up: Changed 4 years ago by jdemeyer

Just a comment: I need this to fix the installation of widgetsnbextension, see #21256.

comment:78 Changed 4 years ago by jdemeyer

Is this ready for review?

comment:79 in reply to: ↑ 77 Changed 4 years ago by jdemeyer

Replying to jdemeyer:

Just a comment: I need this to fix the installation of widgetsnbextension, see #21256.

And it needs the --ignored-installed option that you are adding here, otherwise the broken installation remains broken.

comment:80 follow-up: Changed 4 years ago by jdemeyer

I do not like the --no-deps flag: it used to be an error if dependencies were not installed, now you will silently have a broken installation.

comment:81 Changed 4 years ago by jdemeyer

I also like --verbose: it actually shows the output from setup.py instead of

    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: still running...
    Running setup.py install for scipy: finished with status 'done'

comment:82 in reply to: ↑ 80 ; follow-up: Changed 4 years ago by embray

Replying to jdemeyer:

I do not like the --no-deps flag: it used to be an error if dependencies were not installed, now you will silently have a broken installation.

That's a problem that Sage's package distributions should be handling. In other words, you're asking pip and PyPI to handle dependency resolution for you. But if Sage wants to maintain its own package distribution system it's its responsibility to handle dependency resolution correctly.

Last edited 4 years ago by embray (previous) (diff)

comment:83 Changed 4 years ago by jdemeyer

  • Branch changed from u/embray/pip-install to u/jdemeyer/pip-install

comment:84 Changed 4 years ago by embray

  • Commit changed from d7218a077e4c2bffe8a8cd6a496e62313191a36b to c728bd8af325487e8d8578aa7dafe650c1b07492

I'm fine with adding the --verbose flag but -1 on removing --no-deps.


New commits:

c728bd8Remove --no-deps and add --verbose flag to pip

comment:85 in reply to: ↑ 82 Changed 4 years ago by jdemeyer

Replying to embray:

That's a problem that Sage's package distributions should be handling. In other words, you're asking pip and PyPI to handle dependency resolution for you.

Not really. I want dependency checking, not dependency resolution.

It is completely normal for a package to check its dependencies upon installation: GNU configure scripts usually do this.

The flag --no-index does exactly that: it will not install dependencies but it will still check for dependencies.

comment:86 Changed 4 years ago by embray

Okay, as long as it isn't recursive--that is, as long as it doesn't also check the dependencies for the dependencies--you're right that --no-index seems to ensure that.

comment:87 Changed 4 years ago by jdemeyer

Hmm, removing --no-deps breaks a lot of Jupyter stuff since they have setup.py containing stuff like

setuptools_args = {}
install_requires = setuptools_args['install_requires'] = [
    'ipython>=4.0.0',
    'traitlets',
    'jupyter_client',
    'tornado>=4.0',
]

if 'setuptools' in sys.modules:
    setup_args.update(setuptools_args)

These dependencies are not seen when installing using python setup.py install.

comment:88 Changed 4 years ago by jdemeyer

I will try to fix those dependencies.

comment:89 Changed 4 years ago by jdemeyer

Hmm, traitlets thinks that ipython_genutils is not installed but in reality it is installed (and it is properly listed as dependency in Sage):

[traitlets-4.2.2] Processing /usr/local/src/sage-git/local/var/tmp/sage/build/traitlets-4.2.2/src
[traitlets-4.2.2]   Running setup.py (path:/tmp/pip-TnBahx-build/setup.py) egg_info for package from file:///usr/local/src/sage-git/local/var/tmp/sage/build/traitlets-4.2.2/src
[traitlets-4.2.2]     Running command python setup.py egg_info
[traitlets-4.2.2]     running egg_info
[traitlets-4.2.2]     creating pip-egg-info/traitlets.egg-info
[traitlets-4.2.2]     writing requirements to pip-egg-info/traitlets.egg-info/requires.txt
[traitlets-4.2.2]     writing pip-egg-info/traitlets.egg-info/PKG-INFO
[traitlets-4.2.2]     writing top-level names to pip-egg-info/traitlets.egg-info/top_level.txt
[traitlets-4.2.2]     writing dependency_links to pip-egg-info/traitlets.egg-info/dependency_links.txt
[traitlets-4.2.2]     writing manifest file 'pip-egg-info/traitlets.egg-info/SOURCES.txt'
[traitlets-4.2.2]     warning: manifest_maker: standard file '-c' not found
[traitlets-4.2.2] 
[traitlets-4.2.2]     reading manifest file 'pip-egg-info/traitlets.egg-info/SOURCES.txt'
[traitlets-4.2.2]     writing manifest file 'pip-egg-info/traitlets.egg-info/SOURCES.txt'
[traitlets-4.2.2]   Source in /tmp/pip-TnBahx-build has version 4.2.2, which satisfies requirement traitlets==4.2.2 from file:///usr/local/src/sage-git/local/var/tmp/sage/build/traitlets-4.2.2/src
[traitlets-4.2.2] Collecting ipython_genutils (from traitlets==4.2.2)
[traitlets-4.2.2]   0 location(s) to search for versions of ipython-genutils:
[traitlets-4.2.2]   Could not find a version that satisfies the requirement ipython_genutils (from traitlets==4.2.2) (from versions: )
[traitlets-4.2.2] Cleaning up...
[traitlets-4.2.2]   Removing source in /tmp/pip-TnBahx-build
[traitlets-4.2.2] No matching distribution found for ipython_genutils (from traitlets==4.2.2)
[traitlets-4.2.2] Exception information:
[traitlets-4.2.2] Traceback (most recent call last):
[traitlets-4.2.2]   File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pip-8.1.2-py2.7.egg/pip/basecommand.py", line 215, in main
[traitlets-4.2.2]     status = self.run(options, args)
[traitlets-4.2.2]   File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pip-8.1.2-py2.7.egg/pip/commands/install.py", line 299, in run
[traitlets-4.2.2]     requirement_set.prepare_files(finder)
[traitlets-4.2.2]   File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pip-8.1.2-py2.7.egg/pip/req/req_set.py", line 370, in prepare_files
[traitlets-4.2.2]     ignore_dependencies=self.ignore_dependencies))
[traitlets-4.2.2]   File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pip-8.1.2-py2.7.egg/pip/req/req_set.py", line 522, in _prepare_file
[traitlets-4.2.2]     finder, self.upgrade, require_hashes)
[traitlets-4.2.2]   File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pip-8.1.2-py2.7.egg/pip/req/req_install.py", line 268, in populate_link
[traitlets-4.2.2]     self.link = finder.find_requirement(self, upgrade)
[traitlets-4.2.2]   File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pip-8.1.2-py2.7.egg/pip/index.py", line 491, in find_requirement
[traitlets-4.2.2]     'No matching distribution found for %s' % req
[traitlets-4.2.2] DistributionNotFound: No matching distribution found for ipython_genutils (from traitlets==4.2.2)

comment:90 Changed 4 years ago by jdemeyer

Hmm, could it be that the --ignore-installed option also ignores installed dependencies?

comment:91 Changed 4 years ago by embray

I'll look into it.

comment:92 Changed 4 years ago by jdemeyer

I tried --upgrade --force-reinstall instead of --ignore-installed but that gives the same problem:

$ pip install --verbose --no-index --upgrade --force-reinstall .
Ignoring indexes: https://pypi.python.org/simple
Processing /usr/local/src/sage-git/tmp/traitlets-4.2.2
  Running setup.py (path:/tmp/pip-Eh8Et4-build/setup.py) egg_info for package from file:///usr/local/src/sage-git/tmp/traitlets-4.2.2
    Running command python setup.py egg_info
    running egg_info
    creating pip-egg-info/traitlets.egg-info
    writing requirements to pip-egg-info/traitlets.egg-info/requires.txt
    writing pip-egg-info/traitlets.egg-info/PKG-INFO
    writing top-level names to pip-egg-info/traitlets.egg-info/top_level.txt
    writing dependency_links to pip-egg-info/traitlets.egg-info/dependency_links.txt
    writing manifest file 'pip-egg-info/traitlets.egg-info/SOURCES.txt'
    warning: manifest_maker: standard file '-c' not found

    reading manifest file 'pip-egg-info/traitlets.egg-info/SOURCES.txt'
    writing manifest file 'pip-egg-info/traitlets.egg-info/SOURCES.txt'
  Source in /tmp/pip-Eh8Et4-build has version 4.2.2, which satisfies requirement traitlets==4.2.2 from file:///usr/local/src/sage-git/tmp/traitlets-4.2.2
Collecting ipython_genutils (from traitlets==4.2.2)
  0 location(s) to search for versions of ipython-genutils:
  Could not find a version that satisfies the requirement ipython_genutils (from traitlets==4.2.2) (from versions: )
Cleaning up...
  Removing source in /tmp/pip-Eh8Et4-build
No matching distribution found for ipython_genutils (from traitlets==4.2.2)
Exception information:
Traceback (most recent call last):
  File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pip-8.1.2-py2.7.egg/pip/basecommand.py", line 215, in main
    status = self.run(options, args)
  File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pip-8.1.2-py2.7.egg/pip/commands/install.py", line 299, in run
    requirement_set.prepare_files(finder)
  File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pip-8.1.2-py2.7.egg/pip/req/req_set.py", line 370, in prepare_files
    ignore_dependencies=self.ignore_dependencies))
  File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pip-8.1.2-py2.7.egg/pip/req/req_set.py", line 522, in _prepare_file
    finder, self.upgrade, require_hashes)
  File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pip-8.1.2-py2.7.egg/pip/req/req_install.py", line 268, in populate_link
    self.link = finder.find_requirement(self, upgrade)
  File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pip-8.1.2-py2.7.egg/pip/index.py", line 491, in find_requirement
    'No matching distribution found for %s' % req
DistributionNotFound: No matching distribution found for ipython_genutils (from traitlets==4.2.2)

comment:93 follow-up: Changed 4 years ago by embray

Yes, you're right. It seems that --ignore-installed also ignores installed dependencies. The fact that I also had --no-deps hid that. That's disconcerting. It looks like you can't do dependency checking the way you wanted after all, though I'll admit that's unfortunate. You'd think it should work. I might treat that as a defect in pip.

But I don't think it's a showstopper. I still think it should be the responsibility of the sage packager to work this out and not blindly add new packages without making sure all its dependencies are correct too.

comment:94 in reply to: ↑ 93 Changed 4 years ago by jdemeyer

Replying to embray:

I still think it should be the responsibility of the sage packager to work this out and not blindly add new packages without making sure all its dependencies are correct too.

I would love this to become official Sage policy. The main culprit is mass-update tickets like #20625.

comment:95 Changed 4 years ago by embray

A relevant ticket in pip: https://github.com/pypa/pip/issues/3261

It's mentioned there that there's also a new pip check command that will do a sanity-check on the system, so running that could also be policy (without necessarily requiring the check on every package install).

comment:96 follow-up: Changed 4 years ago by jdemeyer

It seems that pip is missing an option to force-reinstall a single package without recursion.

comment:97 Changed 4 years ago by git

  • Commit changed from c728bd8af325487e8d8578aa7dafe650c1b07492 to fd131c1a550242299e367080fa1a77a5823daaed

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

fd131c1Add --verbose flag to pip

comment:98 Changed 4 years ago by jdemeyer

OK, undid the removal of --no-deps.

I would be fine with merging this now, but still thinking about future improvements.

comment:99 in reply to: ↑ 96 ; follow-up: Changed 4 years ago by embray

Replying to jdemeyer:

It seems that pip is missing an option to force-reinstall a single package without recursion.

pip install --upgrade --force --no-deps does it. I've used that in the past.

comment:100 in reply to: ↑ 99 Changed 4 years ago by jdemeyer

Replying to embray:

Replying to jdemeyer:

It seems that pip is missing an option to force-reinstall a single package without recursion.

pip install --upgrade --force --no-deps does it.

--upgrade does recursion:

  -U, --upgrade               Upgrade all specified packages to the newest available version. This process is recursive regardless of whether a dependency is already
                              satisfied.

Edit: I see you added --no-deps but I don't want that since I do want dependency checking.

Last edited 4 years ago by jdemeyer (previous) (diff)

comment:101 follow-up: Changed 4 years ago by embray

Well, yes, but now we know that dependency checking in the sense that you want doesn't work anyways.

comment:102 in reply to: ↑ 101 Changed 4 years ago by jdemeyer

Replying to embray:

Well, yes, but now we know that dependency checking in the sense that you want doesn't work anyways.

It works by itself, but not in combination with the other options.

comment:103 Changed 4 years ago by jdemeyer

Something like

pip uninstall PACKAGE
pip install --no-index .

would work and correctly check dependencies. There is just not a single command which does that.

comment:104 Changed 4 years ago by jdemeyer

For reasons that I do not understand, this breaks running Jupyter from the command line:

$ jupyter notebook
Traceback (most recent call last):
  File "/usr/local/src/sage-git/local/bin/jupyter-notebook", line 5, in <module>
    from pkg_resources import load_entry_point
  File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2927, in <module>
    @_call_aside
  File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2913, in _call_aside
    f(*args, **kwargs)
  File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2940, in _initialize_master_working_set
    working_set = WorkingSet._build_master()
  File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 635, in _build_master
    ws.require(__requires__)
  File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 943, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/usr/local/src/sage-git/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 829, in resolve
    raise DistributionNotFound(req, requirers)
pkg_resources.DistributionNotFound: The 'terminado>=0.3.3' distribution was not found and is required by notebook

comment:105 Changed 4 years ago by jdemeyer

The obvious solution is to add a terminado package to Sage and have notebook depend on it.

comment:106 Changed 4 years ago by jdemeyer

Alternative and simpler solution: keep using distutils instead of pip to install notebook.

comment:107 Changed 4 years ago by git

  • Commit changed from fd131c1a550242299e367080fa1a77a5823daaed to 9b13184db417dbef7b75d28fdd821e93ae46584a

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

9b13184Install the Jupyter notebook using distutils to avoid dependency on terminado

comment:108 Changed 4 years ago by jdemeyer

This is exactly the kind of problem which can happen because of --no-deps: notebook has an unsatisfied dependency (terminado). This breaks stuff but might pass automated tested since actually running jupyter is not tested.

comment:109 follow-up: Changed 4 years ago by embray

I'm not sure I'm following you. Obviously jupyter notebook has a dependency that isn't satisfied because Sage doesn't include it. Just forcing it to install without that dependency and leaving it broken is not a solution.

comment:110 in reply to: ↑ 109 Changed 4 years ago by jdemeyer

Replying to embray:

Obviously jupyter notebook has a dependency that isn't satisfied because Sage doesn't include it. Just forcing it to install without that dependency and leaving it broken is not a solution.

I would argue that the bug is in Jupyter's requirements list: it lists a dependency which in practice is not a dependency at all, but more like an optional add-on.

I just want to get on with this and this is a simple solution which is known to work.

comment:111 Changed 4 years ago by git

  • Commit changed from 9b13184db417dbef7b75d28fdd821e93ae46584a to feed1dceedc23e9662800483ee010f585a90c1ee

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

feed1dcAdd a few missing dependencies

comment:112 Changed 4 years ago by jdemeyer

This last commit adds a few missing dependencies, exposed by #21256.

comment:113 Changed 4 years ago by embray

To be fair, terminado supplies the terminal interface in the Notebook, so even if as you're claiming it's optional, it is certainly nice to have. In any case you can say it's a bug or not but if they're including it in their requirements list then I say it's a requirement unless specified otherwise. This is definitely no solution.

comment:114 Changed 4 years ago by jdemeyer

So what we should then? I see two options:

  1. Patch notebook/setup.py to remove terminado from the dependencies.
  1. Add a terminado package to Sage as dependency of this ticket.

comment:115 Changed 4 years ago by embray

I suggest option 2. As it is, for the Docker image for Sage I'm also installing terminado separately. I don't see why it shouldn't be included.

If it helps, looking at the setup.py for notebook, it seems terminado is being considered a requirement on any non-Windows platform. Probably because even though they've been careful to make it so that the notebook can work without it, not having it means missing functionality==a defect. I don't think we should say that "Sage includes the Jupyter notebook" and then ship a defective product.

The reason you get this

pkg_resources.DistributionNotFound: The 'terminado>=0.3.3' distribution was not found and is required by notebook

is that installing with setuptools also installs the autogenerated console script wrappers which do dependency checking when you run them, so it's right to say that it won't run without some dependency installed.

comment:116 Changed 4 years ago by git

  • Commit changed from feed1dceedc23e9662800483ee010f585a90c1ee to 6c47f0bd4d4342a0f22a9e61cf0721430b240276

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

6c47f0bAdd a few missing dependencies

comment:117 Changed 4 years ago by git

  • Commit changed from 6c47f0bd4d4342a0f22a9e61cf0721430b240276 to 829a5b445e8f6b1e7eab56ae8e428f5995e4cebd

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

afc90d4Add terminado as standard package
115e77bMerge remote-tracking branch 'trac/u/jdemeyer/add_a_terminado_package' into t/20218/pip-install
829a5b4Add a few missing dependencies

comment:118 Changed 4 years ago by jdemeyer

  • Dependencies set to #21260

comment:119 Changed 4 years ago by jdemeyer

  • Dependencies changed from #21260 to #21260, #21261

comment:120 Changed 4 years ago by git

  • Commit changed from 829a5b445e8f6b1e7eab56ae8e428f5995e4cebd to 1e0a377879cd1e9eea3b6651d57ac1fd9564cf95

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

fdb403eAdd entrypoints and configparser packages
c4fc97cMerge remote-tracking branches 'trac/u/jdemeyer/add_a_terminado_package' and 'trac/u/jdemeyer/add_a_entrypoints_package' into t/20218/pip-install
1e0a377Fix installation of Jupyter packages

comment:121 Changed 4 years ago by git

  • Commit changed from 1e0a377879cd1e9eea3b6651d57ac1fd9564cf95 to 7054c41c14c7a619fee1ccaa79da1273e5713727

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

7054c41Fix installation of Jupyter packages

comment:122 Changed 4 years ago by jdemeyer

  • Authors changed from Erik Bray to Erik Bray, Jeroen Demeyer
  • Status changed from needs_work to needs_review

comment:123 Changed 4 years ago by vbraun

  • Status changed from needs_review to positive_review

comment:124 Changed 4 years ago by embray

  • Status changed from positive_review to needs_work

This is minor, but a few packages (configparser, terminado, entrypoints) still have an explicit pip install in their spgk-install instead of the $PIP_INSTALL. The effect is the same, but if it's not fixed now but the idea was to have a standard way of invoking pip for all packages, and it could have surprising effects if some packages aren't getting the same arguments for pip.

comment:125 Changed 4 years ago by jdemeyer

  • Status changed from needs_work to positive_review

That can be fixed in a follow-up ticket. There is no reason to hold up this ticket for that.

comment:126 Changed 4 years ago by vbraun

  • Branch changed from u/jdemeyer/pip-install to 7054c41c14c7a619fee1ccaa79da1273e5713727
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:127 Changed 4 years ago by vbraun

  • Commit 7054c41c14c7a619fee1ccaa79da1273e5713727 deleted
  • Resolution fixed deleted
  • Status changed from closed to new

Breaks OSX when compiling from scratch in numpy

[numpy-1.11.1] gcc version 4.9.3 (GCC) 
[numpy-1.11.1] ****************************************************
[numpy-1.11.1] patching file numpy/core/function_base.py
[numpy-1.11.1] patching file numpy/distutils/system_info.py
[numpy-1.11.1] Hunk #1 succeeded at 1685 (offset -5 lines).
[numpy-1.11.1] Hunk #2 succeeded at 1718 (offset -4 lines).
[numpy-1.11.1] 
[numpy-1.11.1] Usage:   
[numpy-1.11.1]   pip install [options] <requirement specifier> [package-index-options] ...
[numpy-1.11.1]   pip install [options] -r <requirements file> [package-index-options] ...
[numpy-1.11.1]   pip install [options] [-e] <vcs project url> ...
[numpy-1.11.1]   pip install [options] [-e] <local project path> ...
[numpy-1.11.1]   pip install [options] <archive url/path> ...
[numpy-1.11.1] 
[numpy-1.11.1] no such option: ---global-option
[numpy-1.11.1] 
[numpy-1.11.1] real	0m0.519s
[numpy-1.11.1] user	0m0.269s
[numpy-1.11.1] sys	0m0.142s
[numpy-1.11.1] ************************************************************************
[numpy-1.11.1] Error installing package numpy-1.11.1

Note on OSX:

osx:build buildslave-sage$ NUMPY_FCONFIG="config_fc --noopt --noarch"
osx:build buildslave-sage$ echo $NUMPY_FCONFIG | sed -e 's/\(\S*\)/--global-option \1/g'
--global-option c--global-option o--global-option n--global-option f--global-option i--global-option g--global-option _--global-option f--global-option c--global-option  --global-option ---global-option ---global-option n--global-option o--global-option o--global-option p--global-option t--global-option  --global-option ---global-option ---global-option n--global-option o--global-option a--global-option r--global-option c--global-option h--global-option 

comment:128 follow-up: Changed 4 years ago by embray

Apparently on OSX that should be sed -E not sed -e :(

comment:129 in reply to: ↑ 128 Changed 4 years ago by embray

Replying to embray:

Apparently on OSX that should be sed -E not sed -e :(

In fact the -e doesn't really even need to be there. I'm not sure why I put it. But on OSX it seems the -E is needed to interpret the command as a "modern" perl-style regexp.

comment:130 Changed 4 years ago by embray

Or for a more portable way without invoking sed at all:

$(for opt in $NUMPY_FCONFIG; do printf ' %s %s' --global-option $opt; done)

comment:131 Changed 4 years ago by vbraun

  • Branch changed from 7054c41c14c7a619fee1ccaa79da1273e5713727 to u/jdemeyer/pip-install
  • Commit set to 7054c41c14c7a619fee1ccaa79da1273e5713727

I don't really care which variant long as it works ;-)


Last 10 new commits:

a692a30Update some packages that were added since the start of this branch to use pip.
3e16e00Some of the packages changed to use pip in 2fe1d357600e60d3422141ba5a7fb16efb3a691b needed pip added to their build deps.
332d915Move PIP_INSTALL into sage-env so that it's available so long as the Sage environment is activated
d6121b2pip install sagenb_exports
d7218a0Three new Python packages were added that should use pip for installation
fd131c1Add --verbose flag to pip
afc90d4Add terminado as standard package
fdb403eAdd entrypoints and configparser packages
c4fc97cMerge remote-tracking branches 'trac/u/jdemeyer/add_a_terminado_package' and 'trac/u/jdemeyer/add_a_entrypoints_package' into t/20218/pip-install
7054c41Fix installation of Jupyter packages

comment:132 Changed 4 years ago by jdemeyer

  • Dependencies #21260, #21261 deleted

Is this ready for review?

comment:133 Changed 4 years ago by jdemeyer

I guess not, since no changes have been made :-)

comment:134 follow-up: Changed 4 years ago by git

  • Commit changed from 7054c41c14c7a619fee1ccaa79da1273e5713727 to 99e1494840eac3dbb10d57b4721c4ff281c47c4e

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

99e1494Revert changes to numpy

comment:135 Changed 4 years ago by git

  • Commit changed from 99e1494840eac3dbb10d57b4721c4ff281c47c4e to f5c0b161de6ad824e7eb8752c072c0adab105a41

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

f5c0b16wcwidth depends on pip

comment:136 in reply to: ↑ 134 ; follow-ups: Changed 4 years ago by embray

Replying to git:

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

99e1494Revert changes to numpy

I mean that's fine--what this is doing is in effect the same. But you could have taken one of my suggestions above. I think the for loop is most effective. I mean I would just do it but you sort of took over this ticket

Last edited 4 years ago by embray (previous) (diff)

comment:137 in reply to: ↑ 136 Changed 4 years ago by jdemeyer

I just want this ticket to get merged. We can deal with the difficult cases (like numpy) later.

Last edited 4 years ago by jdemeyer (previous) (diff)

comment:138 in reply to: ↑ 136 Changed 4 years ago by jdemeyer

  • Status changed from new to needs_review

Replying to embray:

I mean that's fine

May I take that as positive review?

comment:139 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:140 Changed 4 years ago by embray

Yeah, yeah :) *fingers crossed*

comment:141 Changed 4 years ago by vbraun

  • Status changed from positive_review to needs_work

IPython on OSX appears to require a platform-specific package:

sage -t --long src/sage/tests/cmdline.py
**********************************************************************
File "src/sage/tests/cmdline.py", line 476, in sage.tests.cmdline.test_executable
Failed example:
    out.find("5559060566555523") >= 0
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/tests/cmdline.py", line 478, in sage.tests.cmdline.test_executable
Failed example:
    err
Expected:
    ''
Got:
    'Traceback (most recent call last):\n  File "/Users/buildslave-sage/slave/sage_git/build/local/bin/ipython", line 5, in <module>\n    from pkg_resources import load_entry_point\n  File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2927, in <module>\n    @_call_aside\n  File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2913, in _call_aside\n    f(*args, **kwargs)\n  File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2940, in _initialize_master_working_set\n    working_set = WorkingSet._build_master()\n  File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 635, in _build_master\n    ws.require(__requires__)\n  File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 943, in require\n    needed = self.resolve(parse_requirements(requirements))\n  File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 829, in resolve\n    raise DistributionNotFound(req, requirers)\npkg_resources.DistributionNotFound: The \'appnope\' distribution was not found and is required by ipython\n'
**********************************************************************
File "src/sage/tests/cmdline.py", line 480, in sage.tests.cmdline.test_executable
Failed example:
    ret
Expected:
    0
Got:
    1
**********************************************************************
1 item had failures:
   3 of 239 in sage.tests.cmdline.test_executable
    [238 tests, 3 failures, 71.27 s]

comment:143 Changed 4 years ago by vbraun

osx:build buildslave-sage$ ./sage --ipython
Traceback (most recent call last):
  File "/Users/buildslave-sage/slave/sage_git/build/local/bin/ipython", line 5, in <module>
    from pkg_resources import load_entry_point
  File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2927, in <module>
    @_call_aside
  File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2913, in _call_aside
    f(*args, **kwargs)
  File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2940, in _initialize_master_working_set
    working_set = WorkingSet._build_master()
  File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 635, in _build_master
    ws.require(__requires__)
  File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 943, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/pkg_resources/__init__.py", line 829, in resolve
    raise DistributionNotFound(req, requirers)
pkg_resources.DistributionNotFound: The 'appnope' distribution was not found and is required by ipython
osx:build buildslave-sage$ ./sage -pip install appnope
Collecting appnope
  Downloading appnope-0.1.0-py2.py3-none-any.whl
Installing collected packages: appnope
Successfully installed appnope-0.1.0
osx:build buildslave-sage$ ./sage --ipython
Python 2.7.10 (default, Aug 27 2016, 01:49:52) 
Type "copyright", "credits" or "license" for more information.

IPython 5.0.0 -- An enhanced Interactive Python.
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object', use 'object??' for extra details.

In [1]: 

comment:144 Changed 4 years ago by git

  • Commit changed from f5c0b161de6ad824e7eb8752c072c0adab105a41 to 55f9e9d6aa4e49777418dcfe74ddaa55fb4a269a

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

55f9e9dAdd appnope package

comment:145 Changed 4 years ago by jdemeyer

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:146 Changed 4 years ago by vbraun

  • Status changed from needs_review to positive_review

comment:147 Changed 4 years ago by vbraun

  • Status changed from positive_review to needs_work
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',

comment:148 Changed 4 years ago by embray

Heh. What an insidious test :)

comment:149 Changed 4 years ago by jdemeyer

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

comment:150 Changed 4 years ago by git

  • Commit changed from 55f9e9d6aa4e49777418dcfe74ddaa55fb4a269a to 7f365e3b0d648976056e11d3e02918aca2b525a7

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

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

comment:151 Changed 4 years ago by git

  • Commit changed from 7f365e3b0d648976056e11d3e02918aca2b525a7 to f6859c31c0322c22d9ad2b6ab21b8e8c8948e16d

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

f6859c3Mark package tests as random

comment:152 Changed 4 years ago by jdemeyer

  • Status changed from needs_work to needs_review

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.

Last edited 4 years ago by jdemeyer (previous) (diff)

comment:153 Changed 4 years ago by jdemeyer

Please review this trivial fix...

comment:154 Changed 4 years ago by embray

  • Status changed from needs_review to positive_review

comment:155 Changed 4 years ago by vbraun

  • Branch changed from u/jdemeyer/pip-install to f6859c31c0322c22d9ad2b6ab21b8e8c8948e16d
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:156 in reply to: ↑ 31 Changed 4 years ago by jdemeyer

  • Commit f6859c31c0322c22d9ad2b6ab21b8e8c8948e16d deleted

Replying to embray:

pip will uninstall them, mostly.

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

comment:157 Changed 4 years ago by embray

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.

Note: See TracTickets for help on using tickets.