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

Do not use $PWD in installation proccess #381

Closed
turicas opened this Issue Nov 5, 2011 · 5 comments

Comments

Projects
None yet
5 participants
@turicas

turicas commented Nov 5, 2011

Please see this gist:
https://gist.github.com/1340858

When I don't use the -i option of sudo, I receive an error because pip installation process is trying to write in /root (pwd).

So, the problem is: pip is trying to write something to pwd assuming that it is writable. pip should not assume it and use /tmp, $HOME or something like that.

Note: thanks to @wraithan helping me find the problem (I was trying to create a virtualenv using fabric when this problem appeared).

@turicas

This comment has been minimized.

Show comment
Hide comment
@turicas

turicas Nov 11, 2011

Talking with @hugobr we found that the bug is on lines 22 and 23 at pip/locations.py.

turicas commented Nov 11, 2011

Talking with @hugobr we found that the bug is on lines 22 and 23 at pip/locations.py.

hltbra added a commit that referenced this issue Nov 25, 2011

Fix issue #381 - Do not use $PWD in installation proccess
The folder `build` and `src` are under is found by following the rules:
  * use sys.prefix if is under virtualenv
  * if not, try using the current working dir
  * if does not have write permission to current dir, use a temporary folder
@hltbra

This comment has been minimized.

Show comment
Hide comment
@hltbra

hltbra Nov 25, 2011

Member

By now I have created a fix to it in a branch. Want to see some opinons before merging into develop branch.

Check the diff here: 8d7d8a6

Member

hltbra commented Nov 25, 2011

By now I have created a fix to it in a branch. Want to see some opinons before merging into develop branch.

Check the diff here: 8d7d8a6

@Schuk

This comment has been minimized.

Show comment
Hide comment
@Schuk

Schuk Jan 10, 2012

+1 pip shouldnt ever expect to use pwd/cwd for building or storing any files. It might write in directories which possibly breaks things

failed installs leave data within the directory including $PWD/pip-log.txt. This should also be kept within a tmp or dedicated directory.

also I do not agree with the proposed fix for only checking if cwd is writable. pip working dir should either be tempfile.mkdtemp() or $HOME/.python-pip/modulename

Schuk commented Jan 10, 2012

+1 pip shouldnt ever expect to use pwd/cwd for building or storing any files. It might write in directories which possibly breaks things

failed installs leave data within the directory including $PWD/pip-log.txt. This should also be kept within a tmp or dedicated directory.

also I do not agree with the proposed fix for only checking if cwd is writable. pip working dir should either be tempfile.mkdtemp() or $HOME/.python-pip/modulename

@turicas

This comment has been minimized.

Show comment
Hide comment
@turicas

turicas Jan 10, 2012

I think pip should use tempfile for download/extract and $HOME for logging problems.

turicas commented Jan 10, 2012

I think pip should use tempfile for download/extract and $HOME for logging problems.

@TC01

This comment has been minimized.

Show comment
Hide comment
@TC01

TC01 Apr 23, 2012

Contributor

I decided to tackle this bug, since as commented above, the proposed patch still attempts to write to pwd/cwd if it is writeable.

However, in doing so, there's another issue I discovered- before, locations.py would not actually create the build folder, only define the name. The actual folder would be created later in the process, and that is when it would be marked for deletion. If the folder already exists (which it will, now that locations.py / tempfile is creating it previously), it will not be marked for delete.

This would apply to the already posted patch as well- you would get a lot of temporary folders floating around in /tmp that will never be automatically deleted.

Contributor

TC01 commented Apr 23, 2012

I decided to tackle this bug, since as commented above, the proposed patch still attempts to write to pwd/cwd if it is writeable.

However, in doing so, there's another issue I discovered- before, locations.py would not actually create the build folder, only define the name. The actual folder would be created later in the process, and that is when it would be marked for deletion. If the folder already exists (which it will, now that locations.py / tempfile is creating it previously), it will not be marked for delete.

This would apply to the already posted patch as well- you would get a lot of temporary folders floating around in /tmp that will never be automatically deleted.

pnasrat added a commit that referenced this issue May 31, 2012

Merge pull request #516 from TC01/develop
Temporary build and src directories no longer created in cwd/pwd (bugs #339, #381)

@pnasrat pnasrat closed this May 31, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment