Merge pull request #77 from ausimian/fix/atomic-mlx-install

Make the MLX build robust and reorganise emily's caches

Author: ausimian

.github/workflows/ci.yml

@@ -38,6 +38,12 @@ jobs:
     env:
       MIX_ENV: test
       EMILY_MLX_VARIANT: ${{ matrix.variant }}
+      # Pin the cache to the path the actions/cache step below
+      # populates. Without it, mix.exs's macOS default of
+      # DARWIN_USER_CACHE_DIR (`/private/var/folders/...`) misses
+      # the restored cache and rebuilds MLX from scratch every run.
+      # mix.exs runs `Path.expand` on this, so `~` resolves correctly.
+      EMILY_CACHE: ~/Library/Caches/emily
     steps:
       - uses: actions/checkout@v6
 
@@ -114,6 +120,12 @@ jobs:
     env:
       MIX_ENV: test
       EMILY_MLX_VARIANT: ${{ matrix.variant }}
+      # Pin the cache to the path the actions/cache step below
+      # populates. Without it, mix.exs's macOS default of
+      # DARWIN_USER_CACHE_DIR (`/private/var/folders/...`) misses
+      # the restored cache and rebuilds MLX from scratch every run.
+      # mix.exs runs `Path.expand` on this, so `~` resolves correctly.
+      EMILY_CACHE: ~/Library/Caches/emily
     steps:
       - uses: actions/checkout@v6
 

.github/workflows/release-nif.yml

@@ -85,6 +85,11 @@ jobs:
       MIX_ENV: dev
       EMILY_MLX_VARIANT: ${{ matrix.variant }}
       VERSION: ${{ needs.prepare-release.outputs.version }}
+      # Pin the cache to the path the actions/cache step below
+      # populates. Without it, mix.exs's macOS default of
+      # DARWIN_USER_CACHE_DIR (`/private/var/folders/...`) misses
+      # the restored cache and rebuilds MLX from scratch every run.
+      EMILY_CACHE: ~/Library/Caches/emily
     steps:
       - uses: actions/checkout@v6
 

Makefile

@@ -2,7 +2,7 @@ PRIV_DIR       := $(MIX_APP_PATH)/priv
 NIF_SO         := $(PRIV_DIR)/libemily.so
 METALLIB       := $(PRIV_DIR)/mlx.metallib
 
-BUILD_DIR := $(EMILY_CACHE_DIR)/build-$(EMILY_VERSION)
+BUILD_DIR := $(MIX_APP_PATH)/obj
 
 # Sources — include ops/* and any other subdirs under c_src.
 SOURCES := $(shell find c_src -name '*.cpp')

RELEASE.md

@@ -16,6 +16,19 @@
   set `config :emily, variant: :jit` in their own
   `config/config.exs`. The old `:mlx_variant` key and
   `config/local.exs` override are gone.
+- macOS default cache location moves from `~/Library/Caches/emily/`
+  to `DARWIN_USER_CACHE_DIR` (`/private/var/folders/<hash>/C/emily`)
+  — the per-user sandboxed cache root Apple's own sandboxed apps
+  use. Persistent across reboots, lives outside `~/Library/`.
+  Linux / Windows still use the XDG convention. Override via
+  `EMILY_CACHE`. Existing macOS users can `rm -rf
+  ~/Library/Caches/emily/` to reclaim the orphaned data after
+  upgrade.
+- NIF object files move from the user-level cache to
+  `$(MIX_APP_PATH)/obj/` (i.e. `_build/<env>/lib/emily/obj/`). As a
+  consequence, plain `mix clean` now correctly removes them via the
+  existing Makefile rule — they were previously left behind because
+  `make clean` didn't see the cache-dir env vars.
 
 ### Added
 
@@ -24,6 +37,40 @@
   uploads tarball + `.sha256` sidecar to a draft GitHub release.
   `workflow_dispatch` is also wired for out-of-band rebuilds
   (artefacts go to workflow storage; the release is untouched).
