Rebase #4501 to resolve merge conflicts #4562

alex · 2017-06-22T21:36:39Z

takluyver · 2017-06-23T09:49:31Z

Thanks @alex for picking this up :-)

alex · 2017-06-23T11:49:48Z

@takluyver No problem! I'm very excited about pip 10, so I'm trying to help as I can :-)

pradyunsg · 2017-06-26T09:23:43Z

pip/cache.py

@@ -27,45 +29,61 @@ def __init__(self, cache_dir, format_control):
            binaries being read from the cache.
        """
        self._cache_dir = expanduser(cache_dir) if cache_dir else None
+        # Ephemeral cache: store wheels just for this run
+        self._ephem_cache_dir = tempfile.mkdtemp(suffix='-pip-ephem-cache')


Please use pip.utils.temp_dir.TempDirectory, instead of this. :)

alex · 2017-06-26T11:19:35Z

@pradyunsg done.

pradyunsg · 2017-06-26T11:41:26Z

pip/wheel.py

@@ -591,6 +591,8 @@ def __init__(self, requirement_set, finder, build_options=None,
        self.requirement_set = requirement_set
        self.finder = finder
        self._cache_root = requirement_set._wheel_cache._cache_dir
+        self._ephem_cache_root = (
+            requirement_set._wheel_cache._ephem_cache_dir.path)


This looks pretty bad. 😕

I guess this can stay for now because I would be moving caches around as a part of my GSoC project, so, all this would likely change that time anyway.

BrownTruck · 2017-06-28T08:30:05Z

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will eligible for code review and hopefully merging!

pradyunsg · 2017-06-28T08:55:13Z

I've added this to the 10.0 milestone, in exchange of #4501, since this is basically that PR taken forward.

pradyunsg · 2017-07-02T10:28:20Z

pip/commands/install.py

                try:
+                    self.populate_requirement_set(


I don't think that this change is needed - populate_requirement_set moving inside the try.

Am I missing something?

I think there was a test failure because of temporary files not getting cleaned up; it's in the try block so that the finally fires on failure and calls the cleanup. I don't remember the details, though, and I might have confused it with something else.

I'll try moving it out of try and let you know what happens.

Seems like it's needed - https://travis-ci.org/pradyunsg/pip/jobs/249302943.

This isn't a blocker then for this PR - if someone would be able to explain to me why this is needed though it'd be nice.

AssertionError: temp files not expected

I think it's like this:

Before the code in question, a temporary directory has been created to store ephemeral wheels. This is new in this PR.

The tests you see failing are those which exercise an error case, e.g. no requirements specified.

If self.populate_requirement_set(...) fails, we want to call the cleanup code to remove that temporary dir.

The cleanup code is in the finally block, so we need the step that can fail to be in the try.

The change in this PR is that a temporary directory is created before this; previously it only created temporary files later, so there was no need for cleanup if the earlier stage failed.

Ah, Okie... Thanks @takluyver for the explanation!

I haven't really looked into this PR, I've just been skimming over it for the most part.

Just one more question - the wheels are still being built after the population of the RequirementSet, correct?

Yes, it's still the WheelBuilder a few lines below that builds wheels for the requirement set. The crucial difference is that previously it only built wheels if they could be cached (e.g. if we've downloaded an sdist, so the wheel corresponds to a particular version). Now it also builds ephemeral wheels that won't be cached, e.g. if you install from a local directory.

Cool. I thought so too but just felt like confirming. :)

Thanks again!

Follow up from pypagh-4144 To allow build system abstractions, we want installation to go through wheels in more cases. In particular, installing packages from a local directory or a VCS URL currently runs 'setup.py install'. The aim of this PR is to have it build a wheel, which is stored in an ephemeral cache directory, used for installation, and then discarded. We can't cache it permanently based on the path/URL, because the code there might change, but our cache wouldn't be invalidated.

When there's a previous build directory, no_clean is set

alex · 2017-07-06T00:58:20Z

@dstufft woudl it be helpful if I split some of the conflict prone bits into their own PR?

pradyunsg · 2017-07-06T08:15:12Z

this has a very high conflict rate with the word @pradyunsg is doing

Sorry!

I'm actually refactoring the cache rn... There's probably more PRs I'm gonna make that'll conflict with this PR, after that I'll probably stop moving stuff around so much and start building on new things that got added. If it's fine by you to wait a week or so and then fix the conflicts, I'll happily ping here once the refactor work is done. :)

pradyunsg · 2017-07-19T10:26:32Z

@alex Once #4623 merges, I think you can use the API that's used in it.

You can create a new WheelCache with the temporary directory as the cache_dir and use that object for the caching work.

Additionally, I don't intend to change the Cache API in any other PRs.

pradyunsg · 2017-07-19T10:36:05Z

@alex If you want, I'll be happy to merge master in this branch to fix the conflicts; after #4623 merges. :)

alex · 2017-07-20T14:07:21Z

@pradyunsg if you'd like to take over this PR, that'd be great.

pradyunsg · 2017-07-21T07:37:01Z

@pradyunsg if you'd like to take over this PR, that'd be great.

Sure! I'll happily take it ahead from here. :)

astrojuanlu · 2017-07-31T08:38:56Z

Any ETA, @pradyunsg? I am as excited about pip 10 as anyone else :)

pradyunsg · 2017-07-31T13:48:38Z

Any ETA, @pradyunsg?

I'm working on another issue in pip rn... Once that is done, this is next in the stack. Alas, that means not within the next 3 weeks, at least.

pfmoore · 2017-10-04T09:00:40Z

Hi what's the current status on this? @pradyunsg are you working on this? My understanding is that this is needed both for its own sake, and to complete PEP 518 support (because pip install . currently goes direct via setup.py and so bypasses pip's implementation of the PEP).

Is someone in a position to fix the merge conflicts? I'm willing to review the PR once that's done (although I'll admit I'm pretty unclear as to what's landed and what's in-progress in this area - @pradyunsg mentions using the new cache API from #4623, I assume that's not yet been incorporated here?)

pradyunsg · 2017-10-04T11:07:33Z

@pradyunsg are you working on this?

Right now actually.

pradyunsg · 2017-10-04T13:34:00Z

@pfmoore I ~~just~~ made #4764.

It didn't take too much time to do. I'm kind of happy about the new Cache API now.

pfmoore · 2017-10-04T13:47:49Z

Cool - I'll try to review that this evening.

ghost · 2017-11-14T01:37:30Z

@pradyunsg Please close.

pradyunsg · 2017-11-14T05:40:29Z

Closing since this has been carried forward in #4764 and that links back here. :)

lock · 2019-06-02T16:05:41Z

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

alex mentioned this pull request Jun 22, 2017

Try to build wheels for directories/VCS #4501

Closed

pradyunsg reviewed Jun 26, 2017

View reviewed changes

pradyunsg added the type: enhancement Improvements to functionality label Jun 27, 2017

BrownTruck added the needs rebase or merge PR has conflicts with current master label Jun 28, 2017

pradyunsg added this to the 10.0 milestone Jun 28, 2017

alex force-pushed the ephem-cache-wheels branch from 8dfd676 to 168fe32 Compare June 28, 2017 14:12

pypa-bot removed the needs rebase or merge PR has conflicts with current master label Jun 28, 2017

alex force-pushed the ephem-cache-wheels branch from d0f2ee7 to 277d9b8 Compare June 29, 2017 03:18

rgommers mentioned this pull request Jun 29, 2017

Comply with PEP 518. scipy/scipy#7309

Merged

pradyunsg reviewed Jul 2, 2017

View reviewed changes

takluyver added 14 commits July 5, 2017 20:04

Add news file

a32ebea

Fix list comp

fe5e9cc

Code style fixes

30109c3

Allow temp files for another test

ee67867

When there's a previous build directory, no_clean is set

Fix cleanup on 'pip install' with no args

46c2c54

Cleanup wheel cache after 'pip freeze'

9fdcfaf

Expect temp files in another test

9109322

Update test of install output

ad242d1

Expect temp files on another test

a51843a

Update another install output test

73440ed

Update test to check for dist-info

5610636

Remove string formatting operation

1c03c1b

Install 'wheel' for test that now requires it

ad50ac9

fix

5f88252

alex mentioned this pull request Jul 6, 2017

Move get_cache_path_for_link to WheelCache #4603

Merged

segevfiner mentioned this pull request Jul 8, 2017

pip should always install dist-info rather than egg-info when possible #4611

Closed

pradyunsg added the needs rebase or merge PR has conflicts with current master label Jul 19, 2017

pradyunsg self-assigned this Jul 21, 2017

astrojuanlu mentioned this pull request Jul 24, 2017

Clarify installation process poliastro/poliastro#187

Closed

pradyunsg mentioned this pull request Aug 6, 2017

PEP 518 support: setup.py is called before build requirements have been installed #4647

Closed

pradyunsg added the !release blocker Hold a release until this is resolved label Aug 18, 2017

ghost mentioned this pull request Sep 7, 2017

try to build wheels for directories/vcs #4714

Closed

pradyunsg mentioned this pull request Oct 4, 2017

Do wheel installs from VCS/directories using ephemeral caching #4764

Merged

pradyunsg removed this from the 10.0 milestone Oct 5, 2017

pradyunsg removed the !release blocker Hold a release until this is resolved label Oct 22, 2017

pradyunsg closed this Nov 14, 2017

lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 2, 2019

lock bot locked as resolved and limited conversation to collaborators Jun 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rebase #4501 to resolve merge conflicts #4562

Rebase #4501 to resolve merge conflicts #4562

alex commented Jun 22, 2017 •

edited

Loading

takluyver commented Jun 23, 2017

alex commented Jun 23, 2017

pradyunsg Jun 26, 2017 •

edited

Loading

alex commented Jun 26, 2017

pradyunsg Jun 26, 2017

BrownTruck commented Jun 28, 2017

pradyunsg commented Jun 28, 2017

pradyunsg Jul 2, 2017

takluyver Jul 2, 2017

pradyunsg Jul 2, 2017

pradyunsg Jul 2, 2017

takluyver Jul 2, 2017

pradyunsg Jul 2, 2017

pradyunsg Jul 2, 2017

takluyver Jul 2, 2017

pradyunsg Jul 3, 2017

alex commented Jul 6, 2017

pradyunsg commented Jul 6, 2017 •

edited

Loading

pradyunsg commented Jul 19, 2017

pradyunsg commented Jul 19, 2017

alex commented Jul 20, 2017

pradyunsg commented Jul 21, 2017

astrojuanlu commented Jul 31, 2017

pradyunsg commented Jul 31, 2017

pfmoore commented Oct 4, 2017

pradyunsg commented Oct 4, 2017

pradyunsg commented Oct 4, 2017 •

edited

Loading

pfmoore commented Oct 4, 2017

ghost commented Nov 14, 2017

pradyunsg commented Nov 14, 2017

lock bot commented Jun 2, 2019

Rebase #4501 to resolve merge conflicts #4562

Rebase #4501 to resolve merge conflicts #4562

Conversation

alex commented Jun 22, 2017 • edited Loading

takluyver commented Jun 23, 2017

alex commented Jun 23, 2017

pradyunsg Jun 26, 2017 • edited Loading

Choose a reason for hiding this comment

alex commented Jun 26, 2017

Choose a reason for hiding this comment

BrownTruck commented Jun 28, 2017

pradyunsg commented Jun 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alex commented Jul 6, 2017

pradyunsg commented Jul 6, 2017 • edited Loading

pradyunsg commented Jul 19, 2017

pradyunsg commented Jul 19, 2017

alex commented Jul 20, 2017

pradyunsg commented Jul 21, 2017

astrojuanlu commented Jul 31, 2017

pradyunsg commented Jul 31, 2017

pfmoore commented Oct 4, 2017

pradyunsg commented Oct 4, 2017

pradyunsg commented Oct 4, 2017 • edited Loading

pfmoore commented Oct 4, 2017

ghost commented Nov 14, 2017

pradyunsg commented Nov 14, 2017

lock bot commented Jun 2, 2019

alex commented Jun 22, 2017 •

edited

Loading

pradyunsg Jun 26, 2017 •

edited

Loading

pradyunsg commented Jul 6, 2017 •

edited

Loading

pradyunsg commented Oct 4, 2017 •

edited

Loading