Build local directories in place #7882

sbidoul · 2020-03-22T13:58:01Z

This is a rough experiment with in-place builds, to see the effect on the code base and tests, and advance the discussion in #7555.

sbidoul · 2020-03-28T19:24:14Z

This is getting in good shape.
We can see that quite a lot of code relating to copying the source directory can be removed.
Tests pass except on travis. I still need to check why (might be a transient).

As expected some tests had to be changed to cope with the .egg-info directory that gets created by setuptools in the source directory.

The only thing that stands out is this pip-egg-info directory that pip creates in the project directory to generate (temporary) egg-info metadata. I think it can be converted to a temporary directory to have that out of the way.

@pypa/pip-committers what do you think? Should I continue down this path?

uranusjr · 2020-03-28T21:17:47Z

The Travis failures are all the same, the worker has problems reaching PyPI for setup (it didn’t even reach the test phase).

pradyunsg · 2020-03-31T09:46:57Z

@sbidoul Sounds like a good idea! Still need to look at this PR, and hopefully by the time this gets updated, I'd have cleaned up my notifications situation significantly. :)

sbidoul · 2020-03-31T10:44:38Z

Note: maybe we should disable the copy while keeping the code around to facilitate an emergency revert in case this change raises problem for valid uses cases we'd have not anticipated. And then remove the dead code after 3 months.

pradyunsg · 2020-04-10T13:22:47Z

while keeping the code around

git revert is cheap to do, so removing is fine.

src/pip/_internal/operations/prepare.py

pradyunsg · 2020-04-10T18:13:27Z

@sbidoul Do you want this to be on the pip 20.1 release? I personally think it would reasonable to try to be on this train, since bundling this in the same release as the new resolver... might not be the best thing to do. OTOH, it might feel hurried to some but, hey, who knows. :)

Plus, this isn't a big "code change" but is a whole bunch of communication + community management discussion. We will have a beta period for 20.1, and it'd make a lot of sense to try to get feedback on this change as part of that IMO.

news/7555.removal

sbidoul · 2020-04-11T10:34:08Z

@pradyunsg ok, to get this out in 20.1. TIL there will be a beta period, so yes it's a good candidate to test the beta feedback process.

pradyunsg · 2020-04-11T11:14:33Z

TIL

It got announced today. :P

#7951 (comment)

pradyunsg

Implementation -- LGTM.
Documentation -- would be important to have for this IMO; NEWS fragment feels like it's compensating for the lack of docs. :/

pradyunsg · 2020-04-11T12:35:16Z

news/7555.removal

+Building of local directories is now done in place. Previously pip did copy the
+local directory tree to a temporary location before building. That approach had
+a number of drawbacks, among which performance issues, as well as various
+issues arising when the python project directory depends on its parent
+directory (such as the presence of a VCS directory). The user visible effect of
+this change is that secondary build artifacts, if any, may therefore be created
+in the local directory, whereas before they were created in a temporary copy of
+the directory and then deleted. This notably includes the ``build`` and
+``.egg-info`` directories in the case of the setuptools build backend.


This feels a bit excessive. I think we should only summarize the change here, add a section to the documentation and link to that from here.

@pradyunsg would you have a suggestion about a suitable place for this in the documentation?

https://pip.pypa.io/en/latest/reference/pip_install/#local-project-installs needs updating, so that would likely be the best place.

@pradyunsg thanks for the documentation pointer, I updated that section. Regarding the changelog I thought the breaking change warranted a bit more verbosity than usual.

src/pip/_internal/operations/prepare.py

Since we now build in place, pip install calls setup.py in place which in turn creates fake_pkg.egg-info. Since in this test the package we are installing is in script.temp_path, we must tell script to expect temporary files to be created.

This particular test checks which files are created. Since we now build in place, expect the .egg-info directory to be created.

docs/html/reference/pip_install.rst

Co-Authored-By: Noah <noah.bar.ilan@gmail.com>

pradyunsg · 2020-04-12T23:27:51Z

