Skip to content

rehash: Drop redundant sort -u from make_shims call#3410

Merged
native-api merged 1 commit intopyenv:masterfrom
jakelodwick:pr/01-drop-sort-u
Mar 1, 2026
Merged

rehash: Drop redundant sort -u from make_shims call#3410
native-api merged 1 commit intopyenv:masterfrom
jakelodwick:pr/01-drop-sort-u

Conversation

@jakelodwick
Copy link
Copy Markdown
Contributor

@jakelodwick jakelodwick commented Mar 1, 2026

The make_shims call in pyenv-rehash pipes list_executable_names through sort -u before processing. This deduplication is redundant: register_shim() already handles duplicates via associative array (bash 4+) or string matching (bash 3.2).

Removing the pipe saves one subprocess fork during pyenv rehash.

Before:

make_shims $(list_executable_names | sort -u)

After:

make_shims $(list_executable_names)

All existing tests pass. The change is behavior-preserving: shim creation is order-independent and duplicates are handled downstream.

register_shim() already deduplicates via associative array (bash 4+) or
string-containment check (bash 3.2). The sort -u pipe in the make_shims
call is redundant -- the dedup happens downstream regardless. Shim
creation is order-independent and idempotent, so sorting has no semantic
effect either.

Saves one subprocess fork during every rehash invocation.
@jakelodwick jakelodwick requested review from a team as code owners March 1, 2026 19:34
Comment thread libexec/pyenv-rehash
Comment on lines 170 to 180
register_shim() {
registered_shims="${registered_shims}${1} "
}

install_registered_shims() {
local shim file
for shim in $registered_shims; do
file="${SHIM_PATH}/${shim}"
[ -e "$file" ] || cp "$PROTOTYPE_SHIM_PATH" "$file"
done
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The Bash 3 path does not handle duplicate names with "substring match" as the PR description claims. It does however handle them with [ -e "$file" ] before creating a shim.

So accepting with an altered description

@native-api native-api merged commit 47871b2 into pyenv:master Mar 1, 2026
20 checks passed
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.

2 participants