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.

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

Create proxies for pip, wheel, etc. to get around problems with long paths #1004

Conversation

HaraldNordgren
Copy link

@HaraldNordgren HaraldNordgren commented Jan 9, 2017

This will move pip to .pip after installation, then creates a new pip file containing

#!/usr/bin/env sh
"/path/to/env/bin/python" "/path/to/env/bin/.pip" $@

This eliminates the problems with long paths that stem from limitations of the shebang on Unix. This should also solve the problems with spaces in path names.

(See #997 for more discussion)

@HaraldNordgren
Copy link
Author

Related to #994

os.rename(script, hidden_destination)

with open(script, "w") as script_proxy:
text = "#!/usr/bin/env sh"
Copy link
Member

Choose a reason for hiding this comment

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

Should this not force an appropriate encoding? What if the venv name contains non-ASCII characters?

@yan12125
Copy link

yan12125 commented Jan 9, 2017

The change should be in distlib instead of virtualenv.

$ patch -p1 -i 1004.patch 
patching file virtualenv.py
$ python virtualenv.py "foo bar"
Using base prefix '/usr'
New python executable in /home/yen/Projects/virtualenv/foo bar/bin/python
Installing setuptools, pip, wheel...done.
$ source ./foo\ bar/bin/activate
(foo bar) $ pip --version   
pip 9.0.1 from /home/yen/Projects/virtualenv/foo bar/lib/python3.6/site-packages (python 3.6)
(foo bar) $ pip install nose     
Collecting nose
  Using cached nose-1.3.7-py3-none-any.whl
Installing collected packages: nose
Successfully installed nose-1.3.7
(foo bar) $ nosetests
zsh: /home/yen/Projects/virtualenv/foo bar/bin/nosetests: bad interpreter: "/home/yen/Projects/virtualenv/foo: no such file or directory

@HaraldNordgren
Copy link
Author

HaraldNordgren commented Jan 9, 2017

@yan12125 Oh, this is not applied to packages installed by the new virtualenv. That's bad.

Either I extend the code to do the replacement on every installed executable starting with a shebang. Or we find some way to put the code deeper into the system like distlib.

@pfmoore
Copy link
Member

pfmoore commented Jan 9, 2017

@HaraldNordgren I assumed that it was a deliberate to only target the wrappers virtualenv itself installs. If it wasn't, then yes it would need to go into distlib at a minimum so that anything installed by pip would be handled. And technically into any other tool that people might use to install wrappers stuff into the virtualenv (like easy_install/setuptools). Ultimately, as it's a workaround for an OS limitation, it's up to you how far you want to go patching the various tools.

@HaraldNordgren
Copy link
Author

HaraldNordgren commented Jan 15, 2017

@yan12125 @pfmoore

I created a pull request for the corresponding changes in distlib: https://bitbucket.org/pypa/distlib/pull-requests/31/create-proxy-files-pip-pip-to-get-around/diff.

For development, I modified virtualenv/virtualenv.py with

    SCRIPT = textwrap.dedent("""
        import sys
        import pkgutil
        import tempfile
        import os

        import sys
        sys.path.insert(0, '/home/harald/git-repos/pip')

        import pip
        ...

to get virtualenv to run my custom pip with this symlink

~/git-repos/pip/pip/_vendor/distlib -> ~/git-repos/distlib/distlib/

And after creating and activating the env, I also run this in the terminal

export PYTHONPATH=/home/harald/git-repos/pip

before trying for example pip install flask.

Hopefully this won't have to be done in production, if we can get these changes into an actual release on distlib.

@pfmoore
Copy link
Member

pfmoore commented Jan 15, 2017

@HaraldNordgren Did you test your PR on Windows? Because I saw it and had a quick look, and it doesn't look like it limits the change to Windows. I'd comment on the PR itself, but I don't have access to bitbucket right at the moment,,,

@HaraldNordgren
Copy link
Author

HaraldNordgren commented Jan 15, 2017

It has not been tested on Windows, only Ubuntu 16.04.

I was hoping that the use_launcher logic (https://bitbucket.org/pypa/distlib/src/f702f6afc1d03717fa88a742b9fb99bedd599ff2/distlib/scripts.py#scripts.py-206) would isolate the change to Unix.

@pfmoore
Copy link
Member

pfmoore commented Jan 15, 2017

Hmm, OK. Someone will have to test on Windows, I guess.

@HaraldNordgren
Copy link
Author

HaraldNordgren commented Jan 15, 2017

Maybe here is an even better solution, based on an idea from a Mach bootstrap script:
https://bitbucket.org/pypa/distlib/pull-requests/32
https://bitbucket.org/pypa/distlib/pull-requests/33

@pfmoore
Copy link
Member

pfmoore commented Jan 16, 2017

IMO, this is an immensely complex (and hence difficult to maintain) workaround to an OS limitation of certain Unix systems, that comes up relatively rarely. While I agree that it fails in a pretty unpleasant way, and it would be good to do something, I'd rather any workaround be limited to cases where it's needed - which is of course hard to determine precisely because it's an OS issue rather than a pip/distlib/virtualenv issue.

On the other hand, I don't have any direct interest in the solution, as I am a Windows user and this issue doesn't occur on Windows, so if the Unix developers are happy with the proposed solution, I won't object (as long as it is only a Unix solution, and Windows behaviour remains as it currently is).

@HaraldNordgren
Copy link
Author

HaraldNordgren commented Jan 16, 2017

I think you have a point that we could limit the fix to the situations where it breaks. I tried to change the Bitbucket PR to achieve that. So only pad the shebang when it would otherwise break. That also highlights the need for the PR -- without it distlib and virtualenv won't work at all.

Since we are dealing with shebangs, I am fairly certain that it won't affect Windows.

@yan12125
Copy link

Since we are dealing with shebangs, I am fairly certain that it won't affect Windows.

Just FYI: py.exe on Windows (PC\launcher.c) uses shebangs.

@HaraldNordgren
Copy link
Author

HaraldNordgren commented Jan 16, 2017

Ok, good to know.

I'll keep working on this. I don't think Vinay Sajip at Distlib will accept the change there, but he pointed me to ScriptMaker.exectuable which should be able to achieve the same thing. So I submitted another pull reuqest for pip instead: pypa/pip#4237

@HaraldNordgren HaraldNordgren deleted the master_create_proxy_binaries branch September 18, 2018 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants