Skip to content

Keep symlinks as symlinks when installing packages from local src tree (or bundles) #1166

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
Closed
wants to merge 3 commits into from
Closed

Keep symlinks as symlinks when installing packages from local src tree (or bundles) #1166

wants to merge 3 commits into from

Conversation

myint
Copy link
Contributor

@myint myint commented Aug 24, 2013

This fixes #1006.

This is like pull request #1012, which has been sitting there for two months. But this time, I added a test. Shall we merge this now?

myint added 2 commits August 24, 2013 16:27
@myint
Keep symlinks as symlinks when installing
56ddabb 
Without this, "pip install" crashes when encountering symlinks that
point to external paths. The culprit is "shutil.copytree()", which
throws an error in such cases if the "symlinks" parameter is not set to
"True".

This fixes #1006.
@myint
Add test for #1006.
e6a7090
@myint myint mentioned this pull request Aug 24, 2013
Closed
@qwcode
Copy link
Contributor

qwcode commented Aug 25, 2013

changed the subject on this PR to be clearer about the scope.

qwcode
qwcode reviewed Aug 25, 2013
View reviewed changes
tests/data/packages/FSPkg/do_not_crash_on_symlink
@@ -0,0 +1 @@
/tmp
Copy link
Contributor

Choose a reason for hiding this comment

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

3 problems with this:

  1. this could result in one of our tests horribly copying all of /tmp in py2 (if copytree was set back to symlinks=False)
  2. this will fail on windows
  3. you said you were "adding a test". adding this link to an existing test package (for other tests), is not adding a specific test .

@myint
Copy link
Contributor Author

myint commented Aug 25, 2013

  1. You can switch it to .. if you want. Actually, even an invalid symlink triggers the bug, the link to /tmp could just be switched to point to non_existent.
  2. How will this fail on Windows? Won't it just ignore this? (The symlink is meant to be just some temporary file that happens to be in the directory. The .tox temporary files are an example.)
  3. Not sure what to tell you here.

@myint
Copy link
Contributor Author

myint commented Nov 12, 2013

Apparently this was already reported over a year go in #611. And it was fixed, but never merged, in #813.

@myint myint closed this Nov 12, 2013
@myint myint deleted the fix branch November 19, 2013 15:59
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
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.
2 participants
@myint @qwcode