LGTM! I think I'll try to do another pass on the documentation prior to the release, but no promises.

Let's open the flood gates, AKA merge this PR! \o/

pradyunsg · 2020-04-13T16:04:27Z

Hurrah! Thanks for actually figuring out a small-patch-that's-actually-reasonable for doing this! ^.^

Thanks @NoahGorny for taking a look and for the reviews + inputs @xavfernandez @uranusjr! ^>^

sbidoul · 2020-04-13T21:24:56Z

Thanks @pradyunsg. I went ahead and closed #7555 and linked issues.

pradyunsg · 2020-04-14T09:33:51Z

Oh... I was waiting until after the beta release, to use those issues to get users to test the beta and report feedback on this change, instead of closing them now, which sorta indicates that "there's nothing more to do here".

I guess we can still post the "hey test this beta" messages posted there, but it might not have the same effect anymore. This makes things a bit more... tricky... in terms of communicating around this change. :/

sbidoul · 2020-04-14T09:50:18Z

Oh, sorry. I was under the impression that we were always closing issues as soon as the fixing PR was merged. I did not know you wanted to apply another process here.

Wrt the beta process, I personally have little doubt the above issues are resolved with this PR, so the beta process is probably not so much about verifying that. We probably need to focus on surfacing possible new issues that this change may create to users and use cases we don't know yet. And communicate clearly about potential impacts.

pradyunsg · 2020-04-14T10:34:28Z

Yea, it's not like you did anything inappropriate or anything along those lines.

I'm still figuring out how exactly to phrase the communication around this, which is basically why I hadn't actually commented that I'm thinking about using this approach. We learn more as we do more stuff, I guess. :P

Starting with pip 20.1b1+, the *-info subdir appears as a build artifact under the "local" directory (here, /project). See - pypa/pip#7555 - pypa/pip#7882 - pypa/pip@ace0c16

sbidoul force-pushed the build-in-place-7555-sbi branch 3 times, most recently from 61602fe to 6b6dee2 Compare March 28, 2020 18:26

sbidoul mentioned this pull request Apr 4, 2020

Generate legacy metadata in temporary directory #7978

Merged

sbidoul force-pushed the build-in-place-7555-sbi branch 2 times, most recently from 1969caa to 1cbbf58 Compare April 10, 2020 12:17

sbidoul force-pushed the build-in-place-7555-sbi branch from 1cbbf58 to 1a2a433 Compare April 10, 2020 13:32

sbidoul marked this pull request as ready for review April 10, 2020 13:35

sbidoul mentioned this pull request Apr 10, 2020

Prevent infinite recursion with pip wheel with $TMPDIR in $PWD #7873

Merged

pradyunsg reviewed Apr 10, 2020

View reviewed changes

src/pip/_internal/operations/prepare.py Show resolved Hide resolved

src/pip/_internal/operations/prepare.py Outdated Show resolved Hide resolved

xavfernandez approved these changes Apr 10, 2020

View reviewed changes

news/7555.removal Outdated Show resolved Hide resolved

sbidoul force-pushed the build-in-place-7555-sbi branch from 1a2a433 to 516e6c0 Compare April 11, 2020 10:21

sbidoul force-pushed the build-in-place-7555-sbi branch from 516e6c0 to 9fd3384 Compare April 11, 2020 10:40

pradyunsg added the type: enhancement Improvements to functionality label Apr 11, 2020

pradyunsg added this to the 20.1 milestone Apr 11, 2020

pradyunsg added the !release blocker Hold a release until this is resolved label Apr 11, 2020

pradyunsg reviewed Apr 11, 2020

View reviewed changes

sbidoul force-pushed the build-in-place-7555-sbi branch from 9fd3384 to c001102 Compare April 12, 2020 09:55

sbidoul added 4 commits April 12, 2020 12:00

build in place

88e6e6b

remove _copy_source_tree and friends

873f1e6

fix test_entrypoints_work test

ace0c16

