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 92fc30b..92be5fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,22 @@ 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//…`, 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 ### Added 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 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,