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

pyenv-rehash: look for python* #379

Merged
merged 2 commits into from
Sep 22, 2015
Merged

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented May 18, 2015

This was rejected for rbenv, where it does not make much sense
(rbenv/rbenv#735).

Ref: #368 (comment)

@yyuu
Copy link
Contributor

yyuu commented Jun 26, 2015

@blueyed Could you explain the actual use case scenario of this?

@blueyed
Copy link
Contributor Author

blueyed commented Jun 26, 2015

@yyuu
The "use case" is that python2 and python3 get handled like python for this code path (PYENV_DIR gets set).

@yyuu
Copy link
Contributor

yyuu commented Jun 26, 2015

Idealistically all python, python2 and python3 should be handled evenly. Although, at least with current impl, python is the special name for executable in pyenv (and ruby for rbenv). Honestly I'm not sure what is the benefit of setting PYENV_DIR on invocation of python2 and python3 in addition to python....

@blueyed
Copy link
Contributor Author

blueyed commented Jun 26, 2015

python is the special name for executable in pyenv

So it's just for internal calls?
Then this PR is void - and I might add a comment instead. It wasn't really clear to me what this was doing. I've thought it would be triggered by calling python3 some.py, too.

@yyuu
Copy link
Contributor

yyuu commented Jun 26, 2015

Kind of internal call, or some kind of legacy code. I've just forgotten what that's for....

The PYENV_DIR will be read by ./libexec/pyenv and pyenv will once try to chdir to the directory if it is possible or not. However it will return to original cwd once it suceeded to chdir. So if I understand correctly it isn't so meaningful.

@blueyed
Copy link
Contributor Author

blueyed commented Jul 8, 2015

Re-opening. See #404 for where this is relevant.

@blueyed blueyed reopened this Jul 8, 2015
@blueyed blueyed force-pushed the pyenv-rehash-python-glob branch 2 times, most recently from dab4742 to fd8af9f Compare July 8, 2015 16:41
This is required for the shims to handle `#!/usr/bin/env python3` in a
shebang, just like `python` is handled currently: it will set
`PYENV_DIR` to the root of the invoked script, which is required for a
`.python-version` script to get picked up from there.

This was rejected for rbenv, where it does not make much sense
(rbenv/rbenv#735).

Ref: pyenv#368 (comment)
`PYENV_FILE_ARG` is used here to make use of `abs_dirname` later in
`libexec/pyenv`.

Fixes pyenv#404
@yyuu
Copy link
Contributor

yyuu commented Jul 12, 2015

Good catch 👍

I confirmed the behaviour reported on #404, and confirmed that this changes the behaviour to respect local version at symlink destination.

I believe your changes seem to be reasonable and useful for most cases, though it changes current behaviour slightly. Please add tests for this feature, then I'll merge this. (and, I think it's worth to reopen PR on rbenv as well)

@yyuu yyuu merged commit 493f036 into pyenv:master Sep 22, 2015
@yyuu
Copy link
Contributor

yyuu commented Sep 22, 2015

I added tests in 438e828 and merged into master. Thanks for PR.

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

Successfully merging this pull request may close these issues.

None yet

2 participants