powerlevel9k icon indicating copy to clipboard operation
powerlevel9k copied to clipboard

pyenv shows incorrect version

Open romkatv opened this issue 5 years ago • 8 comments

This is on master.

To reproduce:

  1. Install pyenv.
  2. Use master branch of Powerlevel9k with pyenv as one of the prompt segments.
  3. Type these commands:
pyenv install 3.7.3
export PYENV_VERSION=python-3.7.3

Expected behavior: pyenv segment shows 3.7.3 (the same as pyenv version-name).

Actual behavior: pyenv segment shows python-3.7.3.

romkatv avatar May 22 '19 09:05 romkatv

rbenv looks very similar to pyenv, I suppose it suffers the same issue...

Syphdias avatar May 22 '19 22:05 Syphdias

It's because PYENV_VERSION is detected firstly and used by powerlevel9k. And powerlevel9k just reflects what it get from PYENV_VERSION.

This is not a problem at all, cause you're doing things in the right way, try export PYENV_VERSION=3.7.3. From output of pyenv versions, there's no prefix for version names like 3.7.3, 2.7.16.

https://github.com/bhilburn/powerlevel9k/blob/3057e8fc07374e9c580790a023cb85998ca91fc8/powerlevel9k.zsh-theme#L1645-L1659


@Syphdias Your simplification slows down the virtual environment detection dramatically. Get python version from PYENV_VERSION is much faster than pyenv version-name

❯ time (pyenv version-name)
2.7.16
0.05s user 0.04s system 96% cpu 0.097 total

❯ time (echo $PYENV_VERSION)
2.7.16
0.00s user 0.00s system 59% cpu 0.001 total

pyenv version-name is just an implementation of the version detection logic:

  1. PYENV_VERSION
  2. .python-version
  3. ${PYENV_ROOT}/version

There's some overhead of pyenv version-name compared with checking PYENV_VERSION directly.

laggardkernel avatar Jun 21 '19 04:06 laggardkernel

Reading from the manual of pyenv, version name without python- prefix is the canonical way to use it.

PYENV_VERSION is exported by pyenv shell <venv-name> and pyenv activate <venv-name>, you'd better not touch it directly.

There's some usage examples in the test.

https://github.com/pyenv/pyenv/blob/d08c9cfb362c5a7e18a92acd2253a16935ad9a99/test/exec.bats#L17-L21

@test "fails with invalid version" {
  export PYENV_VERSION="3.4"
  run pyenv-exec python -V
  assert_failure "pyenv: version \`3.4' is not installed (set by PYENV_VERSION environment variable)"
}

laggardkernel avatar Jun 23 '19 16:06 laggardkernel

It's not my call, but if you opt for using PYENV_VERSION, it's safer to treat it the same way pyenv command treats it. If pyenv strips off python- prefix, you should also strip it.

Assumptions about what users do are bound to go wrong once your user base is large enough. In the limit, users do anything that can possibly do. If you are removing layers of abstractions for greater performance, you'll give your users maximum value by making your actions transparent so that they keep getting the same results. It's higher maintenance cost, of course.

P.S.

pyenv uses python- prefix in some places because it supports multiple python implementations (e.g., there is pypy in addition to regular python). python is the most popular implementation, so python- prefix is implied.

P.P.S.

As with all issues that I open, I don't have a stake in the outcome. If you think the code is working as intended, or that it's not worth fixing it, it won't upset me in any way.

romkatv avatar Jun 23 '19 16:06 romkatv

Dude, you should follow the manual of a command line tool. python-3.7.3 with prefix python- is not a canonical version name. pypy3.6-7.1.1 is another thing, don't mix theme together.

pyenv versions and pyenv install -l list the canonical name of each python version. There's no python- prefixed to 3.7.3. pyenv local python-3.7.3 and pyenv shell python-3.7.3 just throw out the following errs,

pyenv: version `python-3.7.3' not installed

# or
pyenv-virtualenv: version `python-3.7.3' is not a virtualenv

You're right. You can use PYENV_VERSION to switch virtual environment, but you need to follow the rule.

In fact, the venv-name within export PYENV_VERSION=<venv-name> and commands like pyenv activate <venv-name> reflects exactly the folder name, ${PYENV_ROOT}/versions/<venv-name>, of this very python.

If you couldn't bear to do things in the python- prefix way, stop arguing with me and create a feature request under the repo of pyenv. Otherwise, please follow the manual.

laggardkernel avatar Jun 23 '19 17:06 laggardkernel

Dude, you should follow the manual of a command line tool. python-3.7.3 with prefix python- is not a canonical version name. pypy3.6-7.1.1 is another thing, don't mix theme together.

I'm assuming your reply is directed to me but it's unclear how it's connected to my comment. Yes, python-3.7.3 is not a canonical name and pypy is not the same as python.

If you couldn't bear to do things in the python- prefix way, stop arguing with me and create a feature request under the repo of pyenv. Otherwise, please follow the manual.

I can see that you categorically disagree with something but I can't grasp what it is. I'll try to clarify what I wrote earlier. Perhaps it'll help.

pyenv treats PYENV_VERSION=python-3.7.3 identically to PYENV_VERSION=3.7.3. Every pyenv command behaves the same way. If I gave you a shell with PYENV_VERSION variable set to one of these two values, you wouldn't be able to tell which value it is without looking at the environment variable explicitly. pyenv versions, pyenv version-name and python --version would all produce the same results for PYENV_VERSION=python-3.7.3 and for PYENV_VERSION=3.7.3.

So far I've been making factual statements. If you see a mistake, please point it out. It'll be easy to sort out our facts.

Now comes opinion, not facts. First, the expected content of pyenv prompt when it is shown at all is the same as the output of pyenv version-name. Second, if you use something other than pyenv version-name in the implementation, it's a good idea to preserve the semantics so that the content of the prompt stays the same as before.

All the caveats of my not calling shots in p9k still apply. My opinion has no consequences here.

romkatv avatar Jun 23 '19 18:06 romkatv

What I categorically disagree with you is that PYENV_VERSION=python-3.7.3 with prefix python- is a wrong use. The manual/tutorial/wiki never tells you to do it like this. I don't understand why you choose to trust your imagination but not the manual.

I'll make it clear again. python- prefix is never official supported. Not only it's not supported in commands like pyenv shell python-3.7.3, but also the author never tried to use it like that.

Search with keyword PYENV_VERSION in the pyenv or the pyenv-virtual repo, you'll find the author never used it with prefix python-. I've been a python web developer for years, and I've never seen any of my colleague do it like that. Most of us learn the usage of pyenv from wiki or readme of pyenv, and don't think it possible cause it's not documented.

pyenv treats PYENV_VERSION=python-3.7.3 identically to PYENV_VERSION=3.7.3.

Are you sure? Try pyenv which python, it even doesn't give back the correct python location, which is an indirect proof that prefix python- is not supported.

I'll stop here. There's no need to keep arguing cause neither of us will be persuaded by the other. Maybe only the author of pyenv could explain why he strip prefix python- in codes of pyenv-version-name but never officially supported names with prefix python-.

laggardkernel avatar Jun 24 '19 02:06 laggardkernel

What I categorically disagree with you is that PYENV_VERSION=python-3.7.3 with prefix python- is a wrong use. The manual/tutorial/wiki never tells you to do it like this. I don't understand why you choose to trust your imagination but not the manual.

I'll make it clear again. python- prefix is never official supported. Not only it's not supported in commands like pyenv shell python-3.7.3, but also the author never tried to use it like that.

We are almost in complete agreement.

Yes to these:

  • The manual/tutorial/wiki never tells you to set PYENV_VERSION=python-3.7.3.
  • python- prefix is not mentioned in the documentation.
  • pyenv shell python-3.7.3 doesn't work. I thought it did, so this fact was new to me. More on this below.

I cannot say whether the author ever tried to set PYENV_VERSION=python-3.7.3 but I believe your argument doesn't break down if this statement turns out false.

pyenv treats PYENV_VERSION=python-3.7.3 identically to PYENV_VERSION=3.7.3.

Are you sure? Try pyenv which python, it even doesn't give back the correct python location

Thanks! This is useful.

... which is an indirect proof that prefix python- is not supported.

We are talking somewhat past each other here. I'm not claiming that python- is supported, or not "wrong use". It's irrelevant to my point.

I'll try to rephrase your position and would appreciate if you could say whether I'm completely off target.

  1. When pyenv segment in p9k is visible, it should display the same content as the output of pyenv version-name but only if pyenv is used according to the documentation.
  2. If the user sets PYENV_VERSION=python-3.7.3, they are engaging in "wrong use", and hence there is no reasonable expectation that pyenv segment displays $(pyenv version-name).

For the last and most important statement I have two variants. I'm not sure which one you are advocating for.

3a. If the user sets PYENV_VERSION=python-3.7.3, pyenv segment must display python-3.7.3.

or

3b. If the user sets PYENV_VERSION=python-3.7.3, pyenv segment may display 3.7.3 but only if doing so doesn't make prompt slower. It's better to display python-3.7.3 fast than to display 3.7.3 slow. Displaying 3.7.3 fast is allowed.

I'll stop here. There's no need to keep arguing cause neither of us will be persuaded by the other. Maybe only the author of pyenv could explain why he strip prefix python- in codes of pyenv-version-name but never officially supported names with prefix python-.

I've done some code archaeology. pyenv is a fork of rbenv that is still pulling all changes from its parent. Hence, pyenv handling of python- is identical to ruby- handling in rbenv. Consistency with upstream is sufficient justification for this.

Support for ruby- prefix was added to rbenv in https://github.com/rbenv/rbenv/pull/302. Originally rbenv was emitting a warning when this prefix was used but it was removed in https://github.com/rbenv/rbenv/pull/687. Later, the support for ruby- was broken in rbenv which with https://github.com/rbenv/rbenv/commit/6bb7f07d2d74aee3bb02421265bd28767e94dcce. The breakage looks like an accidental side effect rather than intended change.

I've sent https://github.com/rbenv/rbenv/pull/1168 to rbenv to restore ruby- prefix support in rbenv which and to add it to rbenv prefix (it's the only other command that doesn't support ruby- prefix; as far as I can tell, it never had this support). Some tests are failing but I don't want to spend time updating them until rbenv folks say whether this PR is in the right direction or not.

The two opinion statements I've expressed in https://github.com/bhilburn/powerlevel9k/issues/1281#issuecomment-504774567 don't depend on the outcome of this PR. I think it's best to keep the content of pyenv identical to $(pyenv version-name). Straying from this ideal should be done only if the benefits are great.

romkatv avatar Jun 24 '19 10:06 romkatv