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

[jantonguirao]

Category:

Other (Refactoring)

Description:

install_pip_pkg in qa/test_template_impl.sh tries the offline /pip-packages mirror, then
falls back to the index. Neither branch survives pip's uninstall-no-record-file error: 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.

This is what happened to #6483's CI run: it asked for unpinned flask, pip resolved flask==3.1.3,
which needs blinker>=1.9, and the base image's apt-installed blinker==1.7.0 has no RECORD, so
the install died before a single test ran. That PR worked around it with a version pin
(flask==3.0.3, which is satisfied by the preinstalled blinker and never triggers an upgrade),
per JanuszL's suggestion on that thread
to fix the underlying installer logic instead.

install_pip_pkg now recognizes the failure - the pip message differs across versions, so it
matches all three wordings pip has used - and retries with --ignore-installed, which is what
pip's own hint now recommends for this exact error. Since install_pip_pkg is on the path of every
qa suite that sources qa/test_template.sh, the change stays narrow: the happy path is unchanged,
and a genuine failure unrelated to an unrecordable uninstall still propagates and aborts the caller
exactly as before.

Not a clean recovery, on purpose left as-is:

--ignore-installed is not scoped to the failing package - it clears the resolver's whole view of
installed distributions, so nothing is uninstalled and the new wheel is written over the old one.
That is enough to make the install succeed, but the stale .dist-info survives, so
importlib.metadata/pip list still report the old version afterwards, and a later install needing
the same upgrade hits the identical error again. The retry is commented loudly for this reason.

Sequencing with #6483:

This does not touch qa/TL0_python-self-test-readers-decoders/test_nofw.sh - the flask==3.0.3 pin
lives on #6483's branch, which has not merged yet, so there is nothing here to relax. Once both land,
relaxing that pin to flask<4 (keeping an upper bound because moto is pinned at 5.2.3 and
moto.server sits on flask + flask-cors) would be a one-line follow-up that also gives this path its
first real CI exercise, since nothing on main today hits an unrecordable uninstall.

Affected modules and functionalities:

• qa/test_template_impl.sh: install_pip_pkg().

Key points relevant for the review:

• No call site changes; every one of the 4 call sites in qa/ (the per-package loop, the CUDA
  wheel install, TL0_python-self-test-core's numpy<2 downgrade, TL1_tensorflow-dali_test) gets
  the fallback for free.
• The exit status is threaded through a temp file rather than $?/${PIPESTATUS[0]}: the if
  list is piped into tee to keep streaming output into the test log, which runs it in a subshell;
  under the set -e every caller of this function has, a non-zero status inside that subshell would
  tear it down before an assignment after the list could run. ${PIPESTATUS[0]} would work under
  set -o pipefail, which qa/test_template.sh sets today, but not for a caller that does not.

Tests:

• Existing tests apply
• New tests added
• N/A

Verified against real pip (22.0.2 and 26.2.1), with blinker's RECORD file removed to reproduce
the apt/dnf layout, both with and without set -o pipefail:

• a flask==3.1.0 install that trips the RECORD-less blinker upgrade: recovers, returns 0
• an install of a nonexistent package: still returns non-zero, still aborts the caller
• a plain already-satisfied install: unchanged, returns 0 on the first attempt

Checklist

Documentation

• Existing documentation applies
• Documentation updated
• N/A

DALI team only

Requirements

• Implements new requirements
• Affects existing requirements
• N/A

REQ IDs: N/A

JIRA TASK: N/A

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[greptile-apps]

Confidence Score: 4/5

The implementation appears safe to merge, with a non-blocking gap in automated coverage for the new fallback behavior.

Findings

1.  Retry Path Lacks Coverage ▶

Summary

This PR extends the shared QA pip installer to recover when pip cannot uninstall a distribution lacking a RECORD file.

• Captures streamed pip output and the final initial-attempt status.
• Detects three known forms of pip's unrecordable-uninstall diagnostic.
• Retries the offline mirror and index with --ignore-installed.
• Preserves nonzero status propagation for unrelated installation failures.
• Would benefit from automated regression coverage for the new recovery path.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Try offline mirror] -->|Success| Z[Return success]
  A -->|Failure| B[Try package index]
  B -->|Success| Z
  B -->|Failure| C{Log matches no-RECORD diagnostic?}
  C -->|No| F[Return original failure]
  C -->|Yes| D[Retry offline with ignore-installed]
  D -->|Success| Z
  D -->|Failure| E[Retry index with ignore-installed]
  E -->|Success| Z
  E -->|Failure| G[Return retry failure]

Loading

_{Reviews (1) · Last reviewed commit: "Retry install_pip_pkg with --ignore-inst..."}

[greptile-apps]

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!

[Copilot]

🔵 Needs a closer look

Cleanup must handle stale metadata left by the fallback.

Pull request overview

This PR updates shared QA pip installation to recover upgrades blocked by missing RECORD metadata.

Changes:

• Captures pip output and status.
• Detects known unrecordable-uninstall failures.
• Retries with --ignore-installed while preserving unrelated failures.

File summaries

File	Summary
qa/test_template_impl.sh	Adds fallback retry logic; cleanup may still fail on stale RECORD-less metadata.

Review details

Suppressed comments (1)

qa/test_template_impl.sh:61

• This retry can leave a stale RECORD-less .dist-info for a package that is itself in pip_packages. The common cleanup later in this file runs pip uninstall -y $remove without any recovery, and setup_packages.py includes direct package entries in that removal list, so a direct distro-installed package can make the suite fail after the tests have passed. Please either clean/replace the stale metadata before returning success or make cleanup explicitly handle packages that required this fallback.

        if ${install_cmd} --ignore-installed --no-index || ${install_cmd} --ignore-installed; then

• Files reviewed: 1/1 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[dali-automaton]

CI MESSAGE: [69108883]: BUILD STARTED

[dali-automaton]

CI MESSAGE: [69108883]: BUILD FAILED

[dali-automaton]

CI MESSAGE: [69108883]: BUILD PASSED