From b3627bc29e5440a13169677b1340a1ed1e4e7ded Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 8 Apr 2026 12:17:27 +0000 Subject: [PATCH] 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). --- install.sh | 58 +++++++++++++++++++++------- studio/backend/utils/hardware/amd.py | 15 +++---- 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/install.sh b/install.sh index cd428878d1..0dc5a712c2 100755 --- a/install.sh +++ b/install.sh @@ -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" ;; diff --git a/studio/backend/utils/hardware/amd.py b/studio/backend/utils/hardware/amd.py index 93daae4605..76ce0e6eac 100644 --- a/studio/backend/utils/hardware/amd.py +++ b/studio/backend/utils/hardware/amd.py @@ -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"