From ec334e576583b9cced81e37ec65b548dbb1e051b Mon Sep 17 00:00:00 2001 From: Vyncint Ng Date: Sat, 8 Aug 2026 20:12:17 +0700 Subject: [PATCH 1/2] fix: keep the Homebrew symlink in the launchd plist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit setup resolved symlinks before recording the driver's path, so a cask install wrote Caskroom/openshell-driver-applecontainer//… into the launchd plist. `brew upgrade` deletes that directory, so the service would point at a path that no longer exists and the driver would stay down until setup ran again — silently, since nothing fails until a sandbox is created. Keep Homebrew's /bin symlink, which outlives every upgrade. Other symlinks are still resolved, so the plist does not depend on one staying put. Found on the machine right after installing 0.2.8 from the tap: setup logged binary=/opt/homebrew/Caskroom/openshell-driver-applecontainer/0.2.8/… The cask detection now lives in internal/hostsetup and is shared with update, which needs the same answer for the opposite reason. Signed-off-by: Vyncint Ng --- CHANGELOG.md | 8 ++ cmd/openshell-driver-applecontainer/update.go | 21 ++--- .../update_test.go | 25 ------ internal/hostsetup/homebrew.go | 38 ++++++++ internal/hostsetup/homebrew_test.go | 90 +++++++++++++++++++ internal/hostsetup/setup.go | 4 +- 6 files changed, 142 insertions(+), 44 deletions(-) create mode 100644 internal/hostsetup/homebrew.go create mode 100644 internal/hostsetup/homebrew_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 92fc30b..2d7d5c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +### Fixed + +- `setup` no longer pins the launchd service to a Homebrew version directory. It resolved symlinks + before writing the plist, so on a cask install the service pointed at + `Caskroom/openshell-driver-applecontainer//…` — a path the next `brew upgrade` deletes, + leaving the driver dead until `setup` ran again. The plist now keeps Homebrew's + `/bin` symlink, which survives upgrades; every other symlink is still resolved. + ## [0.2.8] - 2026-08-08 ### Added diff --git a/cmd/openshell-driver-applecontainer/update.go b/cmd/openshell-driver-applecontainer/update.go index 153af56..b6017f8 100644 --- a/cmd/openshell-driver-applecontainer/update.go +++ b/cmd/openshell-driver-applecontainer/update.go @@ -16,6 +16,8 @@ import ( "path/filepath" "strings" "time" + + "github.com/vyncint/openshell-driver-applecontainer/internal/hostsetup" ) const ( @@ -55,7 +57,9 @@ func runUpdate(args []string) int { // path is gone by the time setup runs. The symlink in /bin is not. setupPath := self - if cask, brewManaged := homebrewCask(self); brewManaged { + // Replacing a cask's staged binary in place would leave Homebrew believing + // it still has the version it staged, so let brew do the upgrade. + if cask, brewManaged := hostsetup.HomebrewCask(self); brewManaged { if *targetVersion != "" { log.Error("update: this install is managed by Homebrew, which only tracks the tap's latest release. "+ "To pin a version, remove it (`brew uninstall --cask "+cask+"`) and install with install.sh", @@ -105,21 +109,6 @@ func runUpdate(args []string) int { return 0 } -// homebrewCask reports whether binPath is a binary staged by a Homebrew cask, -// and the cask's token. Cask artifacts live at -// /Caskroom///, symlinked into -// /bin — so replacing that file in place would leave Homebrew -// believing it still has the version it staged. -func homebrewCask(binPath string) (string, bool) { - parts := strings.Split(filepath.ToSlash(binPath), "/") - for i, p := range parts { - if p == "Caskroom" && i+1 < len(parts) && parts[i+1] != "" { - return parts[i+1], true - } - } - return "", false -} - // selfUpdate downloads release `version`, verifies its checksum, and replaces // binPath (the running binary) in place. func selfUpdate(log *slog.Logger, binPath, version string) error { diff --git a/cmd/openshell-driver-applecontainer/update_test.go b/cmd/openshell-driver-applecontainer/update_test.go index 8bfb004..0299504 100644 --- a/cmd/openshell-driver-applecontainer/update_test.go +++ b/cmd/openshell-driver-applecontainer/update_test.go @@ -21,31 +21,6 @@ func TestReleaseArchiveName(t *testing.T) { } } -func TestHomebrewCask(t *testing.T) { - cases := []struct { - path string - cask string - brew bool - }{ - {"/opt/homebrew/Caskroom/openshell-driver-applecontainer/0.2.8/openshell-driver-applecontainer", - "openshell-driver-applecontainer", true}, - // Intel prefix, and a version directory that itself looks like a path. - {"/usr/local/Caskroom/some-tool/1.2.3_1/some-tool", "some-tool", true}, - // Installed by install.sh: a real file in the brew prefix, not a cask. - {"/opt/homebrew/bin/openshell-driver-applecontainer", "", false}, - {"/usr/local/bin/openshell-driver-applecontainer", "", false}, - // A directory merely named Caskroom, with nothing under it. - {"/tmp/Caskroom", "", false}, - {"", "", false}, - } - for _, tc := range cases { - cask, brew := homebrewCask(tc.path) - if brew != tc.brew || cask != tc.cask { - t.Errorf("homebrewCask(%q) = (%q, %v), want (%q, %v)", tc.path, cask, brew, tc.cask, tc.brew) - } - } -} - // makeTarGz writes a gzipped tar of name->content and returns its path. func makeTarGz(t *testing.T, dir string, entries map[string][]byte) string { t.Helper() diff --git a/internal/hostsetup/homebrew.go b/internal/hostsetup/homebrew.go new file mode 100644 index 0000000..0badfe7 --- /dev/null +++ b/internal/hostsetup/homebrew.go @@ -0,0 +1,38 @@ +package hostsetup + +import ( + "path/filepath" + "strings" +) + +// HomebrewCask reports whether binPath is a binary staged by a Homebrew cask, +// and the cask's token. Cask artifacts live at +// /Caskroom/// and are symlinked into +// /bin, so the staged path is pinned to one version while the +// symlink is not. +func HomebrewCask(binPath string) (string, bool) { + parts := strings.Split(filepath.ToSlash(binPath), "/") + for i, p := range parts { + if p == "Caskroom" && i+1 < len(parts) && parts[i+1] != "" { + return parts[i+1], true + } + } + return "", false +} + +// resolveBinPath canonicalises the running binary's path for the launchd plist. +// Symlinks are resolved so the plist does not depend on one staying put — with +// one exception: a Homebrew cask's symlink, which resolves into a version +// directory that the next `brew upgrade` deletes. Recording that would leave +// the service pointing at a path that no longer exists; the /bin +// symlink outlives every upgrade, so keep it. +func resolveBinPath(exe string) string { + resolved, err := filepath.EvalSymlinks(exe) + if err != nil { + return exe + } + if _, isCask := HomebrewCask(resolved); isCask { + return exe + } + return resolved +} diff --git a/internal/hostsetup/homebrew_test.go b/internal/hostsetup/homebrew_test.go new file mode 100644 index 0000000..3a905a3 --- /dev/null +++ b/internal/hostsetup/homebrew_test.go @@ -0,0 +1,90 @@ +package hostsetup + +import ( + "os" + "path/filepath" + "testing" +) + +func TestHomebrewCask(t *testing.T) { + cases := []struct { + path string + cask string + brew bool + }{ + {"/opt/homebrew/Caskroom/openshell-driver-applecontainer/0.2.8/openshell-driver-applecontainer", + "openshell-driver-applecontainer", true}, + // Intel prefix, and a version directory carrying a revision suffix. + {"/usr/local/Caskroom/some-tool/1.2.3_1/some-tool", "some-tool", true}, + // Installed by install.sh: a real file in the brew prefix, not a cask. + {"/opt/homebrew/bin/openshell-driver-applecontainer", "", false}, + {"/usr/local/bin/openshell-driver-applecontainer", "", false}, + // A directory merely named Caskroom, with nothing under it. + {"/tmp/Caskroom", "", false}, + {"", "", false}, + } + for _, tc := range cases { + cask, brew := HomebrewCask(tc.path) + if brew != tc.brew || cask != tc.cask { + t.Errorf("HomebrewCask(%q) = (%q, %v), want (%q, %v)", tc.path, cask, brew, tc.cask, tc.brew) + } + } +} + +// A Homebrew cask's bin symlink must survive into the launchd plist as-is: +// resolving it would pin the plist to a version directory that the next +// `brew upgrade` deletes. +func TestResolveBinPathKeepsHomebrewSymlink(t *testing.T) { + root := t.TempDir() + staged := filepath.Join(root, "Caskroom", "driver", "0.2.8") + if err := os.MkdirAll(staged, 0o755); err != nil { + t.Fatal(err) + } + target := filepath.Join(staged, "driver") + if err := os.WriteFile(target, []byte("binary"), 0o755); err != nil { // #nosec G306 -- stand-in for an executable + t.Fatal(err) + } + bin := filepath.Join(root, "bin") + if err := os.MkdirAll(bin, 0o755); err != nil { + t.Fatal(err) + } + link := filepath.Join(bin, "driver") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + + if got := resolveBinPath(link); got != link { + t.Errorf("resolveBinPath(%q) = %q, want the symlink itself", link, got) + } +} + +// Every other symlink is still resolved, so the plist does not depend on one +// staying put. +func TestResolveBinPathResolvesOtherSymlinks(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "real-driver") + if err := os.WriteFile(target, []byte("binary"), 0o755); err != nil { // #nosec G306 -- stand-in for an executable + t.Fatal(err) + } + link := filepath.Join(root, "link-driver") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + + want, err := filepath.EvalSymlinks(target) + if err != nil { + t.Fatal(err) + } + if got := resolveBinPath(link); got != want { + t.Errorf("resolveBinPath(%q) = %q, want %q", link, got, want) + } +} + +// A path that cannot be resolved (a dangling symlink, say) is returned as-is +// rather than turning setup into a hard failure. +func TestResolveBinPathUnresolvable(t *testing.T) { + missing := filepath.Join(t.TempDir(), "does-not-exist") + if got := resolveBinPath(missing); got != missing { + t.Errorf("resolveBinPath(%q) = %q, want it unchanged", missing, got) + } +} diff --git a/internal/hostsetup/setup.go b/internal/hostsetup/setup.go index 9b4b677..2516d1d 100644 --- a/internal/hostsetup/setup.go +++ b/internal/hostsetup/setup.go @@ -65,9 +65,7 @@ func New(rt backend.Runtime, log *slog.Logger) (*Setup, error) { if err != nil { return nil, fmt.Errorf("resolve driver binary path: %w", err) } - if resolved, err := filepath.EvalSymlinks(bin); err == nil { - bin = resolved - } + bin = resolveBinPath(bin) return &Setup{ RT: rt, Log: log, From a2b2bfcd44f4a207bd7543bbf90b117b196bb539 Mon Sep 17 00:00:00 2001 From: Vyncint Ng Date: Sat, 8 Aug 2026 20:19:39 +0700 Subject: [PATCH 2/2] fix: keep the Homebrew symlink in the launchd plist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit setup resolved symlinks before recording the driver's path, so a cask install wrote Caskroom/openshell-driver-applecontainer//… into the launchd plist — a path tied to one version. Keep Homebrew's /bin symlink, which is stable across versions; other symlinks are still resolved, so the plist does not depend on one staying put. Also document what testing the upgrade path turned up: `brew upgrade` REMOVES the driver's launchd service. Homebrew replaces a cask by uninstalling the old version first, and that runs the cask's `uninstall launchctl:` directive — UPGRADE_REINSTALL_SKIP_DIRECTIVES skips only `signal`. Keeping the directive is deliberate: without it a real `brew uninstall` leaves a loaded agent respawning a binary that no longer exists. So `setup` after an upgrade is mandatory, not advisory, and the README and caveats now say so plainly. The cask detection now lives in internal/hostsetup and is shared with update, which needs the same answer for the opposite reason. Signed-off-by: Vyncint Ng --- .goreleaser.yaml | 11 ++++++++--- CHANGELOG.md | 14 +++++++++++--- README.md | 8 ++++++-- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/.goreleaser.yaml b/.goreleaser.yaml index 525e32d..7a096d8 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -123,7 +123,11 @@ homebrew_casks: end uninstall: # Stop and unload the launchd agent `setup` installed, before the binary - # it points at disappears. + # it points at disappears. Homebrew runs this on UPGRADE as well — only + # `signal` is skipped there — so an upgrade removes the driver's service + # and `setup` has to put it back. Dropping the directive would avoid that + # but leave a real uninstall with a loaded agent respawning a binary that + # no longer exists, which is worse. launchctl: - local.openshell-driver-applecontainer # `brew uninstall --zap` territory: state a plain uninstall must not touch. @@ -147,5 +151,6 @@ homebrew_casks: openshell-driver-applecontainer setup - Re-run `setup` after every `brew upgrade` too: it restarts the launchd - service onto the new binary. + Run it after every upgrade as well. Homebrew removes a cask's launchd + service when it replaces the old version, so until `setup` runs again + the driver is installed but not running. diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d7d5c1..92be5fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,17 @@ All notable changes to this project are documented here. The format follows - `setup` no longer pins the launchd service to a Homebrew version directory. It resolved symlinks before writing the plist, so on a cask install the service pointed at - `Caskroom/openshell-driver-applecontainer//…` — a path the next `brew upgrade` deletes, - leaving the driver dead until `setup` ran again. The plist now keeps Homebrew's - `/bin` symlink, which survives upgrades; every other symlink is still resolved. + `Caskroom/openshell-driver-applecontainer//…`, which `brew upgrade` deletes. The plist + now keeps Homebrew's `/bin` symlink, which is stable across versions; every other + symlink is still resolved, so the plist does not depend on one staying put. + +### Changed + +- Documented that **`brew upgrade` removes the driver's launchd service**, so `setup` must follow + it. Homebrew replaces a cask by uninstalling the old version first, which runs the cask's + `uninstall launchctl:` directive — Homebrew skips only `signal` on upgrade. Keeping the + directive is deliberate: without it a real `brew uninstall` would leave a loaded agent + respawning a binary that no longer exists. ## [0.2.8] - 2026-08-08 diff --git a/README.md b/README.md index 7aff4d2..3eb1e31 100644 --- a/README.md +++ b/README.md @@ -91,8 +91,12 @@ Homebrew, so take it from [its releases](https://github.com/apple/container/rele install. `setup` is still yours to run: Homebrew places the binary, `setup` wires the launchd service, -gateway configuration, vmnet network and images. Re-run it after every `brew upgrade` so the -service restarts on the new binary. +gateway configuration, vmnet network and images. + +**Run `setup` after every upgrade too — it is not optional.** Homebrew replaces a cask by +uninstalling the old version first, and that runs the cask's `uninstall launchctl:` directive +(only `signal` is skipped on upgrade), so an upgrade *removes* the driver's launchd service. The +driver is then installed but not running until `setup` puts it back. `update` does this for you. ```sh brew upgrade --cask openshell-driver-applecontainer && openshell-driver-applecontainer setup