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

Extract wheels directly to install location #6030

Closed
zooba opened this issue Nov 21, 2018 · 12 comments · Fixed by #8562
Closed

Extract wheels directly to install location #6030

zooba opened this issue Nov 21, 2018 · 12 comments · Fixed by #8562
Labels
C: wheel The wheel format and 'pip wheel' command type: enhancement Improvements to functionality

Comments

@zooba
Copy link
Contributor

zooba commented Nov 21, 2018

The current sequence of events for installing a wheel involves extracting it to a temporary location, then copying it to the install location. This requires additional work to copy metadata and permissions, which would not be necessary if it started in the target location (or within one directory rename of the target location).

Avoiding the copy step would be a significant performance increase for installing wheels, particularly on Windows where the current process triggers anti-malware scanners multiple times.

(Forked from #3055)

@zooba
Copy link
Contributor Author

zooba commented Nov 21, 2018

Okay, so I have two choices for the shorter install step. I'm happy to do either, but the latter is a bigger change and I'll only go that way if you were wanting to do it anyway:

  1. Pass enough options (prefix, root, etc.) to RequirementPreparer that I can calculate the location to extract wheels to
  2. Defer wheel extraction (other than metadata) until the install step

Right now there's consistency between all the different source types as handled by the pieces of the install command, so taking the second approach will complicate that. Then again, if you were hoping to streamline the install command to something more like "acquire files, resolve dependencies, repeat until ready, build non-wheels, install wheels" then I can start moving this way?

@pradyunsg
Copy link
Member

pradyunsg commented Dec 2, 2018

Let's do the later.

@pradyunsg pradyunsg added type: enhancement Improvements to functionality C: wheel The wheel format and 'pip wheel' command labels Dec 2, 2018
@zooba
Copy link
Contributor Author

zooba commented Dec 3, 2018

I'll try and map things out and come up with a proper design proposal before moving too far forward. I think I want a new requirement type/subclass for wheels (to separate "install requirement" from "build/install requirement"), but I'm not totally sure yet.

@zooba
Copy link
Contributor Author

zooba commented Dec 3, 2018

Okay, so here's my high level plan:

  • in the install command, ensure all reqs are editable or wheels before starting the actual install (this should just be a bit more filtering)
  • in InstallRequirement.install, only support editable and wheel installs
  • make building non-wheel requirements always produce a wheel file or directory (no point unzipping when we can copy, but the wheel file will be produced for the cache)
  • when an install requirement links to a wheel, treat source_dir as pointing directly at the wheel file
  • (maybe this means just changing the value; maybe it means adding more if checks; not 100% sure yet)
  • use ZipImporter (or a manually implemented equivalent) for extracting metadata from wheel files
  • make move_wheel_files able to extract from a file when source_dir points at a wheel, and in this case use the AdjacentTempDirectory I'm adding in Fixes #3055 Uninstall causes paths to exceed MAX_PATH limit #6029 to avoid overwriting files before they're fully extracted

I think because of the way that pkg_resources is being used for metadata, it'll turn out to be simplest to change source_dir to point directly at the wheel file. Flushing out the code paths that don't like that assumption is the tricky part.

@pradyunsg
Copy link
Member

pradyunsg commented Dec 4, 2018

There is a no-binary option - which basically means that a certain package should be installed from an sdist directly. We can't remove the ability to setup.py install just yet. ;)

#5051 has some notes on a potential design for changing InstallRequirement. It's basically making a Distribution class which will be used for preparation / installation that differs for legacy sdist vs PEP 517 sdist vs wheel. It's a multi stage process to get there and I've been working on actually implementing it, for a day now. One of the effects of that would be of untangling the code to make things like identifying code that's only run for sdist vs wheel vs editable become easier to spot.

@pfmoore
Copy link
Member

pfmoore commented Dec 4, 2018

@pradyunsg My understanding is that --no-binary simply prohibits the download of wheels. It doesn't require that a sdist has to be direct-installed - and indeed if PEP 517 is being used, a sdist is installed by building a wheel, and then installing the wheel, even when --no-binary is present.

Having said that, switching to always installing by building a wheel first, even on the "legacy" path, is a definite change in behaviour, and one that we need to explicitly and clearly document. It'll probably be controversial, but honestly I think we'll have to bite the bullet at some point, and doing it as part of a change that adds other value is better than doing it as a standalone piece of work that only has internal benefits.

@zooba
Copy link
Contributor Author

zooba commented Dec 4, 2018

I agree with Paul that biting the bullet is going to have to happen, and this seems like a good time.

I'm following #5051 now, so let me know how I can help. It's certainly got a lot of overlap with what I'm thinking here - once it's done, the change to avoid eager-extraction of wheels should be much simpler.

@pradyunsg
Copy link
Member

pradyunsg commented Dec 5, 2018

--no-binary says "Do not use binary packages" in it's docstring. Currently, doing a pip install --no-binary six six results in a setup.py install (without going through a wheel).

My understanding stems from the fact that it is that a commonly suggested way on Stackoverflow etc for cases where wheel based installation is to be avoided.

I think I've suggested it to people offline as well as a workaround for when wheels don't do what they want.

That said, I have no issues removing that code path. In fact, the idea behind #5051 came from an attempt to understand what be done to separate the build paths that use setup.py install to be able to treat them differently and removing them in a post PEP 517 world, is something I'm on board for.

@pradyunsg
Copy link
Member

pradyunsg commented Dec 5, 2018

@zooba I've started working on #5051 already -- I'd hate for us to duplicate effort here.

If you want to take up some parts/all of it, I have no issues deferring to you - just let's not do the exact same thing independently in slightly different ways. ;)

@pfmoore
Copy link
Member

pfmoore commented Dec 5, 2018

Currently, doing a pip install --no-binary six six results in a setup.py install (without going through a wheel).

If six had a pyproject.toml, we'd go via a wheel (using the PEP 517 backend). But you're correct that the "legacy" setuptools install path doesn't build a wheel and then install it. I don't consider that to be something that --no-binary guarantees, though. (If it were, then we would have needed to remove that guarantee when PEP 517 support landed).

@pradyunsg
Copy link
Member

pradyunsg commented Dec 5, 2018

Right. I've been thinking about it in terms of the most common use case I've seen for that functionality - the way you put it makes sense to me. The no-binary direction is probably a false alarm due to my (mis)interpretation of the option's effect.

@pradyunsg
Copy link
Member

pradyunsg commented Dec 5, 2018

FTR, IIUC, the setup.py install removal discussion has to happen in #5204.

bors bot added a commit to duckinator/emanate that referenced this issue Jul 30, 2020
153: Update pip to 20.2 r=duckinator a=pyup-bot


This PR updates [pip](https://pypi.org/project/pip) from **20.1.1** to **20.2**.



<details>
  <summary>Changelog</summary>
  
  
   ### 20.2
   ```
   =================

Deprecations and Removals
-------------------------

- Deprecate setup.py-based builds that do not generate an ``.egg-info`` directory. (`6998 &lt;https://github.com/pypa/pip/issues/6998&gt;`_, `8617 &lt;https://github.com/pypa/pip/issues/8617&gt;`_)
- Disallow passing install-location-related arguments in ``--install-options``. (`7309 &lt;https://github.com/pypa/pip/issues/7309&gt;`_)
- Add deprecation warning for invalid requirements format &quot;base&gt;=1.0[extra]&quot; (`8288 &lt;https://github.com/pypa/pip/issues/8288&gt;`_)
- Deprecate legacy setup.py install when building a wheel failed for source
  distributions without pyproject.toml (`8368 &lt;https://github.com/pypa/pip/issues/8368&gt;`_)
- Deprecate -b/--build/--build-dir/--build-directory. Its current behaviour is confusing
  and breaks in case different versions of the same distribution need to be built during
  the resolution process. Using the TMPDIR/TEMP/TMP environment variable, possibly
  combined with --no-clean covers known use cases. (`8372 &lt;https://github.com/pypa/pip/issues/8372&gt;`_)
- Remove undocumented and deprecated option ``--always-unzip`` (`8408 &lt;https://github.com/pypa/pip/issues/8408&gt;`_)

Features
--------

- Log debugging information about pip, in ``pip install --verbose``. (`3166 &lt;https://github.com/pypa/pip/issues/3166&gt;`_)
- Refine error messages to avoid showing Python tracebacks when an HTTP error occurs. (`5380 &lt;https://github.com/pypa/pip/issues/5380&gt;`_)
- Install wheel files directly instead of extracting them to a temp directory. (`6030 &lt;https://github.com/pypa/pip/issues/6030&gt;`_)
- Add a beta version of pip&#39;s next-generation dependency resolver.

  Move pip&#39;s new resolver into beta, remove the
  ``--unstable-feature=resolver`` flag, and enable the
  ``--use-feature=2020-resolver`` flag. The new resolver is
  significantly stricter and more consistent when it receives
  incompatible instructions, and reduces support for certain kinds of
  :ref:`Constraints Files`, so some workarounds and workflows may
  break. More details about how to test and migrate, and how to report
  issues, at :ref:`Resolver changes 2020` . Maintainers are preparing to
   ```
   
  
  
   ### 20.2b1
   ```
   ===================

Bug Fixes
---------

- Correctly treat wheels containing non-ASCII file contents so they can be
  installed on Windows. (`5712 &lt;https://github.com/pypa/pip/issues/5712&gt;`_)
- Prompt the user for password if the keyring backend doesn&#39;t return one (`7998 &lt;https://github.com/pypa/pip/issues/7998&gt;`_)

Improved Documentation
----------------------

- Add GitHub issue template for reporting when the dependency resolver fails (`8207 &lt;https://github.com/pypa/pip/issues/8207&gt;`_)
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/pip
  - Changelog: https://pyup.io/changelogs/pip/
  - Homepage: https://pip.pypa.io/
</details>



Co-authored-by: pyup-bot <github-bot@pyup.io>
bors bot added a commit to duckinator/emanate that referenced this issue Jul 31, 2020
153: Update pip to 20.2 r=duckinator a=pyup-bot


This PR updates [pip](https://pypi.org/project/pip) from **20.1.1** to **20.2**.



<details>
  <summary>Changelog</summary>
  
  
   ### 20.2
   ```
   =================

Deprecations and Removals
-------------------------

- Deprecate setup.py-based builds that do not generate an ``.egg-info`` directory. (`6998 &lt;https://github.com/pypa/pip/issues/6998&gt;`_, `8617 &lt;https://github.com/pypa/pip/issues/8617&gt;`_)
- Disallow passing install-location-related arguments in ``--install-options``. (`7309 &lt;https://github.com/pypa/pip/issues/7309&gt;`_)
- Add deprecation warning for invalid requirements format &quot;base&gt;=1.0[extra]&quot; (`8288 &lt;https://github.com/pypa/pip/issues/8288&gt;`_)
- Deprecate legacy setup.py install when building a wheel failed for source
  distributions without pyproject.toml (`8368 &lt;https://github.com/pypa/pip/issues/8368&gt;`_)
- Deprecate -b/--build/--build-dir/--build-directory. Its current behaviour is confusing
  and breaks in case different versions of the same distribution need to be built during
  the resolution process. Using the TMPDIR/TEMP/TMP environment variable, possibly
  combined with --no-clean covers known use cases. (`8372 &lt;https://github.com/pypa/pip/issues/8372&gt;`_)
- Remove undocumented and deprecated option ``--always-unzip`` (`8408 &lt;https://github.com/pypa/pip/issues/8408&gt;`_)

Features
--------

- Log debugging information about pip, in ``pip install --verbose``. (`3166 &lt;https://github.com/pypa/pip/issues/3166&gt;`_)
- Refine error messages to avoid showing Python tracebacks when an HTTP error occurs. (`5380 &lt;https://github.com/pypa/pip/issues/5380&gt;`_)
- Install wheel files directly instead of extracting them to a temp directory. (`6030 &lt;https://github.com/pypa/pip/issues/6030&gt;`_)
- Add a beta version of pip&#39;s next-generation dependency resolver.

  Move pip&#39;s new resolver into beta, remove the
  ``--unstable-feature=resolver`` flag, and enable the
  ``--use-feature=2020-resolver`` flag. The new resolver is
  significantly stricter and more consistent when it receives
  incompatible instructions, and reduces support for certain kinds of
  :ref:`Constraints Files`, so some workarounds and workflows may
  break. More details about how to test and migrate, and how to report
  issues, at :ref:`Resolver changes 2020` . Maintainers are preparing to
   ```
   
  
  
   ### 20.2b1
   ```
   ===================

Bug Fixes
---------

- Correctly treat wheels containing non-ASCII file contents so they can be
  installed on Windows. (`5712 &lt;https://github.com/pypa/pip/issues/5712&gt;`_)
- Prompt the user for password if the keyring backend doesn&#39;t return one (`7998 &lt;https://github.com/pypa/pip/issues/7998&gt;`_)

Improved Documentation
----------------------

- Add GitHub issue template for reporting when the dependency resolver fails (`8207 &lt;https://github.com/pypa/pip/issues/8207&gt;`_)
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/pip
  - Changelog: https://pyup.io/changelogs/pip/
  - Homepage: https://pip.pypa.io/
</details>



Co-authored-by: pyup-bot <github-bot@pyup.io>
Co-authored-by: Ellen Marie Dash <me@duckie.co>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C: wheel The wheel format and 'pip wheel' command type: enhancement Improvements to functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants