Repository navigation
[Bug] Fix dashboard backend build and socket tests on macOS - #4829
JiaweiGithub wants to merge 1 commit into
Conversation
a4d5bac to
c35668a
Compare
Xunzhuo
left a comment
There was a problem hiding this comment.
Thanks for fixing the macOS build and socket-path issues. One required Linux lint failure needs a scoped fix before this can land; details inline. Both Source Checks and Dashboard CI reproduce it. Darwin runtime tests were not run.
| if int(stat.Uid) == os.Getuid() && stat.Mode&0o060 != 0o060 { | ||
| if err := unix.Fchmod(lockFD, stat.Mode&0o777|0o060); err != nil { | ||
| // stat.Mode is uint16 on darwin; convert for Fchmod portability. | ||
| if err := unix.Fchmod(lockFD, uint32(stat.Mode)&0o777|0o060); err != nil { |
There was a problem hiding this comment.
[P2] Keep the Darwin conversion lint-clean on Linux
The cast is needed on Darwin, but on Linux Stat_t.Mode is already uint32, so the required unconvert check rejects this line. Both Source Checks (make check) and Dashboard CI fail here at this commit, before the backend tests run. Please retain the portability fix and add a narrowly scoped //nolint:unconvert with the Darwin type rationale (or an equivalent platform-aware helper), then rerun both checks.
There was a problem hiding this comment.
Thanks for the review! Fixed in the latest commit.
Signed-off-by: jiawei.liu <jiawei.liu@daocloud.io>
66150dc to
1694eea
Compare
Xunzhuo
left a comment
There was a problem hiding this comment.
Thanks for the focused lint fix. Both Darwin cross-builds and the Linux socket and lock regressions pass. Native macOS runtime tests were not run; no blockers found.
Closes #4828
Purpose
Fix two macOS-only failures in
dashboard/backendreported in #4828:handlers/runtime_config_lock_unix.gopassesstat.Modedirectly tounix.Fchmod. The file builds for all non-Windows platforms(
//go:build !windows), butunix.Stat_t.Modeisuint16on darwinwhile
unix.Fchmodexpectsuint32, so the wholehandlerspackagefails to compile on macOS.
handlers/instance_test.gojoinst.TempDir()withcontrol.sock.t.TempDir()embeds the full test name, which exceeds the 104-bytesun_pathlimit for unix sockets on macOS, soTestInstanceUnixStatusAndModelsandTestInstanceEngineUsesPersistentFrontendForInferencealways failwith
bind: invalid argument.Fixes: convert the mode with
uint32(stat.Mode)(an identity conversion onlinux/freebsd where
Modeis alreadyuint32), and create the test socketin a short
os.MkdirTemp("", "sr")directory instead oft.TempDir().Affected module:
dashboard/backendonly. No behavior change on Linux.Windows is unaffected (file excluded by build tag).
Test Plan
Test Result
Before (macOS, darwin/arm64):
With only the compile fix applied, the two socket tests still fail:
After this PR:
go build ./...passes;go test ./handlers/ -count=1reportsok(all tests pass)go buildandgo vetpass on linux/amd64, linux/arm64, darwin/amd64, darwin/arm64, freebsd/amd64Remaining risk: none expected;
uint32(stat.Mode)is an identity conversionon platforms where
Modeis alreadyuint32.