Virtualenvwrapper stopped working after d686d

[jeffwidman]

Before d686da3c039a1ed0ae271860d271ec966f59c16a, virtualenvwrapper shell scripts were sourced properly, the workon etc commands were on my path, etc.

After that commit, the shell script doesn't get loaded for me at all.

I'm still debugging what specifically changed, but clearly it is something in that commit.

[belak]

I don't have prezto installed at the moment, but there's a chance it's related to a workaround I added a while back which got removed in that commit: https://github.com/sorin-ionescu/prezto/commit/d686da3c039a1ed0ae271860d271ec966f59c16a#diff-414db2cc4cb0c69d0fea2d6304ceb9921bf9d0d96f8d47f545e19bfa217dc39dL120-R119

[belak]

Are you seeing the same problem as in https://github.com/sorin-ionescu/prezto/issues/1416, or is it a separate issue?

If it is that issue, this patch, should re-add that hack:

diff --git a/modules/python/init.zsh b/modules/python/init.zsh
index 91aa6d0..95bb992 100644
--- a/modules/python/init.zsh
+++ b/modules/python/init.zsh
@@ -123,10 +123,19 @@ if (( $+VIRTUALENVWRAPPER_VIRTUALENV || $+commands[virtualenv] )) \
       pyenv "$pyenv_plugins[(R)virtualenvwrapper(_lazy|)]"
     fi
   else
-    # Fallback to 'virtualenvwrapper' without 'pyenv' wrapper if 'python' is
-    # available in '$path'.
-    if (( ! $+VIRTUALENVWRAPPER_PYTHON )) && (( $#commands[(i)python[23]#] )); then
-      VIRTUALENVWRAPPER_PYTHON=$commands[(i)python[23]#]
+    # Fallback to 'virtualenvwrapper' without 'pyenv' wrapper if available
+    # in '$path' or an alternative location on a Debian based system.
+
+    # If homebrew is installed and the python location wasn't overridden via
+    # environment variable we fall back to python3 then python2 in that order.
+    # This is needed to fix an issue with virtualenvwrapper as homebrew no
+    # longer shadows the system python.
+    if (( ! $+VIRTUALENVWRAPPER_PYTHON )) && (( $+commands[brew] )); then
+      if (( $+commands[python3] )); then
+        export VIRTUALENVWRAPPER_PYTHON=$commands[python3]
+      elif (( $+commands[python2] )); then
+        export VIRTUALENVWRAPPER_PYTHON=$commands[python2]
+      fi
     fi
 
     virtenv_sources=(

[jeffwidman]

It's this line: https://github.com/sorin-ionescu/prezto/commit/d686da3c039a1ed0ae271860d271ec966f59c16a#diff-414db2cc4cb0c69d0fea2d6304ceb9921bf9d0d96f8d47f545e19bfa217dc39dR37

Specifically, the chang from $+commands[python] to $#commands[(i)python[23]#] that causes the problem.

Adding basic echo statements and the new version doesn't catch python so it returns early. Without that return, it works as expected.

For me both python and python3 are found in my path, but not python2. Not sure if that matters.

The change is a little cryptic to me, so trying to figure it out.

[jeffwidman]

Are you seeing the same problem as in #1416, or is it a separate issue? If it is that issue, this patch, should re-add that hack...

I suspected the same bit of code at first, but as best I can tell, that doesn't matter. If I comment out the one line I mentioned above, everything else appears to work as expected.

[jeffwidman]

Non-working:

• $#commands[(i)python[23]#]
• $commands[(i)python[23]]

Working:

• $+commands[(i)python[23]#]
• $#commands[(i)python[23]]
• $+commands[(i)python[23]]

So it's related to the #. I'm not sure what they do though... checking the docs, but if anyone knows off top of their head, feel free to chime in.

[jeffwidman]

I did some reading, and still not totally clear, but that may be because it's an hour past normal bed time. I'll re-read in the morning. @indrajitr if you happy to know, I'd appreciate any pointers.

Also, based on the above working/non-working, this other lines may need tweaking too: https://github.com/sorin-ionescu/prezto/commit/d686da3c039a1ed0ae271860d271ec966f59c16a#diff-414db2cc4cb0c69d0fea2d6304ceb9921bf9d0d96f8d47f545e19bfa217dc39dR118-R119

[belak]

Yep, I've spent a good hour on this and I still don't understand what it's trying to do - this is precisely why I don't like zsh parameter expansion. The docs are really hard to understand, especially when # has something like 4 or 5 separate meanings.

[indrajitr]

Non-working:

• $#commands[(i)python[23]#]
• $commands[(i)python[23]]

Working:

• $+commands[(i)python[23]#]
• $#commands[(i)python[23]]
• $+commands[(i)python[23]]

So it's related to the #. I'm not sure what they do though... checking the docs, but if anyone knows off top of their head, feel free to chime in.

The details should be mostly in Zsh Parameter Expansion doc. But here is the TL;DR:

• $commands[(i)python[23]] gives you the first key from among all the keys of the commands associative array that match the patterns python[23]. If you want to see all the keys, try $commands[(I)python[23]] (notice uppercase I).
• $#commands[(i)python[23]] is like above, except it gives you the count instead of the actual key. Again (i) is for the first match and (I) is for all the matches.
• $+commands[(i)python[23]] just returns 1 if there is a match, else 0.
• The extra # in the array suffix is for completeness to capture python in addition to python2 and python3.

Relevant shell examples:

~ ❯❯❯ echo $commands[(i)python[23]]
python2
~ ❯❯❯ echo $#commands[(i)python[23]]
1
~ ❯❯❯ echo $+commands[(i)python[23]]
1
~ ❯❯❯ echo $commands[(I)python[23]]
python2 python3
~ ❯❯❯ echo $#commands[(I)python[23]]
2
~ ❯❯❯ echo $+commands[(I)python[23]]
1
~ ❯❯❯ echo $commands[(I)python[23]#]
python2 python3 python
~ ❯❯❯ echo $#commands[(I)python[23]#]
3
~ ❯❯❯ echo $+commands[(I)python[23]#]
1

So as you can see, having # in the array suffix is preferred. So that python[23]# captures all the variations python python2 python3.

Switching from $#commands[(i)python[23]#] to $+commands[(i)python[23]#] shouldn't really matter in this specific case, but it would be important to get to the bottom of this.

@jeffwidman can I request you to share the output of the following commands?

• echo $commands[(I)python[23]#]
• echo $#commands[(I)python[23]#]
• echo $+commands[(I)python[23]#]
• for k v in ${(kv)commands}; do echo "$k => $v"; done | grep python

[jeffwidman]

Sorry for the delay here, been on vacation in the mountains.

@indrajitr thanks for the detailed explanation!

Here's the output as requested:

~ ❯❯❯ echo $commands[(I)python[23]#]
python3 python
~ ❯❯❯ echo $#commands[(I)python[23]#]
2
~ ❯❯❯ echo $+commands[(I)python[23]#]
1
~ ❯❯❯ for k v in ${(kv)commands}; do echo "$k => $v"; done | grep python
pythonw2.7 => /usr/bin/pythonw2.7
python2.7 => /usr/bin/python2.7
python-config => /usr/bin/python-config
python3.9-config => /usr/local/bin/python3.9-config
python3 => /usr/local/bin/python3
pythonw => /usr/bin/pythonw
python3-config => /usr/local/bin/python3-config
python3.9 => /usr/local/bin/python3.9
python => /usr/bin/python
ipython => /usr/local/bin/ipython
python2.7-config => /usr/bin/python2.7-config

[indrajitr]

Okay, in that case:

1. Both echo $commands[(i)python[23]] and echo $commands[(i)python[23]#] should give you python3.
2. And echo $#commands[(i)python[23]#] should give you 1.

Can you please confirm @jeffwidman?

[jeffwidman]

Yes, that's correct:

~ ❯❯❯ echo $commands[(i)python[23]]
python3
~ ❯❯❯ echo $commands[(i)python[23]#]
python3
~ ❯❯❯ echo $#commands[(i)python[23]#]
1

[indrajitr]

So looks like echo $#commands[(i)python[23]#] didn't work for you per your previous observation. Did something change in your environment?

Further, based on what we found so far, either of these should work:

• if (( ! $#commands[(i)python[23]#] && ! $+functions[pyenv] )); then echo "Python not there"; fi
• if (( ! $+commands[(i)python[23]#] && ! $+functions[pyenv] )); then echo "Python not there"; fi

Can you please give these a try?

[jeffwidman]

Yep, both find a python as expected:

~ ❯❯❯ if (( ! $#commands[(i)python[23]#] && ! $+functions[pyenv] )); then echo "Python not there"; fi
~ ❯❯❯ if (( ! $+commands[(i)python[23]#] && ! $+functions[pyenv] )); then echo "Python not there"; fi

Did something change in your environment?

Nope, nothing different, still the same.

And despite the output above seeming to indicate that everything should work as expected, I'm still experiencing the behavior I originally reported where the virtualenvwrapper commands like mkvirtualenv, workon, etc don't load anything... but as soon as I revert back to one of the other variations mentioned in https://github.com/sorin-ionescu/prezto/issues/1949#issuecomment-891561833, it starts working immediately.

I don't understand why $#commands[(i)python[23]#] finds python in a single-line if statement, but fails in the multi-line format that's currently on master... 🤔

[jeffwidman]

Also, @indrajitr if it'd be faster, happy to do a quick call sometime when our timezones overlap and do a video call and try to work through this together. I find it very perplexing.

[belak]

I think it's extendedglob. It's set in the directory module which in @jeffwidman's dotfiles, is loaded after python. So, when it's trying to load python, extendedglob is not set, but in the shell afterwards, it is set.

What happens if you run:

setopt noextendedglob
if (( ! $#commands[(i)python[23]#] && ! $+functions[pyenv] )); then echo "Python not there"; fi
if (( ! $+commands[(i)python[23]#] && ! $+functions[pyenv] )); then echo "Python not there"; fi

[jeffwidman]

Oh, that sounds like a very reasonable hypothesis, that never even crossed my radar.

A quick check proves you're right @belak:

~ ❯❯❯ setopt noextendedglob
~ ❯❯❯ if (( ! $#commands[(i)python[23]#] && ! $+functions[pyenv] )); then echo "Python not there"; fi
Python not there
~ ❯❯❯ if (( ! $+commands[(i)python[23]#] && ! $+functions[pyenv] )); then echo "Python not there"; fi
~ ❯❯❯

So I'm thinking several things:

1. the command to check for python should be switched to $+commands[(i)python[23]#] to avoid this cropping up for anyone else. Ideally our commands don't care about whether extendedglob is set or not since some users may not want to set it.
2. IIF I am loading the directory module, should it be before most of the language-specific modules? If so, we'll want to document this on the directory readme and in the zpreztorc section that specifies the modules where we document the other modules that have to be loaded before/after certain things.

Thoughts?

Also @belak what prompted you to come up with this? Super curious as I said it was completely off my radar.

[indrajitr]

Great callout @belak! Thank you.

So here are my thoughts:

1. We should probably do setopt EXTENDED_GLOB in respective module init (in this case python/init.zsh) as is the convention in zsh/prezto world instead of relying on another module setting it. In prezto both directory and completion sets extended glob but in the case of @jeffwidman both are loaded after python. This will also prevent unnecessarily imposing module orders.

2. Like I said earlier, I am fine with switching $#commands[(i)python[23]#] to $+commands[(i)python[23]#] but that still won't solve the actual problem. The assignment (VIRTUALENVWRAPPER_PYTHON=$commands[(i)python[23]#]) will still fail because # won't be considered a glob character.

Summary:

1. Add setopt EXTENDED_GLOB in <module>/init.zsh wherever applicable.
2. Use (i)python[0-9.]# as subscript for commands associative array, following the convention of detection python more exhaustively. We do it in _pip completion, just that I didn't get around applying it here with my previous PR.

Fresh PR coming up :)

[belak]

That seems like it would be a good fix. That being said, it's a weird choice to set extended_glob for the user purely because a module needs it, but I suppose it works - and it looks like I added the one in completion so I'm partially to blame here :laughing:

Maybe in the future, we could make those local options when loading packages, but that's not a requirement for the incoming PR... and I'd be unsure when we should set extended glob for the user... so oh, well I suppose.

Thanks for doing this!

[hardkrash]

I was working through something like this as well and with d840f0fc7bb9e604fedef40eff4ed53f4c3c60f6 it expects that pyenv is installed.

So adding a check for pyenv into the test, workon is loaded by the fallback.

if (( $+VIRTUALENVWRAPPER_VIRTUALENV || $+commands[virtualenv] )) \
      && (($+commands[pyenv] )) \
      && zstyle -T ':prezto:module:python:virtualenv' initialize ; then

[jeffwidman]

Summary:

1. Add setopt EXTENDED_GLOB in <module>/init.zsh wherever applicable.
2. Use (i)python[0-9.]# as subscript for commands associative array, following the convention of detection python more exhaustively. We do it in _pip completion, just that I didn't get around applying it here with my previous PR.

Fresh PR coming up :)

I put up #2021 to resolve 1.

I looked at 2, but didn't actually understand it... is this saying I should add #compdef -P pip[0-9.]# or that the calls to $#commands[(i)python[23]#] should be replaced with $#commands[(i)python[0-9.]#]?

@indrajitr maybe the fastest way to answer ☝️ is to simply put up that PR fixing that?

[belak]

It's good to keep that future proofing in mind, but I hope there isn't a python 4 migration any time soon and it seems unrelated to this issue.

I'm going to close this for now since the PR Jeff submitted should fix the underlying issue.

[jeffwidman]

Sounds good.

Note for future searchers that virtualenvwrapper is now broken again for me due to https://github.com/sorin-ionescu/prezto/pull/1981#issuecomment-1291220229, but that's an unrelated issue from the problem here.

[jeffwidman]

Also @hardkrash thanks for the heads up on the new breakage.

Made it much quicker to realize why this alone didn't solve things... I filed #2022 for it.

However, your suggested solution skips setting some handy env vars. I put up an alternative fix in https://github.com/sorin-ionescu/prezto/pull/2023. Feel free to try that out and comment on the PR re: whether it fixes things for you.