-
-
Notifications
You must be signed in to change notification settings - Fork 423
Improve performance and output of pyenv virtualenvs
#502
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
base: master
Are you sure you want to change the base?
Changes from 6 commits
ae6373c
f442d7f
76a18ae
30a15a5
db5d69d
ee98390
04cd302
a075b3c
10a9658
b459977
61c1b45
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,13 +1,13 @@ | ||
#!/usr/bin/env bash | ||
# | ||
# Summary: List all Python virtualenvs found in `$PYENV_ROOT/versions/*'. | ||
# Usage: pyenv virtualenvs [--bare] [--skip-aliases] | ||
# Usage: pyenv virtualenvs [--bare] [--skip-aliases] [--only-aliases] | ||
# | ||
# 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 | ||
if [ -L "${BASH_SOURCE[0]}" ]; then | ||
READLINK=$(type -p greadlink readlink | head -1) | ||
if [ -z "$READLINK" ]; then | ||
echo "pyenv: cannot find readlink - are you missing GNU coreutils?" >&2 | ||
|
@@ -16,12 +16,12 @@ if [ -L "${BASH_SOURCE}" ]; then | |
resolve_link() { | ||
$READLINK -f "$1" | ||
} | ||
script_path=$(resolve_link ${BASH_SOURCE}) | ||
script_path=$(resolve_link "${BASH_SOURCE[0]}") | ||
else | ||
script_path=${BASH_SOURCE} | ||
script_path="${BASH_SOURCE[0]}" | ||
fi | ||
|
||
. ${script_path%/*}/../libexec/pyenv-virtualenv-realpath | ||
. "${script_path%/*}"/../libexec/pyenv-virtualenv-realpath | ||
|
||
if [ -z "$PYENV_ROOT" ]; then | ||
PYENV_ROOT="${HOME}/.pyenv" | ||
|
@@ -34,10 +34,11 @@ for arg; do | |
case "$arg" in | ||
--complete ) | ||
echo --bare | ||
echo --skip-aliases | ||
samdoran marked this conversation as resolved.
Show resolved
Hide resolved
|
||
echo --only-aliases | ||
exit ;; | ||
--bare ) bare=1 ;; | ||
--skip-aliases ) skip_aliases=1 ;; | ||
--only-aliases ) only_aliases=1 ;; | ||
* ) | ||
pyenv-help --usage virtualenvs >&2 | ||
exit 1 | ||
|
@@ -47,24 +48,60 @@ done | |
|
||
versions_dir="${PYENV_ROOT}/versions" | ||
|
||
if [ -d "$versions_dir" ]; then | ||
versions_dir="$(realpath "$versions_dir")" | ||
if ! enable -f "${BASH_SOURCE%/*}"/pyenv-realpath.dylib realpath 2>/dev/null; then | ||
if [ -n "$PYENV_NATIVE_EXT" ]; then | ||
echo "pyenv: failed to load \`realpath' builtin" >&2 | ||
exit 1 | ||
fi | ||
|
||
READLINK=$(type -P readlink) | ||
if [ -z "$READLINK" ]; then | ||
echo "pyenv: cannot find readlink - are you missing GNU coreutils?" >&2 | ||
exit 1 | ||
fi | ||
|
||
resolve_link() { | ||
$READLINK "$1" | ||
} | ||
|
||
realpath() { | ||
local path="$1" | ||
local name | ||
|
||
# Use a subshell to avoid changing the current path | ||
( | ||
while [ -n "$path" ]; do | ||
name="${path##*/}" | ||
[ "$name" = "$path" ] || cd "${path%/*}" | ||
path="$(resolve_link "$name" || true)" | ||
samdoran marked this conversation as resolved.
Show resolved
Hide resolved
|
||
done | ||
|
||
echo "${PWD}/$name" | ||
) | ||
} | ||
fi | ||
|
||
if [ -n "$bare" ]; then | ||
hit_prefix="" | ||
miss_prefix="" | ||
if ((BASH_VERSINFO[0] > 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] > 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 | ||
|
@@ -82,39 +119,73 @@ 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 | ||
echo "${miss_prefix}${1}${print_origin+$2}" | ||
version_repr="$version" | ||
fi | ||
if [[ ${BASH_VERSINFO[0]} -ge 4 && ${current_versions["$1"]} ]]; then | ||
samdoran marked this conversation as resolved.
Show resolved
Hide resolved
|
||
echo "${hit_prefix}${version_repr} (set by $(pyenv-version-origin))" | ||
elif (( BASH_VERSINFO[0] <= 3 )) && exists "$1" "${current_versions[@]}"; then | ||
echo "${hit_prefix}${version_repr} (set by $(pyenv-version-origin))" | ||
else | ||
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 | ||
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 | ||
|
||
if [ -z "$only_aliases" ]; then | ||
for env_path in "${venv_dir_entries[@]}"; do | ||
if [ -d "${env_path}" ]; then | ||
print_version "${env_path#"${PYENV_ROOT}"/versions/}" "${env_path}" | ||
fi | ||
virtualenv_prefix="$(pyenv-virtualenv-prefix "${path##*/}" 2>/dev/null || true)" | ||
if [ -d "${virtualenv_prefix}" ]; then | ||
print_version "${path##*/}" " (created from ${virtualenv_prefix})" | ||
done | ||
fi | ||
|
||
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 | ||
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 | ||
fi | ||
done | ||
done | ||
fi | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens with conda installations? They are currently listed, now they seem to stop being so. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know anything about or use conda, so I omitted those. 😊 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reading through the previous code, it seems like the intent was to look for certain indicator files and if they are present, list the directory as a virtual environment. I don't really know if that's a good heuristic or not. Calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, the changes in this PR may actually be doing a better job with this than the current code. I installed
I only see Current output running
|
||
|
||
shopt -u dotglob | ||
shopt -u nullglob | ||
|
||
if [ "$num_versions" -eq 0 ] && [ -n "$include_system" ]; then | ||
echo "Warning: no Python virtualenv detected on the system" >&2 | ||
exit 1 | ||
fi | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
} | ||
|
||
create_alias() { | ||
mkdir -p "${PYENV_ROOT}/versions" | ||
ln -s "$2" "${PYENV_ROOT}/versions/$1" | ||
} | ||
|
||
@test "list virtual environments only" { | ||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Diagnosed this failure. The logic does not equate " 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, |
||
OUT | ||
|
||
unstub pyenv-version-name | ||
unstub pyenv-virtualenv-prefix | ||
# unstub pyenv-version-name | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
# 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 | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussions.