Skip to content

fix(install): resolve repo_version placeholder in driver dry-run output - #307

Open
r0x0r wants to merge 1 commit into
mainfrom
fix/driver-dry-run-repo-version-placeholder
Open

fix(install): resolve repo_version placeholder in driver dry-run output#307
r0x0r wants to merge 1 commit into
mainfrom
fix/driver-dry-run-repo-version-placeholder

Conversation

@r0x0r

@r0x0r r0x0r commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

rocm install driver --dry-run printed an unexpanded shell-style placeholder on the repo_version: line:

repo_version: ${ROCM_CLI_AMDGPU_VERSION:-7.2.4}

The literal ${ROCM_CLI_AMDGPU_VERSION:-7.2.4} template was emitted verbatim instead of being resolved to the effective version.

Root cause

The driver install plan stores the repo version as a shell parameter-expansion template so it can be embedded verbatim into the shell commands that later expand it at runtime. The human-readable plan summary printed the same template on its repo_version: line, leaking the raw shell placeholder into user-facing output.

Fix

Resolve the template to its effective value for display only:

  • the value of ROCM_CLI_AMDGPU_VERSION when set and non-empty (matching shell :- semantics), otherwise the 7.2.4 default.

The executable commands still carry the literal template, so the runtime shell resolves it as before — only the summary line changes.

Before:

repo_version: ${ROCM_CLI_AMDGPU_VERSION:-7.2.4}

After (env unset):

repo_version: 7.2.4

Tests

Added:

  • resolve_repo_version_uses_default_when_env_unset
  • resolve_repo_version_prefers_env_value_when_set
  • resolve_repo_version_treats_empty_env_as_unset
  • resolve_repo_version_passes_through_non_template
  • driver_plan_dry_run_repo_version_line_is_resolved — render-level regression asserting the dry-run summary shows the resolved version and not the raw placeholder.

The regression test fails before the fix (summary line showed the raw placeholder) and passes after.

Verification

  • cargo fmt -p rocm
  • cargo clippy -p rocm --all-targets -- -D warnings
  • cargo test -p rocm --bin rocm driver_plan (20 passed) and the new resolver tests

This is a cosmetic/clarity change to dry-run output; no behavior change to the executed commands.

The driver install plan stores the repo version as a shell parameter-
expansion template (${ROCM_CLI_AMDGPU_VERSION:-7.2.4}) so it can be
embedded verbatim into the shell commands that will later expand it. The
human-readable plan summary printed the same template on its
"repo_version:" line, leaking an unexpanded shell placeholder into
user-facing dry-run output.

Resolve the template to its effective value for display only: the value
of ROCM_CLI_AMDGPU_VERSION when set and non-empty (matching shell :-
semantics), otherwise the 7.2.4 default. The executable commands still
carry the literal template so the runtime shell resolves it.

Add unit tests for the resolver (default, env override, empty-as-unset,
non-template pass-through) and a render-level regression test asserting
the dry-run summary shows the resolved version rather than the raw
placeholder.

Signed-off-by: Roman Sirokov <roman.sirokov@amd.com>
@r0x0r
r0x0r requested a review from a team as a code owner August 24, 2026 10:57
@r0x0r
r0x0r requested a review from juhovainio August 24, 2026 10:57
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.

1 participant