docs: incorporate Charon review — safer serverEnv() filtering
Co-Authored-By: Virgil <virgil@lethean.io> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
78e244f26f
commit
9dda860df4
1 changed files with 26 additions and 17 deletions
|
|
@ -582,6 +582,7 @@ package rocm
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"os"
|
"os"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
|
|
@ -627,26 +628,26 @@ func TestFreePort_UniquePerCall(t *testing.T) {
|
||||||
|
|
||||||
func TestServerEnv_HIPVisibleDevices(t *testing.T) {
|
func TestServerEnv_HIPVisibleDevices(t *testing.T) {
|
||||||
env := serverEnv()
|
env := serverEnv()
|
||||||
found := false
|
|
||||||
for _, e := range env {
|
|
||||||
if e == "HIP_VISIBLE_DEVICES=0" {
|
|
||||||
found = true
|
|
||||||
break
|
|
||||||
}
|
|
||||||
}
|
|
||||||
assert.True(t, found, "HIP_VISIBLE_DEVICES=0 must be in server env")
|
|
||||||
|
|
||||||
// Also verify it overrides any existing setting
|
|
||||||
t.Setenv("HIP_VISIBLE_DEVICES", "1")
|
|
||||||
env = serverEnv()
|
|
||||||
// The last occurrence wins — ours must be present
|
|
||||||
var hipVals []string
|
var hipVals []string
|
||||||
for _, e := range env {
|
for _, e := range env {
|
||||||
if len(e) > 20 && e[:20] == "HIP_VISIBLE_DEVICES=" {
|
if strings.HasPrefix(e, "HIP_VISIBLE_DEVICES=") {
|
||||||
hipVals = append(hipVals, e)
|
hipVals = append(hipVals, e)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
assert.Contains(t, hipVals, "HIP_VISIBLE_DEVICES=0")
|
assert.Equal(t, []string{"HIP_VISIBLE_DEVICES=0"}, hipVals)
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestServerEnv_FiltersExistingHIP(t *testing.T) {
|
||||||
|
t.Setenv("HIP_VISIBLE_DEVICES", "1")
|
||||||
|
env := serverEnv()
|
||||||
|
var hipVals []string
|
||||||
|
for _, e := range env {
|
||||||
|
if strings.HasPrefix(e, "HIP_VISIBLE_DEVICES=") {
|
||||||
|
hipVals = append(hipVals, e)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Must filter the old value and only have our override
|
||||||
|
assert.Equal(t, []string{"HIP_VISIBLE_DEVICES=0"}, hipVals)
|
||||||
}
|
}
|
||||||
```
|
```
|
||||||
|
|
||||||
|
|
@ -670,6 +671,7 @@ import (
|
||||||
"os"
|
"os"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
"strconv"
|
"strconv"
|
||||||
|
"strings"
|
||||||
"syscall"
|
"syscall"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
|
@ -713,9 +715,16 @@ func freePort() (int, error) {
|
||||||
}
|
}
|
||||||
|
|
||||||
// serverEnv returns the environment for the llama-server subprocess.
|
// serverEnv returns the environment for the llama-server subprocess.
|
||||||
// Always includes HIP_VISIBLE_DEVICES=0 to mask the iGPU.
|
// Filters any existing HIP_VISIBLE_DEVICES and sets it to 0 to mask the iGPU.
|
||||||
func serverEnv() []string {
|
func serverEnv() []string {
|
||||||
return append(os.Environ(), "HIP_VISIBLE_DEVICES=0")
|
env := os.Environ()
|
||||||
|
filtered := make([]string, 0, len(env)+1)
|
||||||
|
for _, e := range env {
|
||||||
|
if !strings.HasPrefix(e, "HIP_VISIBLE_DEVICES=") {
|
||||||
|
filtered = append(filtered, e)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return append(filtered, "HIP_VISIBLE_DEVICES=0")
|
||||||
}
|
}
|
||||||
|
|
||||||
// startServer spawns llama-server and waits for it to become ready.
|
// startServer spawns llama-server and waits for it to become ready.
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue