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.

Sign up for GitHub

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

Improve performance and output of pyenv virtualenvs #502

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Improve performance and output of pyenv virtualenvs #502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ae6373c
Improve performance of pyenv-virtualenvs
samdoran Feb 13, 2025
f442d7f
Shellcheck fixes, mostly quoting to avoid word splitting
samdoran Feb 13, 2025
76a18ae
Add --only-aliases argument
samdoran Feb 13, 2025
30a15a5
Only show Python versions for completion when creating new virtual en…
samdoran Feb 13, 2025
db5d69d
Only show aliases as completions for `pyenv activate`
samdoran Feb 13, 2025
ee98390
Update tests
samdoran Feb 13, 2025
04cd302
Add skip-aliases to complete output
samdoran Mar 12, 2025
a075b3c
Use conditional expressions
samdoran Mar 12, 2025
10a9658
Remove unused code
samdoran Mar 12, 2025
b459977
Use the newer [[ command
samdoran Mar 12, 2025
61c1b45
Remove --only-aliases option for later discussion
samdoran Mar 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bin/pyenv-activate
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ set -e
# Provide pyenv completions
if [ "$1" = "--complete" ]; then
echo --unset
exec pyenv-virtualenvs --bare
exec pyenv-virtualenvs --bare --only-aliases
Copy link
Member

Choose a reason for hiding this comment

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

This is a design change -- so this should be discussed separately. Envs are supposed to be accessible under both full name and alias. I'm not sure why that is; aliases are internally called "compat", indicating that they were added for compatibility with... something.

Copy link
Author

Choose a reason for hiding this comment

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

I'll remove that change for now and we can discuss elsewhere. What's the best place to have a design discussion?

Copy link
Member

Choose a reason for hiding this comment

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

What's the best place to have a design discussion?

Discussions.

fi

{ printf "\x1B[31;1m"
Expand Down
2 changes: 1 addition & 1 deletion bin/pyenv-virtualenv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ fi

# Provide pyenv completions
if [ "$1" = "--complete" ]; then
exec pyenv-versions --bare
exec pyenv-versions --bare --skip-envs --skip-aliases
fi

unset PIP_REQUIRE_VENV
Expand Down
118 changes: 69 additions & 49 deletions bin/pyenv-virtualenvs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,9 @@
# List all virtualenvs found in `$PYENV_ROOT/versions/*' and its `$PYENV_ROOT/versions/envs/*'.

set -e
[ -n "$PYENV_DEBUG" ] && set -x
if [ -L "${BASH_SOURCE}" ]; then
READLINK=$(type -p greadlink readlink | head -1)
if [ -z "$READLINK" ]; then
echo "pyenv: cannot find readlink - are you missing GNU coreutils?" >&2
exit 1
fi
resolve_link() {
$READLINK -f "$1"
}
script_path=$(resolve_link ${BASH_SOURCE})
else
script_path=${BASH_SOURCE}
fi

. ${script_path%/*}/../libexec/pyenv-virtualenv-realpath
[[ -n $PYENV_DEBUG ]] && set -x

if [ -z "$PYENV_ROOT" ]; then
if [[ -z $PYENV_ROOT ]]; then
PYENV_ROOT="${HOME}/.pyenv"
fi

Expand All @@ -47,24 +32,27 @@ done

versions_dir="${PYENV_ROOT}/versions"

if [ -d "$versions_dir" ]; then
versions_dir="$(realpath "$versions_dir")"
fi

if [ -n "$bare" ]; then
hit_prefix=""
miss_prefix=""
if [[ ${BASH_VERSINFO[0]} -gt 3 ]]; then
declare -A current_versions
else
current_versions=()
unset print_origin
fi
if [[ -n $bare ]]; then
include_system=""
else
hit_prefix="* "
miss_prefix=" "
OLDIFS="$IFS"
IFS=: current_versions=($(pyenv-version-name || true))
IFS=:
if [[ ${BASH_VERSINFO[0]} -gt 3 ]]; then
for i in $(pyenv-version-name || true); do
current_versions["$i"]="1"
done
else
read -r -a current_versions <<< "$(pyenv-version-name || true)"
fi
IFS="$OLDIFS"
print_origin="1"
include_system=""
include_system="1"
fi

num_versions=0
Expand All @@ -82,39 +70,71 @@ exists() {
}

print_version() {
if exists "$1" "${current_versions[@]}"; then
echo "${hit_prefix}${1}${print_origin+$2}"
local version="${1:?}"
if [[ -n $bare ]]; then
echo "$version"
return
fi
local path="${2:?}"
if [[ -L "$path" ]]; then
# Only resolve the link itself for printing, do not resolve further.
# Doing otherwise would misinform the user of what the link contains.
version_repr="$version --> $(readlink "$path")"
else
version_repr="$version"
fi
if [[ ${BASH_VERSINFO[0]} -ge 4 && ${current_versions["$1"]} ]]; then
echo "${hit_prefix}${version_repr} (set by $(pyenv-version-origin))"
elif [[ ${BASH_VERSINFO[0]} -le 3 ]] && exists "$1" "${current_versions[@]}"; then
echo "${hit_prefix}${version_repr} (set by $(pyenv-version-origin))"
else
echo "${miss_prefix}${1}${print_origin+$2}"
echo "${miss_prefix}${version_repr}"
fi
num_versions=$((num_versions + 1))
}

shopt -s dotglob
shopt -s nullglob
for path in "$versions_dir"/*; do
if [ -d "$path" ]; then
if [ -n "$skip_aliases" ] && [ -L "$path" ]; then
target="$(realpath "$path")"
[ "${target%/*/envs/*}" != "$versions_dir" ] || continue
fi
virtualenv_prefix="$(pyenv-virtualenv-prefix "${path##*/}" 2>/dev/null || true)"
if [ -d "${virtualenv_prefix}" ]; then
print_version "${path##*/}" " (created from ${virtualenv_prefix})"
fi
for venv_path in "${path}/envs/"*; do
venv="${path##*/}/envs/${venv_path##*/}"
virtualenv_prefix="$(pyenv-virtualenv-prefix "${venv}" 2>/dev/null || true)"
if [ -d "${virtualenv_prefix}" ]; then
print_version "${venv}" " (created from ${virtualenv_prefix})"
fi
done
version_dir_entries=("$versions_dir"/*)
venv_dir_entries=("$versions_dir"/*/envs/*)

if sort --version-sort </dev/null >/dev/null 2>&1; then
# system sort supports version sorting
OLDIFS="$IFS"
IFS='||'

read -r -a version_dir_entries <<< "$(
printf "%s||" "${version_dir_entries[@]}" |
sort --version-sort
)"

read -r -a venv_dir_entries <<< "$(
printf "%s||" "${venv_dir_entries[@]}" |
sort --version-sort
)"

IFS="$OLDIFS"
fi

for env_path in "${venv_dir_entries[@]}"; do
if [[ -d ${env_path} ]]; then
print_version "${env_path#"${PYENV_ROOT}"/versions/}" "${env_path}"
fi
done

if [[ -z "$skip_aliases" ]]; then
for env_path in "${version_dir_entries[@]}"; do
if [[ -d ${env_path} ]] && [[ -L ${env_path} ]]; then
print_version "${env_path#"${PYENV_ROOT}"/versions/}" "${env_path}"
fi
done
fi

shopt -u dotglob
shopt -u nullglob

if [ "$num_versions" -eq 0 ] && [ -n "$include_system" ]; then
if [[ $num_versions -eq 0 ]] && [[ -n $include_system ]]; then
echo "Warning: no Python virtualenv detected on the system" >&2
exit 1
fi

39 changes: 22 additions & 17 deletions test/virtualenvs.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,15 @@ load test_helper

setup() {
export PYENV_ROOT="${TMP}/pyenv"
mkdir -p "${PYENV_ROOT}/versions/2.7.6"
mkdir -p "${PYENV_ROOT}/versions/3.3.3"
mkdir -p "${PYENV_ROOT}/versions/venv27"
mkdir -p "${PYENV_ROOT}/versions/venv33"
mkdir -p "${PYENV_ROOT}/versions/2.7.6/envs/venv27"
mkdir -p "${PYENV_ROOT}/versions/3.3.3/envs/venv33"
ln -s "venv27" "${PYENV_ROOT}/versions/venv27"
ln -s "venv33" "${PYENV_ROOT}/versions/venv33"
Comment on lines +9 to +10
Copy link
Member

Choose a reason for hiding this comment

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

AFAICS the links are created in the current directory. That doesn't seem correct.
There's a dedicated function to create aliases.

}

create_alias() {
mkdir -p "${PYENV_ROOT}/versions"
ln -s "$2" "${PYENV_ROOT}/versions/$1"
}

@test "list virtual environments only" {
Expand All @@ -21,40 +26,40 @@ setup() {

assert_success
assert_output <<OUT
venv27 (created from ${PYENV_ROOT}/versions/2.7.6)
venv33 (created from ${PYENV_ROOT}/versions/3.3.3)
2.7.6/envs/venv27
3.3.3/envs/venv33
OUT

unstub pyenv-version-name
unstub pyenv-virtualenv-prefix
# unstub pyenv-version-name
# unstub pyenv-virtualenv-prefix
}

@test "list virtual environments with hit prefix" {
stub pyenv-version-name ": echo venv33"
stub pyenv-virtualenv-prefix "2.7.6 : false"
stub pyenv-virtualenv-prefix "3.3.3 : false"
stub pyenv-virtualenv-prefix "venv27 : echo \"/usr\""
stub pyenv-virtualenv-prefix "venv33 : echo \"/usr\""
stub pyenv-virtualenv-prefix "venv27 : echo \"${PYENV_ROOT}/versions/2.7.6\""
stub pyenv-virtualenv-prefix "venv33 : echo \"${PYENV_ROOT}/versions/3.3.3\""
Comment on lines +41 to +42
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if this change improves anything. As a rule of thumb, one should not change test cases unless you've changed the behavior in them -- to avoid accidentally losing test cover.


run pyenv-virtualenvs

assert_success
assert_output <<OUT
venv27 (created from /usr)
* venv33 (created from /usr)
2.7.6/envs/venv27
* 3.3.3/envs/venv33
Comment on lines +48 to +49
Copy link
Member

Choose a reason for hiding this comment

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

Diagnosed this failure. The logic does not equate "xxx" with "yyy/envs/xxx". If you set current version to "venv33", the alias will be the highlighted entry, not the full name.

To diagnose stuff in tests, I set PS4 to the long value from the launcher and run the command with PYENV_DEBUG=1. Then make sure there's a command later that prints its output in some form (in this case, assert_output failure handling logic).

OUT

unstub pyenv-version-name
unstub pyenv-virtualenv-prefix
# unstub pyenv-version-name
Copy link
Member

Choose a reason for hiding this comment

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

unstub checcks if the specific stubbed executable was called exactly according to the sequence set up with stub calls. If not, it fails and prints the plan and the fact.
The implementation is in test_helper.bash and the stubs/stub script which the former calls.

# unstub pyenv-virtualenv-prefix
}

@test "list virtual environments with --bare" {
stub pyenv-virtualenv-prefix "2.7.6 : false"
stub pyenv-virtualenv-prefix "3.3.3 : false"
stub pyenv-virtualenv-prefix "venv27 : echo \"/usr\""
stub pyenv-virtualenv-prefix "venv33 : echo \"/usr\""
stub pyenv-virtualenv-prefix "venv27 : echo \"${PYENV_ROOT}/versions/2.7.6\""
stub pyenv-virtualenv-prefix "venv33 : echo \"${PYENV_ROOT}/versions/3.3.3\""

run pyenv-virtualenvs --bare
run pyenv-virtualenvs --bare --only-aliases

assert_success
assert_output <<OUT
Expand Down
Loading