Since we now build in place, pip install calls setup.py in place which in turn creates fake_pkg.egg-info. Since in this test the package we are installing is in script.temp_path, we must tell script to expect temporary files to be created.

fix test_uninstall_console_scripts

877e1cc

This particular test checks which files are created. Since we now build in place, expect the .egg-info directory to be created.

sbidoul force-pushed the build-in-place-7555-sbi branch from c001102 to 877e1cc Compare April 12, 2020 10:01

NoahGorny reviewed Apr 12, 2020

View reviewed changes

docs/html/reference/pip_install.rst Outdated Show resolved Hide resolved

Reword based on suggestion

9e02a1a

Co-Authored-By: Noah <noah.bar.ilan@gmail.com>

pradyunsg approved these changes Apr 12, 2020

View reviewed changes

pradyunsg merged commit f458573 into pypa:master Apr 13, 2020

sbidoul deleted the build-in-place-7555-sbi branch April 13, 2020 17:49

SanketDG mentioned this pull request Apr 14, 2020

Fix issue 7280 - issue warning after retry #7546

Closed

pradyunsg mentioned this pull request Apr 19, 2020

Improve documentation for in-place builds #8093

Merged

PythonCoderAS mentioned this pull request Apr 29, 2020

Pip update causes a local build directory creation praw-dev/praw#1456

Closed

gaborbernat mentioned this pull request Apr 29, 2020

Parallel safe local directory build #8168

Open

rpkilby mentioned this pull request May 5, 2020

Local installs now require write permissions #8196

Closed

sigmavirus24 mentioned this pull request May 6, 2020

Are twine maintainers OK to have a twine build command? pypa/twine#593

Closed

lock bot added the auto-locked Outdated issues that have been locked by automation label May 20, 2020

lock bot locked as resolved and limited conversation to collaborators May 20, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build local directories in place #7882

Build local directories in place #7882

sbidoul commented Mar 22, 2020

sbidoul commented Mar 28, 2020

uranusjr commented Mar 28, 2020

pradyunsg commented Mar 31, 2020

sbidoul commented Mar 31, 2020

pradyunsg commented Apr 10, 2020 •

edited

Loading

pradyunsg commented Apr 10, 2020

sbidoul commented Apr 11, 2020

pradyunsg commented Apr 11, 2020

pradyunsg left a comment

pradyunsg Apr 11, 2020

sbidoul Apr 11, 2020

pradyunsg Apr 11, 2020

sbidoul Apr 12, 2020

pradyunsg commented Apr 12, 2020

pradyunsg commented Apr 13, 2020

sbidoul commented Apr 13, 2020

pradyunsg commented Apr 14, 2020

sbidoul commented Apr 14, 2020 •

edited

Loading

pradyunsg commented Apr 14, 2020

Build local directories in place #7882

Build local directories in place #7882

Conversation

sbidoul commented Mar 22, 2020

sbidoul commented Mar 28, 2020

uranusjr commented Mar 28, 2020

pradyunsg commented Mar 31, 2020

sbidoul commented Mar 31, 2020

pradyunsg commented Apr 10, 2020 • edited Loading

pradyunsg commented Apr 10, 2020

sbidoul commented Apr 11, 2020

pradyunsg commented Apr 11, 2020

pradyunsg left a comment

Choose a reason for hiding this comment

pradyunsg Apr 11, 2020

Choose a reason for hiding this comment

sbidoul Apr 11, 2020

Choose a reason for hiding this comment

pradyunsg Apr 11, 2020

Choose a reason for hiding this comment

sbidoul Apr 12, 2020

Choose a reason for hiding this comment

pradyunsg commented Apr 12, 2020

pradyunsg commented Apr 13, 2020

sbidoul commented Apr 13, 2020

pradyunsg commented Apr 14, 2020

sbidoul commented Apr 14, 2020 • edited Loading

pradyunsg commented Apr 14, 2020

pradyunsg commented Apr 10, 2020 •

edited

Loading

sbidoul commented Apr 14, 2020 •

edited

Loading