+- `mix clean.mlx` — wipes the MLX install dir(s) under the cache.
+  Plain `mix clean` deliberately preserves them since rebuilding
+  MLX from source is ~5-7 minutes.
+
+### Fixed
+
+- MLX source builds are now atomic. The build script installs into
+  `${PREFIX}.staging` and only `mv`s onto the final path after the
+  artefact sanity checks pass; an EXIT trap wipes the scratch dirs
+  on failure. Previously, an interrupted build (Ctrl-C, killed
+  process, concurrent run) left an empty install dir that
+  subsequent `mix compile` runs misread as "MLX is already
+  installed", silently skipping the build and bombing out in
+  `elixir_make` with `make: *** No rule to make target
+  '.../mlx.metallib'`. The compile-time check now requires both
+  `lib/libmlx.a` and `lib/mlx.metallib` to be present before
+  trusting the dir.
+- Concurrent invocations of `build-mlx.sh` against the same install
+  prefix are now serialised via a `mkdir`-based lock with
+  stale-PID reclaim. ElixirLS uses its own build path
+  (`.elixir_ls/build/...`) so an LSP-driven `mix compile` and a CLI
+  `mix compile.emily_mlx --force` lock on *different*
+  `Mix.Project.with_build_lock` keys and freely raced into the same
+  MLX cache dir, clobbering each other's `${PREFIX}.build/`
+  mid-build and surfacing as `clang ... Rename failed: ... No such
+  file or directory` during Metal-shader compilation.
+- CMake's FetchContent sub-build of metal_cpp / json / fmt during
+  configure runs with `CMAKE_BUILD_PARALLEL_LEVEL=1`, dodging a
+  race in its download → extract → rename → stamp-touch pipeline
+  that surfaced as `getcwd: cannot access parent directories`
+  followed by `cd: <dir>/_deps: No such file or directory`. The
+  main MLX build still runs at full NCPU jobs.
+- The MLX scratch build dir (`${PREFIX}.build`) is preserved on
+  configure failure so `CMakeError.log` survives for diagnostics.
 
 ### Removed
 

mix.exs

@@ -143,10 +143,27 @@ defmodule Emily.MixProject do
         "test"
       ],
       "compile.emily_mlx": &build_mlx/1,
-      "compile.emily_nif": &fetch_nif/1
+      "compile.emily_nif": &fetch_nif/1,
+      # The MLX install dir lives in the user-level cache and is
+      # *deliberately* preserved across `mix clean` (rebuilding from
+      # source is ~5–7 min). Wipe it explicitly with `mix clean.mlx`.
+      "clean.mlx": &clean_mlx/1
     ]
   end
 
+  defp clean_mlx(_args) do
+    case Path.wildcard(Path.join(cache_dir(), "mlx-*")) do
+      [] ->
+        Mix.shell().info("No MLX install dirs to clean in #{cache_dir()}")
+
+      dirs ->
+        for dir <- dirs do
+          File.rm_rf!(dir)
+          Mix.shell().info("Removed #{dir}")
+        end
+    end
+  end
+
   defp docs do
     [
       main: "readme",
@@ -196,19 +213,36 @@ defmodule Emily.MixProject do
       "MLX_DIR" => dir,
       "MLX_INCLUDE_DIR" => Path.join(dir, "include"),
       "MLX_LIB_DIR" => Path.join(dir, "lib"),
-      "FINE_INCLUDE_DIR" => Fine.include_dir(),
-      "EMILY_CACHE_DIR" => cache_dir(),
-      "EMILY_VERSION" => @version
+      "FINE_INCLUDE_DIR" => Fine.include_dir()
     }
   end
 
+  # Default cache location, override with `EMILY_CACHE`. On macOS we
+  # use `DARWIN_USER_CACHE_DIR` (`/private/var/folders/<hash>/C/emily`)
+  # — the per-user sandboxed cache root that Apple's own sandboxed apps
+  # use for transient-but-persistent state. It's per-user, persistent
+  # across reboots (unlike `/tmp`), and lives outside `~/Library/` so
+  # it's not subject to user-facing backup/sync tooling defaults.
+  # Linux / Windows fall back to the XDG convention.
   defp cache_dir do
     case System.get_env("EMILY_CACHE") do
-      nil -> :filename.basedir(:user_cache, ~c"emily") |> to_string()
+      nil -> default_cache_dir()
       dir -> Path.expand(dir)
     end
   end
 
+  defp default_cache_dir do
+    case :os.type() do
+      {:unix, :darwin} ->
+        {out, 0} = System.cmd("getconf", ["DARWIN_USER_CACHE_DIR"])
+        Path.join(String.trim(out), "emily")
+
+      _ ->
+        cache_home = System.get_env("XDG_CACHE_HOME") || Path.join(System.user_home!(), ".cache")
+        Path.join(cache_home, "emily")
+    end
+  end
+
   defp mlx_variant do
     case Application.get_env(:emily, :variant, :aot) do
       :aot -> "aot"
@@ -247,14 +281,23 @@ defmodule Emily.MixProject do
       File.rm_rf!(dir)
     end
 
-    if File.dir?(dir) do
+    if mlx_installed?(dir) do
       {:ok, []}
     else
+      File.rm_rf!(dir)
       build_mlx_from_source!(dir)
       {:ok, []}
     end
   end
 
+  # `build-mlx.sh` publishes via `mv staging prefix` only after the
+  # critical artefacts are in place, so a directory missing either of
+  # these is by definition a partial install — don't trust it.
+  defp mlx_installed?(dir) do
+    File.exists?(Path.join([dir, "lib", "libmlx.a"])) and
+      File.exists?(Path.join([dir, "lib", "mlx.metallib"]))
+  end
+
   defp build_mlx_from_source!(install_dir) do
     mlx_src = Path.expand("deps/mlx_src", File.cwd!())
 

scripts/build-mlx.sh

@@ -59,19 +59,96 @@ EOF
   fi
 fi
 
-# Keep the scratch build dir alongside the install prefix so `--force`
-# from mix.exs (which rm -rf's the install dir) doesn't orphan it.
+# Serialise concurrent script invocations against the same install
+# prefix. Mix.Project.with_build_lock can't help here: ElixirLS uses
+# its own build path (.elixir_ls/build/...) so an LSP-driven
+# `mix compile` and a CLI `mix compile.emily_mlx --force` lock on
+# *different* keys and freely race into the same MLX cache dir. Both
+# invocations would then rm each other's `${PREFIX}.build/` mid-build,
+# surfacing as `clang ... Rename failed: ... No such file or
+# directory` during Metal-shader compilation.
+#
+# `flock(1)` isn't shipped on macOS, so we use atomic `mkdir` as the
+# lock primitive. The lock dir is keyed on PREFIX, which both
+# contexts share. A PID file inside lets us reclaim a stale lock if
+# the previous holder died without cleanup.
 BUILD_DIR="${PREFIX}.build"
+STAGING="${PREFIX}.staging"
+LOCK_DIR="${PREFIX}.lock"
+LOCK_PID_FILE="${LOCK_DIR}/pid"
+
+mkdir -p "$(dirname "$LOCK_DIR")"
+
+acquired_lock=0
+printed_wait=0
+
+# Define cleanup + install the trap *before* the lock-acquisition loop
+# so that a concurrent-winner short-circuit (or any other early exit)
+# still releases LOCK_DIR. STAGING is a half-baked install — always
+# wipe. BUILD_DIR holds CMakeFiles/ and CMakeError.log on failure;
+# keep it on non-zero exit so diagnostics survive.
+cleanup() {
+  local exit_code=$?
+  rm -rf "$STAGING"
+  if (( acquired_lock == 1 )); then
+    rm -rf "$LOCK_DIR"
+  fi
+  if [[ $exit_code -eq 0 ]]; then
+    rm -rf "$BUILD_DIR"
+  else
+    echo "==> Build failed (exit ${exit_code}); preserving ${BUILD_DIR} for diagnostics" >&2
+  fi
+}
+trap cleanup EXIT
+
+while :; do
+  if mkdir "$LOCK_DIR" 2>/dev/null; then
+    echo "$$" > "$LOCK_PID_FILE"
+    acquired_lock=1
+    break
+  fi
 
