ceos/unsloth
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: address Codex review on WoA deferral, uninstall port-kill, and build preservation

Browse source 
Three valid findings from the 06-22 Codex review:

1. provision_llama_cuda.sh: when $LLAMA_DIR holds a .git checkout (a prior CPU
   source build), the whole-dir backup was skipped, so a failed CUDA rebuild's
   'rm -rf build' destroyed the working CPU server with nothing to restore --
   leaving NO llama-server despite the 'keeps the existing server' promise (a
   thermal shutdown mid-build is a real failure mode on this hardware). Back up
   build/bin before the rebuild and restore it on total failure; idempotent and
   self-cleaning (never overwrites a freshly built server). Verified both paths.

2. uninstall.ps1: 'fuser -k 8888/tcp' killed ANY listener on 8888 (Jupyter et al.
   default to it), not just Studio. Now only kills a PID whose /proc/cmdline is
   under /root/.unsloth -- matching the adjacent pkill scoping.

3. setup.sh: the 'defer to background CUDA build' branch fired even on a direct
   in-WSL 'unsloth studio update', where install.ps1 never launched a background
   builder -- so the footer claimed a build was running while nothing built. Gate
   it on UNSLOTH_WSL_LLAMA_DEFERRED=1 (set only by install.ps1, and already read
   elsewhere in setup.sh); a direct run now falls through to a real CPU build.

bash -n + PS parse clean; the common install.ps1 WoA path (prebuilt success,
deferred flag set) is unaffected.
This commit is contained in:
Daniel Han 2026-06-21 22:24:41 -07:00
commit fa44cb8c44
3 changed files with 45 additions and 13 deletions
Download patch file Download diff file Expand all files Collapse all files

33
studio/scripts/provision_llama_cuda.sh
View file

@ -172,6 +172,30 @@ if [ ! -d "$LLAMA_DIR/.git" ]; then
fi
cd "$LLAMA_DIR" || { _restore_prev; exit 0; }
# When rebuilding in-place over an existing git checkout, the whole-dir backup above
# was skipped (_LLAMA_BAK empty) -- but build/ may already hold a working (e.g. CPU)
# llama-server from a prior setup.sh source build. The wipe-on-failure paths below
# would destroy it with nothing to restore, leaving NO server despite the "keeps the
# existing server" promise (a thermal shutdown mid-CUDA-build is a real failure mode
# here). Back up the existing binaries so a failed rebuild can put them back. Only
# bin/ (server + dlopen-ed backends) is needed; cheap since any pre-existing server
# here is the non-CUDA fallback (a CUDA one would have exited at step 0).
_BUILD_BAK=""
if [ -z "$_LLAMA_BAK" ] && [ -x "$SERVER" ]; then
_BUILD_BAK="${LLAMA_DIR}.binbak.$$"
rm -rf "$_BUILD_BAK" 2>/dev/null
cp -a "$LLAMA_DIR/build/bin" "$_BUILD_BAK" 2>/dev/null || _BUILD_BAK=""
fi
_restore_build() {
if [ -n "$_BUILD_BAK" ] && [ -e "$_BUILD_BAK" ] && [ ! -x "$SERVER" ]; then
mkdir -p "$LLAMA_DIR/build" 2>/dev/null
rm -rf "$LLAMA_DIR/build/bin" 2>/dev/null
mv "$_BUILD_BAK" "$LLAMA_DIR/build/bin" 2>/dev/null && log "restored previous llama-server (rebuild failed)"
fi
[ -n "$_BUILD_BAK" ] && rm -rf "$_BUILD_BAK" 2>/dev/null
_BUILD_BAK=""
}
log "building CUDA llama.cpp (arch=$CUDA_ARCH, host=$HCXX) - this takes a few minutes..."
_cmake_configure() {
cmake -S . -B build -DCMAKE_BUILD_TYPE=Release \
@ -185,7 +209,7 @@ _cmake_configure() {
if ! _cmake_configure; then
log "stale/incompatible CMake cache detected; wiping build dir for a clean CUDA configure"
rm -rf build
_cmake_configure || { log "cmake configure failed"; cd /; _restore_prev; exit 0; }
_cmake_configure || { log "cmake configure failed"; cd /; _restore_build; _restore_prev; exit 0; }
fi
# Also builds the targets unsloth-zoo's GGUF exporter needs (llama-mtmd-cli,
# llama-gguf-split). Jobs default to ~half the cores (full -j(nproc) CUDA builds
@ -226,10 +250,13 @@ if ! _cmake_build; then
# (undefined ggml_cuda_op_* refs); wipe and rebuild clean.
log "build failed (likely interrupted/partial); wiping build dir and rebuilding clean"
rm -rf build
_cmake_configure || { log "cmake configure failed"; cd /; _restore_prev; exit 0; }
_cmake_build || { log "cmake build failed"; cd /; _restore_prev; exit 0; }
_cmake_configure || { log "cmake configure failed"; cd /; _restore_build; _restore_prev; exit 0; }
_cmake_build || { log "cmake build failed"; cd /; _restore_build; _restore_prev; exit 0; }
fi
_cmake_build_extras
# Drop the backup on a successful build, or restore the prior server if the rebuild
# yielded none (idempotent; only restores when $SERVER is missing).
_restore_build
if is_cuda_server "$SERVER"; then
log "CUDA llama-server ready: $SERVER"