Fix gemini round 5: POSIX compliance and leading-comma visibility parsing

Three medium findings from gemini-code-assist addressed in this commit:

1. _pick_radeon_wheel used grep -o and sort -V, both GNU extensions
   that are not in POSIX and break on BSD/BusyBox coreutils. install.sh
   has a #!/bin/sh shebang so the whole pipeline was rewritten as a
   single awk script that extracts all href="..." hits on each line,
   filters to wheels matching the package prefix and python tag, and
   picks the newest version via zero-padded lexical comparison. No
   external sort or grep is needed.

2. _first_visible_amd_gpu_id in the AMD monitoring backend treated a
   leading comma (e.g. HIP_VISIBLE_DEVICES=",1") as "fall through to
   the next env var", which is surprising given the clear intent to
   narrow to device 1. Filter empty tokens after the split and return
   the first real one. An all-commas value ("," / ",,,") still falls
   through because no real tokens exist; the empty-string and "-1"
   explicit-zero cases are unchanged.

The unrelated amd-smi version awk parser suggestion was not applied
(see round 4 commit message for rationale: defaulting a missing minor
to 0 could silently install the wrong ROCm wheel set).
This commit is contained in:
Daniel Han 2026-04-08 12:17:27 +00:00
parent d25c570a06
commit b3627bc29e
2 changed files with 51 additions and 22 deletions

View file

@ -1158,27 +1158,55 @@ _pick_radeon_wheel() {
# Scans $_RADEON_LISTING for the newest wheel whose filename starts exactly
# with PACKAGE_NAME- and matches _RADEON_PYTAG + linux_x86_64.
# Prints the full URL (resolving relative hrefs against _RADEON_BASE_URL).
#
# POSIX-compliant pipeline: all href parsing, filtering, and version
# selection is done inside a single awk script rather than reaching
# for GNU extensions (grep -o, sort -V) that would break under BSD
# or BusyBox coreutils.
_pkg="$1"
[ -n "$_RADEON_LISTING" ] || return 1
[ -n "$_RADEON_PYTAG" ] || return 1
_tag="$_RADEON_PYTAG"
_href=$(printf '%s\n' "$_RADEON_LISTING" \
| grep -o 'href="[^"]*"' \
| sed 's/href="//;s/"//' \
| awk -F/ -v pkg="$_pkg" -v tag="$_tag" '
| awk -v pkg="$_pkg" -v tag="$_tag" '
BEGIN { max_pad = ""; max_url = "" }
{
base = $NF
sub(/[?#].*/, "", base) # strip query / fragment
prefix = pkg "-"
# Match cpXY-cpXY or cpXY-abi3 with any linux x86_64 platform tag
# (linux_x86_64, manylinux_2_28_x86_64, manylinux2014_x86_64, etc.)
if (substr(base, 1, length(prefix)) == prefix &&
index(base, "-" tag "-") > 0 &&
match(base, /x86_64\.whl$/))
print $0
}' \
| sort -V \
| tail -1)
line = $0
while (match(line, /href="[^"]*"/)) {
# Strip the leading href=" (6 chars) and trailing " (1 char)
url = substr(line, RSTART + 6, RLENGTH - 7)
line = substr(line, RSTART + RLENGTH)
# Extract basename, strip query / fragment
n = split(url, p, "/")
base = p[n]
sub(/[?#].*/, "", base)
prefix = pkg "-"
# Match cpXY-cpXY or cpXY-abi3 with any linux x86_64
# platform tag (linux_x86_64, manylinux_2_28_x86_64,
# manylinux2014_x86_64, etc.)
if (substr(base, 1, length(prefix)) == prefix &&
index(base, "-" tag "-") > 0 &&
match(base, /x86_64\.whl$/)) {
# Extract the version component (first
# dotted-number run) and pad each piece so a
# plain lexical comparison gives us the newest.
if (match(base, /[0-9]+\.[0-9]+(\.[0-9]+)?/)) {
ver = substr(base, RSTART, RLENGTH)
m = split(ver, v, ".")
pad = ""
for (i = 1; i <= m; i++)
pad = pad sprintf("%08d", v[i])
if (pad > max_pad) {
max_pad = pad
max_url = url
}
}
}
}
}
END { if (max_url != "") print max_url }')
[ -z "$_href" ] && return 1
case "$_href" in
http*) printf '%s\n' "$_href" ;;

View file

@ -238,13 +238,14 @@ def _first_visible_amd_gpu_id() -> Optional[str]:
raw = raw.strip()
if raw == "" or raw == "-1":
return None
first = raw.split(",", 1)[0].strip()
if first:
return first
# Leading comma or all-whitespace first token -- fall through to
# the next env var in priority order rather than silently
# returning GPU 0.
continue
# Filter out empty tokens after splitting. This tolerates minor
# typos like ``HIP_VISIBLE_DEVICES=",1"`` (leading comma, user
# clearly meant to narrow to device 1) while still falling
# through to the next env var when every token is empty
# (e.g. ``,,,``).
tokens = [t.strip() for t in raw.split(",") if t.strip()]
if tokens:
return tokens[0]
return "0"