mirror of
https://github.com/unslothai/unsloth.git
synced 2026-07-25 23:53:59 +00:00
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:
parent
d25c570a06
commit
b3627bc29e
2 changed files with 51 additions and 22 deletions
58
install.sh
58
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" ;;
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue