Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drop easy_install script/command, re-implement `fetch_build_egg` to use pip #1830

Merged
merged 3 commits into from Nov 16, 2019

Conversation

@benoit-pierre
Copy link
Member

benoit-pierre commented Aug 24, 2019

Summary of changes

Drop the easy_install script and setuptools command. Re-implement fetch_build_egg to use pip for fetching/building a wheel of the requirement.

Pros:

  • with pip in charge of fetching things, support for python_requires, better support for wheels (proper handling of priority with respect to PEP 425 tags)
  • assuming pip's version is recent enough, PEP 517/518 support

Cons:

  • obviously, pip must be available (and wheel, depending on whether PEP 517/518 is used or not)
  • no support for eggs and Windows executable distributions
  • no support for the allow_hosts easy_install option (there's no corresponding pip option we can use to implement it, index_url/find_links are still honored)

Note: pip environment variables are honored (and take precedence over easy_install options).

The next step would be to drop the develop/install commands support for installing dependencies, trim down easy_install/develop options (the develop command class share all the easy_install supported options): keeping only the bare minimum (including keeping the ones used by pip). See WIP.

Pull Request Checklist

  • Changes have tests
  • News fragment added in changelog.d. See documentation for details
@benoit-pierre

This comment has been minimized.

Copy link
Member Author

benoit-pierre commented Aug 24, 2019

Note that this does not work with pytest-runner: since it uses a custom distribution re-implementing fetch_build_egg, easy_install is still used for handling tests_require. I do have a (backward compatible) patch that I intend to PR if this one is accepted:

 ptr.py | 54 +++++++++---------------------------------------------
 1 file changed, 9 insertions(+), 45 deletions(-)

diff --git i/ptr.py w/ptr.py
index f3952e8..cd753cf 100644
--- i/ptr.py
+++ w/ptr.py
@@ -2,7 +2,6 @@
 Implementation
 """
 
-import os as _os
 import shlex as _shlex
 import contextlib as _contextlib
 import sys as _sys
@@ -37,51 +36,16 @@ class CustomizedDist(Distribution):
     allow_hosts = None
     index_url = None
 
-    def fetch_build_egg(self, req):
-        """ Specialized version of Distribution.fetch_build_egg
+    def get_option_dict(self, command):
+        """ Specialized version of Distribution.get_option_dict
         that respects respects allow_hosts and index_url. """
-        from setuptools.command.easy_install import easy_install
-
-        dist = Distribution({'script_args': ['easy_install']})
-        dist.parse_config_files()
-        opts = dist.get_option_dict('easy_install')
-        keep = (
-            'find_links',
-            'site_dirs',
-            'index_url',
-            'optimize',
-            'site_dirs',
-            'allow_hosts',
-        )
-        for key in list(opts):
-            if key not in keep:
-                del opts[key]  # don't use any other settings
-        if self.dependency_links:
-            links = self.dependency_links[:]
-            if 'find_links' in opts:
-                links = opts['find_links'][1].split() + links
-            opts['find_links'] = ('setup', links)
-        if self.allow_hosts:
-            opts['allow_hosts'] = ('test', self.allow_hosts)
-        if self.index_url:
-            opts['index_url'] = ('test', self.index_url)
-        install_dir_func = getattr(self, 'get_egg_cache_dir', _os.getcwd)
-        install_dir = install_dir_func()
-        cmd = easy_install(
-            dist,
-            args=["x"],
-            install_dir=install_dir,
-            exclude_scripts=True,
-            always_copy=False,
-            build_directory=None,
-            editable=False,
-            upgrade=False,
-            multi_version=True,
-            no_report=True,
-            user=False,
-        )
-        cmd.ensure_finalized()
-        return cmd.easy_install(req)
+        opts = super().get_option_dict(command)
+        if command == 'easy_install':
+            if self.allow_hosts:
+                opts['allow_hosts'] = ('test', self.allow_hosts)
+            if self.index_url:
+                opts['index_url'] = ('test', self.index_url)
+        return opts
 
 
 class PyTest(orig.test):
@pganssle

This comment has been minimized.

Copy link
Member

pganssle commented Aug 24, 2019

@benoit-pierre I think this is probably a good idea, but I suspect it's a pretty major breaking change. How about instead of a hard requirement on pip, we do a somewhat softer adoption of pip by moving all the easy_install stuff into a legacy or compatibility module, move all the documentation onto its own page with a big "legacy" warning, and then do a soft fallback to easy_install. For the rare individuals who don't have pip installed for whatever reason, they'll just get some deprecation warnings whenever they hit a path that requires easy_install.

That should move the majority of people over to pip pretty seamlessly.

@jaraco

This comment has been minimized.

Copy link
Member

jaraco commented Sep 2, 2019

Wow. This is simply amazing. Thanks for working on this issue.

I had forgotten setuptools had a wheel.Wheel.install_as_egg, which allows this change to install eggs. Eventually, setuptools may want to drop support for the egg format altogether, but for now, this approach achieves the big win while relying on eggs for compatibility.

I'm +0 on Paul's suggestion. It seems sound to provide as smooth a transition as possible, just as long as it doesn't scuttle this effort.

@benoit-pierre

This comment has been minimized.

Copy link
Member Author

benoit-pierre commented Sep 4, 2019

This change is also intended as a stepping stone for further trimming down the easy_install code, keeping an easy_install code path when pip is not available would prevent that.

@pganssle

This comment has been minimized.

Copy link
Member

pganssle commented Sep 4, 2019

This change is also intended as a stepping stone for further trimming down the easy_install code, keeping an easy_install code path when pip is not available would prevent that.

I agree that the end goal of this should definitely be removing easy_install, but I think a deprecation period is worth putting that off a bit longer. We're effectively taking an undeclared dependency on pip in this PR, and pip is not guaranteed in Python 2, plus it's not unheard-of for distros to cut pip out of the standard library (not that we should be going out of our way to support this, mind you).

If at all possible I'd like there to be a notice period so that we can ease people into this new world and find any weird problems that arise. People use setuptools in weird ways.

@benoit-pierre

This comment has been minimized.

Copy link
Member Author

benoit-pierre commented Oct 7, 2019

If at all possible I'd like there to be a notice period so that we can ease people into this new world and find any weird problems that arise.

How long would that be? Does that include the removal of the top-level easy_install script?

@jaraco

This comment has been minimized.

Copy link
Member

jaraco commented Oct 7, 2019

How long would that be? Does that include the removal of the top-level easy_install script?

A few months, probably. I imagine the top-level easy_install script can be moved to another library ("easy_install" probably) to provide that functionality while still allowing setuptools versions to advance without it.

@benoit-pierre

This comment has been minimized.

Copy link
Member Author

benoit-pierre commented Oct 7, 2019

I've moved the uncontroversial stuff to separate PRs:

@benoit-pierre benoit-pierre force-pushed the benoit-pierre:pip_wheels branch from 4b2f938 to b8101f0 Nov 15, 2019
@benoit-pierre

This comment has been minimized.

Copy link
Member Author

benoit-pierre commented Nov 15, 2019

OK, so now fetch_build_egg fallbacks to EasyInstall if the pip package is not available. The easy_install script and associated documentation have been removed. The easy_install command is still there, but marked as deprecated.

@benoit-pierre benoit-pierre mentioned this pull request Nov 15, 2019
1 of 1 task complete
@jaraco
jaraco approved these changes Nov 16, 2019
Copy link
Member

jaraco left a comment

Looks good to me.

@jaraco jaraco merged commit 402240c into pypa:master Nov 16, 2019
6 checks passed
6 checks passed
Summary 1 potential rule
Details
codecov/patch 89.16% of diff hit (target 84.55%)
Details
codecov/project Absolute coverage decreased by -0.33% but relative coverage increased by +4.61% compared to 5ef7e75
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
@benoit-pierre benoit-pierre deleted the benoit-pierre:pip_wheels branch Nov 16, 2019
@pganssle

This comment has been minimized.

Copy link
Member

pganssle commented Nov 16, 2019

@benoit-pierre @jaraco Are you sure that this doesn't remove the easy_install command? It looks a lot like easy_install was removed:

$ python -m virtualenv venv
Using base prefix '/home/nlx5/.pyenv/versions/3.7.4'
New python executable in /tmp/test_sut/setuptools/venv/bin/python
Installing setuptools, pip, wheel...
done.
$ source venv/bin/activate
(venv) $ pip uninstall setuptools
Uninstalling setuptools-41.6.0:
  Would remove:
    /tmp/test_sut/setuptools/venv/bin/easy_install
    /tmp/test_sut/setuptools/venv/bin/easy_install-3.7
    /tmp/test_sut/setuptools/venv/lib/python3.7/site-packages/easy_install.py
    /tmp/test_sut/setuptools/venv/lib/python3.7/site-packages/pkg_resources/*
    /tmp/test_sut/setuptools/venv/lib/python3.7/site-packages/setuptools-41.6.0.dist-info/*
    /tmp/test_sut/setuptools/venv/lib/python3.7/site-packages/setuptools/*
Proceed (y/n)? y
  Successfully uninstalled setuptools-41.6.0
(venv) $ python bootstrap.py
...
(venv) $ rm pyproject.toml
(venv) $ pip install .
$ pip install .
Processing /tmp/test_sut/setuptools
Building wheels for collected packages: setuptools
  Building wheel for setuptools (setup.py) ... done
  Created wheel for setuptools: filename=setuptools-41.6.0.post20191116-py2.py3-none-any.whl size=582592 sha256=c93a1e37f07f74a2612076baa0ad7a4d8b8cac44a95c9569c6b2ff3758b142c1
  Stored in directory: /tmp/pip-ephem-wheel-cache-s50pzsam/wheels/76/ad/a3/0c86c1f1e0817014676989545b3ed3d3c227c5dff2fe891c85
Successfully built setuptools
Installing collected packages: setuptools
Successfully installed setuptools-41.6.0.post20191116
$ ls venv/bin
activate      activate.fish  activate_this.py  pip   pip3.7  python3    python-config
activate.csh  activate.ps1   activate.xsh      pip3  python  python3.7  wheel

The thing is, I think this will cause huge problems, because when I do which easy_install within the virtual environment, easy_install is still found, now it just refers to my system python (which is a pyenv shim).

AFAICT invoking easy_install won't send out a deprecation message and it will break out of virtual environments. I think we really need to put a stub easy_install back in place ASAP.

@benoit-pierre

This comment has been minimized.

Copy link
Member Author

benoit-pierre commented Nov 16, 2019

The easy_install script is removed, the setuptools command is still there.

@pganssle

This comment has been minimized.

Copy link
Member

pganssle commented Nov 16, 2019

OK, that explains the discrepancy. I really think we need to restore a deprecated stub ASAP. There's still a lot of examples out there suggesting easy_install, and this could mess up peoples' installation environments (it's already messed up mine, but I'm fine with that because my setup is designed to be easy to wipe out and restore).

@benoit-pierre

This comment has been minimized.

Copy link
Member Author

benoit-pierre commented Nov 16, 2019

As for getting the system version instead, how is that different from what can happen with any other Python script, like people calling the wrong pip because they have not updated their PATH to include ~/.local/bin.

@benoit-pierre

This comment has been minimized.

Copy link
Member Author

benoit-pierre commented Nov 16, 2019

So we'll never remove the easy_install script?

@pganssle

This comment has been minimized.

Copy link
Member

pganssle commented Nov 16, 2019

Probably correct. Note that when I removed python setup.py upload and python setup.py register, I replaced it with a command that just hard-fails rather than removing it and letting it fall back to the distutils version.

We really can't be going 0-60 like this. A lot of people don't know what is and isn't deprecated because we're not great about messaging and documentation. I suggest a 9-12 month period of deprecation where easy_install is explicitly raising deprecation warnings, followed by changing the easy_install script to something like this:

#! python
raise RuntimeError("easy_install has been removed in favor of pip.")
@jaraco

This comment has been minimized.

Copy link
Member

jaraco commented Nov 16, 2019

Good points. Happy to take it slow as long as things can keep moving.

@benoit-pierre

This comment has been minimized.

Copy link
Member Author

benoit-pierre commented Nov 17, 2019

See #1909.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.