-rm -rf "$BUILD_DIR" "$PREFIX"
-mkdir -p "$BUILD_DIR" "$PREFIX"
+  holder=""
+  [[ -r "$LOCK_PID_FILE" ]] && holder=$(cat "$LOCK_PID_FILE" 2>/dev/null || true)
+
+  if [[ -n "$holder" ]] && ! kill -0 "$holder" 2>/dev/null; then
+    echo "==> Reclaiming stale MLX-build lock (dead PID $holder)" >&2
+    rm -rf "$LOCK_DIR"
+    continue
+  fi
+
+  if (( printed_wait == 0 )); then
+    echo "==> Waiting for concurrent MLX build${holder:+ (PID $holder)} on ${PREFIX}" >&2
+    printed_wait=1
+  fi
+  sleep 1
+done
+
+# A concurrent winner may have completed the install while we waited
+# for the lock — re-check and short-circuit if so.
+if [[ -f "${PREFIX}/lib/libmlx.a" && -f "${PREFIX}/lib/mlx.metallib" ]]; then
+  echo "==> MLX already installed at ${PREFIX} (concurrent build won)"
+  exit 0
+fi
+
+rm -rf "$BUILD_DIR" "$STAGING"
+mkdir -p "$BUILD_DIR" "$STAGING"
 
 echo "==> Configuring MLX ${VERSION} (${VARIANT})"
-cmake \
+# Configure triggers `FetchContent_MakeAvailable` for metal_cpp / json /
+# fmt, which CMake implements via a recursive `cmake --build` of a tiny
+# sub-project per dep. Those sub-builds inherit `CMAKE_BUILD_PARALLEL_LEVEL`
+# and race on FetchContent's download → extract → rename → stamp-touch
+# pipeline when run in parallel — observed as `getcwd: cannot access
+# parent directories` followed by `cd: <dir>/_deps: No such file or
+# directory` (FetchContent renames out from under a sub-shell that the
+# parallel make spawned). Pin the env to 1 only for this invocation;
+# the main MLX build below still runs at NCPU jobs.
+CMAKE_BUILD_PARALLEL_LEVEL=1 cmake \
   -S "$MLX_SRC_DIR" \
   -B "$BUILD_DIR" \
   -DCMAKE_BUILD_TYPE=Release \
-  -DCMAKE_INSTALL_PREFIX="$PREFIX" \
+  -DCMAKE_INSTALL_PREFIX="$STAGING" \
   -DBUILD_SHARED_LIBS=OFF \
   -DMLX_BUILD_TESTS=OFF \
   -DMLX_BUILD_EXAMPLES=OFF \
@@ -86,21 +163,31 @@ NCPU="$(sysctl -n hw.ncpu)"
 echo "==> Building with ${NCPU} jobs"
 cmake --build "$BUILD_DIR" --parallel "$NCPU"
 
-echo "==> Installing into ${PREFIX}"
+echo "==> Installing into ${STAGING}"
 cmake --install "$BUILD_DIR"
 
 # MLX's Metal device loader looks for mlx.metallib colocated with the
 # binary. cmake --install places it under lib/ (via the install rules
-# in mlx/backend/metal/CMakeLists.txt); sanity-check both artefacts.
+# in mlx/backend/metal/CMakeLists.txt); sanity-check both artefacts
+# before we rename staging into place.
 for f in "lib/libmlx.a" "lib/mlx.metallib"; do
-  if [[ ! -f "${PREFIX}/${f}" ]]; then
-    echo "error: expected ${PREFIX}/${f} after cmake --install (build is incomplete)" >&2
+  if [[ ! -f "${STAGING}/${f}" ]]; then
+    echo "error: expected ${STAGING}/${f} after cmake --install (build is incomplete)" >&2
     exit 1
   fi
 done
 
-# Scratch build is large (~1.5 GB on the aot lane). mix.exs doesn't
-# need it once the install dir is populated.
-rm -rf "$BUILD_DIR"
+# Atomic publish: rename staging to prefix. If a racing build already
+# populated PREFIX, `mv` refuses to overwrite the non-empty target and
+# exits non-zero — defer to the winner, our trap cleans STAGING up.
+echo "==> Publishing to ${PREFIX}"
+if ! mv "$STAGING" "$PREFIX"; then
+  if [[ -f "${PREFIX}/lib/libmlx.a" && -f "${PREFIX}/lib/mlx.metallib" ]]; then
+    echo "==> ${PREFIX} already populated by a concurrent build; keeping it"
+  else
+    echo "error: failed to publish staging to ${PREFIX}" >&2
+    exit 1
+  fi
+fi
 
 echo "==> Done: ${PREFIX}"