Skip to content

Retry install_pip_pkg with --ignore-installed after an unrecordable uninstall #6485

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

Merged
jantonguirao merged 1 commit into from
Sep 28, 2026
Merged

Retry install_pip_pkg with --ignore-installed after an unrecordable uninstall #6485

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
39 changes: 37 additions & 2 deletions qa/test_template_impl.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,43 @@ last_config_index=$($topdir/qa/setup_packages.py -n -u $pip_packages --cuda ${CU

install_pip_pkg() {
install_cmd="$@"
# if no package was found in our download dir, so install it from index
${install_cmd} --no-index || ${install_cmd}
# pip refuses to uninstall a distribution whose metadata has no RECORD file - which is how
# distro package managers install Python packages - and aborts the whole command instead of
# upgrading it. That can happen with any package list, so recognize it here rather than at
# each call site. The message differs across pip versions: pip >= 23.3 tags it
# "uninstall-no-record-file", older pip just says "RECORD file not found"; the distutils
# sibling case has no reference tag at all.
local no_record_re='uninstall-no-record-file|RECORD file not found|distutils installed project'
local pip_log pip_status
pip_log=$(mktemp)
pip_status=$(mktemp)
# `tee` keeps the output streaming into the test log; the exit status has to be captured
# through a file rather than $? or $PIPESTATUS[0], because the `if` below runs the AND-OR
# list in a pipeline subshell - with `set -e` in force (as in every caller of this function)
# a non-zero status inside that subshell would abort it before any assignment after the list
# could run, so the status has to be written from inside the list itself.
{
# if no package was found in our download dir, so install it from index
if ${install_cmd} --no-index || ${install_cmd}; then
echo 0 >"${pip_status}"
else
echo $? >"${pip_status}"
fi
} 2>&1 | tee "${pip_log}"
local ret
ret=$(cat "${pip_status}" 2>/dev/null || echo 1)
if [ "${ret}" != "0" ] && grep -qE "${no_record_re}" "${pip_log}"; then
# this is what pip itself now suggests for this error; it leaves the stale .dist-info
# behind, so anything introspecting installed versions afterwards may see the old one
echo "pip cannot uninstall a preinstalled package, retrying with --ignore-installed"
if ${install_cmd} --ignore-installed --no-index || ${install_cmd} --ignore-installed; then
ret=0
else
ret=$?
fi
Comment on lines +57 to +65

@greptile-apps greptile-apps Bot Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 Retry Path Lacks Coverage

The new retry branch has no automated regression test. Existing main-branch callers do not exercise a distribution without a RECORD file, so changes to diagnostic matching, status capture, or unrelated-failure propagation could break this recovery without CI detecting it. A shell test using a fake pip command should cover both successful recovery and nonmatching failure propagation.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

fi
rm -f "${pip_log}" "${pip_status}"
return "${ret}"
}

if [ -n "$gather_pip_packages" ]
Expand Down