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.

Sign up for GitHub

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

Jump to bottom

Ignore errors copying socket files for source installs. #6802

Closed
wants to merge 3 commits into from
Closed

Ignore errors copying socket files for source installs. #6802

wants to merge 3 commits into from

Conversation

chrahunt
Copy link
Member

Fixes #5306.

@chrahunt chrahunt force-pushed the bugfix/dont-copy-socket-files-from-source branch 2 times, most recently from 1e741e2 to dcc7948 Compare July 28, 2019 23:56
@chrahunt chrahunt marked this pull request as ready for review July 29, 2019 00:41
cjerdonek
cjerdonek reviewed Jul 29, 2019
View reviewed changes
Copy link
Member

@cjerdonek cjerdonek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple quick comments.

src/pip/_internal/download.py Show resolved Hide resolved
tests/unit/test_download.py Outdated Show resolved Hide resolved
@BrownTruck
Copy link
Contributor

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 be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Aug 6, 2019
@chrahunt chrahunt force-pushed the bugfix/dont-copy-socket-files-from-source branch from dcc7948 to b4d8b10 Compare August 7, 2019 01:42
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Aug 7, 2019
@pradyunsg
Copy link
Member

I'm okay with this.

@cjerdonek
Copy link
Member

This implementation would be a lot simpler and easier to understand using shutil.copytree()'s copy_function in Python 3, which I mentioned before. I'm still concerned about how this PR adds a lot more complexity to get the bug fixed in Python 2. (In two different places, the code needs to be iterating over a list of errors instead of just handling one error which copy_function would allow.) On other issues where supporting Python 2 meant adding complication, we were okay with restricting the fix to Python 3 so as not to complicate the code base unnecessarily. Especially since this bug doesn't seem common, I think it would be better to keep the code simpler.

@chrahunt
Copy link
Member Author

chrahunt commented Aug 7, 2019

I'll submit a separate PR for Python 3 only and we can pick one and close the other.

@cjerdonek
Copy link
Member

Okay, thanks a lot! 👍

@chrahunt chrahunt mentioned this pull request Aug 8, 2019
Merged
@chrahunt
Copy link
Member Author

chrahunt commented Aug 9, 2019

Going with Python 3 only approach.

@chrahunt chrahunt closed this Aug 9, 2019
@chrahunt chrahunt deleted the bugfix/dont-copy-socket-files-from-source branch August 9, 2019 04:18
@chrahunt chrahunt restored the bugfix/dont-copy-socket-files-from-source branch August 11, 2019 19:09
@chrahunt chrahunt deleted the bugfix/dont-copy-socket-files-from-source branch August 21, 2019 23:16
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Sep 20, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@cjerdonek cjerdonek cjerdonek left review comments

@pradyunsg pradyunsg pradyunsg approved these changes

Assignees
No one assigned
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pip install -U . fails when there's a UNIX domain socket in the current directory
5 participants
@chrahunt @BrownTruck @pradyunsg @cjerdonek @pypa-bot