Skip to content

Set VIRTUALENVWRAPPER_SCRIPT portably #117

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

Merged
merged 1 commit into from
Mar 31, 2025

Conversation

ferdnyc
Copy link
Contributor

@ferdnyc ferdnyc commented Mar 24, 2025

  • ${.sh.file} was ksh syntax, but ksh support was dropped in commit d736549
  • Instead of testing only for certain specific shells (and excluding all others), use $BASH_SOURCE if it's set, otherwise fall back to $0 which should work with more shells (hopefully).

Fixes #105

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Mar 24, 2025

(Note: I don't know that this will make virtualenvwrapper WORK with the ash shell (from #105), that depends how it handles $0 in sourced scripts. But it will at least make it stop crashing the script when it tries to source it, because it hits the KSH syntax that's currently being used as the default.)

@dhellmann
Copy link
Contributor

Some of the bash versions in the test matrix don't seem to like the new syntax. I see lots of errors like:

/Users/runner/work/virtualenvwrapper/virtualenvwrapper/.tox/py/bin/virtualenvwrapper.sh: line 99: ${$BASH_SOURCE:-}: bad substitution

@dhellmann
Copy link
Contributor

It also looks like even though we said we dropped zsh support there is still a test job for it.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Mar 30, 2025

Some of the bash versions in the test matrix don't seem to like the new syntax. I see lots of errors like:

/Users/runner/work/virtualenvwrapper/virtualenvwrapper/.tox/py/bin/virtualenvwrapper.sh: line 99: ${$BASH_SOURCE:-}: bad substitution

Oh, crap! Not surprised, that's a really dumb typo. It should be ${BASH_SOURCE:-}. Whoops. I'll fix that, thanks.

Maybe I had Fish on the brain. {$BASH_SOURCE}, REALLY???

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Mar 31, 2025

@dhellmann Fixed now, sorry about that.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Mar 31, 2025

It also looks like even though we said we dropped zsh support there is still a test job for it.

Zsh is still supported, I think? The $0 there is specifically for Zsh, though I hope it will work in some other shells as well.

It's Ksh that support was dropped for, and their crazy ${.sh.file} was never going to work for any other shell so it was the worst possible default.

@ferdnyc
Copy link
Contributor Author

ferdnyc commented Mar 31, 2025

@dhellmann #118 will fix those readthedocs failures.

@dhellmann
Copy link
Contributor

It also looks like even though we said we dropped zsh support there is still a test job for it.

Zsh is still supported, I think? The $0 there is specifically for Zsh, though I hope it will work in some other shells as well.

It's Ksh that support was dropped for, and their crazy ${.sh.file} was never going to work for any other shell so it was the worst possible default.

Wow, yes, you're right. I completely conflated ksh and zsh in my head.

@dhellmann
Copy link
Contributor

@Mergifyio rebase

- `${.sh.file}` was ksh syntax, but ksh support was dropped in commit
  d736549
- Instead of testing only for certain specific shells (and excluding
  all others), use `$BASH_SOURCE` if it's set, otherwise fall back
  to `$0` which should work with more shells (hopefully).

  Fixes python-virtualenvwrapper#105
Copy link
Contributor

mergify bot commented Mar 31, 2025

rebase

✅ Branch has been successfully rebased

@mergify mergify bot merged commit 28a70cf into python-virtualenvwrapper:main Mar 31, 2025
17 checks passed
@ferdnyc ferdnyc deleted the cross-shell branch March 31, 2025 22:11
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.

failing to source virtualenvwrapper on alpine linux (ash)
2 participants