From c2cecd9383dd22e5c003150c61db487c4a5135a7 Mon Sep 17 00:00:00 2001 From: Chili Palmer Date: Mon, 22 Jun 2026 13:44:55 +0200 Subject: [PATCH] fix: app bundling correcrted --- AGENTS.md | 13 ++ lib/bds/mac_bundle.ex | 52 ++++-- lib/bds/mac_bundle/dylibs.ex | 293 ++++++++++++++++++++---------- lib/mix/tasks/bds.bundle.macos.ex | 10 + test/bds/mac_bundle_test.exs | 99 ++++++++++ 5 files changed, 349 insertions(+), 118 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 35e5e62..8b5933a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -102,6 +102,19 @@ This document provides context and best practices for GitHub Copilot when workin --- +## ⚠️ MANDATORY: macOS Bundle Must Be Fully Standalone + +**The built `BDS2.app` MUST run on a Mac without Homebrew.** + +- No Mach-O inside the `.app` may reference a Homebrew/local path (`/opt/homebrew`, `/usr/local`) +- Every external dylib reachable from the release's NIFs must be copied into `Contents/Frameworks` and rewritten to `@loader_path`-relative install names (`BDS.MacBundle.Dylibs`) +- The same physical dylib referenced under multiple names must be deduped (one real file + symlinks) so dyld loads a single image — duplicate images cause duplicate Objective-C class / wx event-table registrations +- `BDS.MacBundle.verify_standalone/1` runs after relocation and fails the build if any external reference remains — do not weaken or bypass this gate + +> **The bundle must be self-contained. No outside references. No exceptions.** + +--- + ## ⚠️ MANDATORY: Proper I18N for UI and Rendering Text **All user-facing text MUST follow proper i18n patterns.** diff --git a/lib/bds/mac_bundle.ex b/lib/bds/mac_bundle.ex index d665e68..7c08d97 100644 --- a/lib/bds/mac_bundle.ex +++ b/lib/bds/mac_bundle.ex @@ -96,14 +96,6 @@ defmodule BDS.MacBundle do end end - @doc "Locate wxWidgets NIFs (`wxe_driver.so`) inside a release tree." - @spec wx_nif_paths(String.t()) :: [String.t()] - def wx_nif_paths(release_dir) do - release_dir - |> Path.join("lib/wx-*/priv/wxe_driver.so") - |> Path.wildcard() - end - defp write_launcher(path) do with :ok <- File.write(path, launcher()) do File.chmod(path, 0o755) @@ -121,19 +113,45 @@ defmodule BDS.MacBundle do defp relocate_dylibs(release_dir, opts) do app = Path.join(opts[:output_dir], @app_dir_name) frameworks = Path.join(app, "Contents/Frameworks") - nifs = wx_nif_paths(Path.join(app, "Contents/Resources/rel")) + rel = Path.join(app, "Contents/Resources/rel") - if nifs == [] do - {:error, {:wx_nif_not_found, release_dir}} - else - Enum.reduce_while(nifs, :ok, fn nif, :ok -> - prefix = "@loader_path/" <> relative_up(Path.dirname(nif), frameworks) + # Every loose Mach-O in the release (NIFs, beam.smp, …) may link a Homebrew + # dylib, so relocate the closure reachable from all of them — not just wx. + case macho_files(rel) do + [] -> + {:error, {:no_release_binaries, release_dir}} - case Dylibs.bundle(nif, frameworks, prefix) do - {:ok, _copied} -> {:cont, :ok} - error -> {:halt, error} + nifs -> + prefix_for = fn nif -> "@loader_path/" <> relative_up(Path.dirname(nif), frameworks) end + + with {:ok, _copied} <- Dylibs.bundle(nifs, frameworks, prefix_for) do + verify_standalone(app) end + end + end + + @doc """ + Fail the build unless the bundle is self-contained: no Mach-O under the `.app` + may reference a Homebrew/local (`/opt/homebrew`, `/usr/local`) path. This is a + hard requirement — the app must run on a Mac without Homebrew installed. + """ + @spec verify_standalone(String.t()) :: :ok | {:error, {:external_refs, [{String.t(), String.t()}]}} + def verify_standalone(app) do + offenders = + app + |> macho_files() + |> Enum.flat_map(fn file -> + external_refs(file, ["-L"], &Dylibs.parse_otool/1) ++ + external_refs(file, ["-l"], &Dylibs.parse_rpaths/1) end) + + if offenders == [], do: :ok, else: {:error, {:external_refs, offenders}} + end + + defp external_refs(file, otool_args, parse) do + case System.cmd("otool", otool_args ++ [file], stderr_to_stdout: true) do + {out, 0} -> out |> parse.() |> Enum.filter(&Dylibs.external?/1) |> Enum.map(&{file, &1}) + _ -> [] end end diff --git a/lib/bds/mac_bundle/dylibs.ex b/lib/bds/mac_bundle/dylibs.ex index dc51f86..b4ec37f 100644 --- a/lib/bds/mac_bundle/dylibs.ex +++ b/lib/bds/mac_bundle/dylibs.ex @@ -1,16 +1,32 @@ defmodule BDS.MacBundle.Dylibs do @moduledoc """ - Makes the bundled Elixir release self-contained on macOS by relocating the - non-system dynamic libraries the wxWidgets NIF (`wxe_driver.so`) links against. + Makes the bundled Elixir release **fully self-contained** on macOS: after + `bundle/3` runs, no Mach-O binary in the `.app` references a Homebrew/local + (`/opt/homebrew`, `/usr/local`) path, so the app launches on a clean Mac with + no Homebrew installed. - Erlang's `:wx` driver is built against the wxWidgets dylibs of whatever host - produced the OTP install (typically Homebrew under `/opt/homebrew`), so a - release copied to a clean Mac would fail to load `:wx`. We copy every external - dylib (and its transitive deps) into `Contents/Frameworks/`, then rewrite the - install names with `install_name_tool` to `@executable_path/../Frameworks/...`. + Erlang NIFs (`wxe_driver.so`, `crypto.so`, …) are built against whatever + produced the host OTP install — typically Homebrew. We walk every Mach-O in + the release, copy the transitive closure of external dylibs into + `Contents/Frameworks/`, and rewrite every external install name to a + `@loader_path`-relative one. + + Two robustness points the previous (per-NIF, exact-path) version missed: + + * **Spelling-independent rewriting.** The same physical dylib is referenced + under different absolute spellings — a NIF links the symlink name + (`.../opt/wxwidgets@3.2/lib/libwx_..._core-3.2.dylib`) while inter-library + deps link the versioned name (`.../Cellar/.../libwx_..._core-3.2.0.4.3.dylib`). + We rewrite by *basename* (`relink_changes/2`), so every spelling of an + external dep is caught regardless of how it was written. + + * **Inode dedup via symlinks.** Those two names are one file. We copy the + real bytes once and make the other name a symlink to it, so dyld loads a + single image — otherwise both copies load and you get duplicate + Objective-C class / wx event-table registrations. The `otool`/`install_name_tool` parsing and argument building are pure so they - can be unit-tested; `bundle/2` performs the actual filesystem + tool work. + can be unit-tested; `bundle/3` performs the actual filesystem + tool work. """ # Homebrew prefixes whose libraries must be copied into the bundle. Anything @@ -55,114 +71,147 @@ defmodule BDS.MacBundle.Dylibs do end @doc """ - Copy every external dylib reachable from `nif_path` into `frameworks_dir` and - rewrite all install names (in the NIF and the copied dylibs) to point inside - the bundle. Returns `{:ok, copied_paths}`. - - References are made `@loader_path`-relative so they resolve against the binary - that triggers the load — independent of where the host process executable - lives (the release runs `beam.smp`, not the bundle launcher) and without any - `DYLD_*` env reliance: - - * the NIF references each dylib as `/`, where - `nif_loader_prefix` is the `@loader_path/../…/Frameworks` path from the - NIF's directory to `frameworks_dir`; - * the copied dylibs live together, so they reference each other (and their - own id) as `@loader_path/`. + Extract the `LC_RPATH` search paths from `otool -l` output. Used to find and + strip Homebrew/local rpaths — an `@rpath` dep would otherwise resolve outside + the bundle, breaking standalone even though `otool -L` looks clean. """ - @spec bundle(String.t(), String.t(), String.t()) :: + @spec parse_rpaths(String.t()) :: [String.t()] + def parse_rpaths(output) when is_binary(output) do + output + |> String.split("\n") + |> Enum.reduce({false, []}, fn line, {in_rpath?, acc} -> + line = String.trim(line) + + cond do + line == "cmd LC_RPATH" -> + {true, acc} + + in_rpath? and String.starts_with?(line, "path ") -> + path = + line + |> String.replace_prefix("path ", "") + |> String.replace(~r/\s+\(offset \d+\)$/, "") + + {false, [path | acc]} + + true -> + {in_rpath?, acc} + end + end) + |> elem(1) + |> Enum.reverse() + end + + @doc """ + Build `{old, new}` rewrites for a binary's `otool -L` dependency list: every + external dep becomes `/`. System (`/usr/lib`, `/System`) and + already-relocatable (`@rpath`/`@loader_path`/`@executable_path`) deps are + dropped. Rewriting by basename — not by the exact source path — is what makes + this catch every absolute spelling of the same library. + """ + @spec relink_changes([String.t()], String.t()) :: [{String.t(), String.t()}] + def relink_changes(deps, prefix) when is_list(deps) do + deps + |> Enum.filter(&external?/1) + |> Enum.map(fn dep -> {dep, prefix <> "/" <> Path.basename(dep)} end) + end + + @doc """ + Relocate every external dylib reachable from `nifs` into `frameworks_dir` and + rewrite all install names so the bundle is self-contained. + + `nifs` is the list of Mach-O binaries in the release that may link external + libs (NIFs, `beam.smp`, …). `prefix_for` maps a NIF to its `@loader_path`-based + prefix to `frameworks_dir` (the NIF runs from `beam.smp`, so references are + `@loader_path`-relative to the NIF's own location, independent of `DYLD_*`). + + Copied dylibs share `frameworks_dir`, so they reference each other (and their + own id) as `@loader_path/`. Returns `{:ok, real_copies}`. + """ + @spec bundle([String.t()], String.t(), (String.t() -> String.t())) :: {:ok, [String.t()]} | {:error, term()} - def bundle(nif_path, frameworks_dir, nif_loader_prefix) do + def bundle(nifs, frameworks_dir, prefix_for) when is_function(prefix_for, 1) do File.mkdir_p!(frameworks_dir) - with {:ok, {_seen, externals}} <- collect(nif_path, %{}, []) do - externals = Enum.reverse(externals) + with {:ok, externals} <- collect(nifs, %{}) do + reals = materialize(externals, frameworks_dir) - {physical, logical_entries, _inode_map} = - externals - |> Enum.reduce({[], [], %{}}, fn src, {phys_acc, log_acc, inode_map} -> - dest = Path.join(frameworks_dir, Path.basename(src)) - - if File.exists?(dest) do - {phys_acc, [{src, dest} | log_acc], inode_map} - else - ino = file_inode(src) - - case Map.get(inode_map, ino) do - nil -> - File.cp!(src, dest) - File.chmod!(dest, 0o644) - {[{src, dest} | phys_acc], [{src, dest} | log_acc], - Map.put(inode_map, ino, dest)} - - existing_dest -> - # Same inode already copied under a different name. - # Point this logical entry to the physical copy. - {phys_acc, [{src, existing_dest} | log_acc], inode_map} - end - end - end) - - copied = Enum.reverse(logical_entries) - physical = Enum.reverse(physical) - - with :ok <- rewrite(nif_path, copied, nif_loader_prefix), - :ok <- rewrite_each(physical) do - {:ok, Enum.map(physical, &elem(&1, 1))} + with :ok <- relink_each(reals, fn _real -> "@loader_path" end, set_id: true), + :ok <- relink_each(nifs, prefix_for, set_id: false) do + {:ok, reals} end end end + # Transitive closure of external deps, keyed by basename (one source path per + # basename — different absolute spellings of the same leaf collapse here). + defp collect(binaries, acc) do + Enum.reduce_while(binaries, {:ok, acc}, fn bin, {:ok, acc} -> + case otool(bin) do + {:ok, deps} -> + deps + |> Enum.filter(&external?/1) + |> Enum.reduce_while({:ok, acc}, fn dep, {:ok, acc} -> + base = Path.basename(dep) + + if Map.has_key?(acc, base) do + {:cont, {:ok, acc}} + else + case collect([dep], Map.put(acc, base, dep)) do + {:ok, _} = ok -> {:cont, ok} + error -> {:halt, error} + end + end + end) + |> case do + {:ok, _} = ok -> {:cont, ok} + error -> {:halt, error} + end + + error -> + {:halt, error} + end + end) + end + + # Copy each external once (deduped by device+inode, which follows symlinks to + # the real file); additional basenames for the same file become symlinks so + # dyld loads a single image. Returns the real (non-symlink) copies. + # ponytail: basename collisions between *different* libraries would clobber; + # never happens for our deps — add content hashing if it ever does. + defp materialize(externals, frameworks_dir) do + {_seen, reals} = + Enum.reduce(externals, {%{}, []}, fn {base, src}, {seen, reals} -> + dest = Path.join(frameworks_dir, base) + inode = file_inode(src) + + case Map.get(seen, inode) do + nil -> + File.cp!(src, dest) + File.chmod!(dest, 0o644) + {Map.put(seen, inode, base), [dest | reals]} + + canonical -> + _ = File.rm(dest) + File.ln_s!(canonical, dest) + {seen, reals} + end + end) + + Enum.reverse(reals) + end + defp file_inode(path) do stat = File.stat!(path) {stat.major_device, stat.inode} end - # Depth-first transitive collection of external dependency paths. Returns - # `{:ok, {seen, acc}}` where `seen` is a plain map used as a set (a MapSet's - # opaque internals trip dialyzer when threaded through the recursion) and - # `acc` is the reverse-discovery-order path list. - defp collect(binary, seen, acc) do - case otool(binary) do - {:ok, deps} -> - deps - |> Enum.filter(&external?/1) - |> Enum.reduce_while({:ok, {seen, acc}}, fn dep, {:ok, {seen_acc, list_acc}} -> - if Map.has_key?(seen_acc, dep) do - {:cont, {:ok, {seen_acc, list_acc}}} - else - seen_acc = Map.put(seen_acc, dep, true) - case collect(dep, seen_acc, [dep | list_acc]) do - {:ok, _} = ok -> {:cont, ok} - error -> {:halt, error} - end - end - end) - error -> - error - end - end + defp relink_each(binaries, prefix_for, opts) do + set_id? = Keyword.fetch!(opts, :set_id) - defp rewrite(binary, copied, prefix) do - changes = - Enum.map(copied, fn {src, dest} -> - {src, prefix <> "/" <> Path.basename(dest)} - end) - - case change_args(binary, changes) do - [] -> :ok - args -> run("install_name_tool", args) - end - end - - # Copied dylibs share Frameworks/, so they reference each other (and their own - # id) relative to their own location with @loader_path. - defp rewrite_each(copied) do - Enum.reduce_while(copied, :ok, fn {_src, dest}, _acc -> - base = Path.basename(dest) - - with :ok <- run("install_name_tool", id_args(dest, "@loader_path/" <> base)), - :ok <- rewrite(dest, copied, "@loader_path") do + Enum.reduce_while(binaries, :ok, fn binary, :ok -> + with :ok <- maybe_set_id(binary, set_id?), + :ok <- relink(binary, prefix_for.(binary)) do {:cont, :ok} else error -> {:halt, error} @@ -170,6 +219,48 @@ defmodule BDS.MacBundle.Dylibs do end) end + defp maybe_set_id(_binary, false), do: :ok + + defp maybe_set_id(binary, true) do + run("install_name_tool", id_args(binary, "@loader_path/" <> Path.basename(binary))) + end + + defp relink(binary, prefix) do + with {:ok, deps} <- otool(binary), + :ok <- apply_changes(binary, change_args(binary, relink_changes(deps, prefix))) do + strip_external_rpaths(binary) + end + end + + defp apply_changes(_binary, []), do: :ok + defp apply_changes(_binary, args), do: run("install_name_tool", args) + + # Drop any LC_RPATH pointing at Homebrew/local. Such an rpath lets an + # @rpath/ dep resolve outside the bundle, so leaving it would break standalone. + defp strip_external_rpaths(binary) do + case rpaths(binary) do + {:ok, paths} -> + paths + |> Enum.filter(&external?/1) + |> Enum.reduce_while(:ok, fn rpath, :ok -> + case run("install_name_tool", ["-delete_rpath", rpath, binary]) do + :ok -> {:cont, :ok} + error -> {:halt, error} + end + end) + + error -> + error + end + end + + defp rpaths(path) do + case System.cmd("otool", ["-l", path], stderr_to_stdout: true) do + {out, 0} -> {:ok, parse_rpaths(out)} + {out, status} -> {:error, {:otool_failed, status, out}} + end + end + defp otool(path) do case System.cmd("otool", ["-L", path], stderr_to_stdout: true) do {out, 0} -> {:ok, parse_otool(out)} diff --git a/lib/mix/tasks/bds.bundle.macos.ex b/lib/mix/tasks/bds.bundle.macos.ex index d0d6214..f4a52fa 100644 --- a/lib/mix/tasks/bds.bundle.macos.ex +++ b/lib/mix/tasks/bds.bundle.macos.ex @@ -10,6 +10,16 @@ defmodule Mix.Tasks.Bds.Bundle.Macos do with `--regen-icon`), assembles the bundle, relocates the wxWidgets dylibs so it runs on a clean Mac, ad-hoc codesigns it, and (with `--register`) registers the `bds2://` URL scheme via LaunchServices. + + ## Standalone is a hard requirement + + The produced `.app` MUST be fully self-contained: no Mach-O inside it may + reference a Homebrew/local path (`/opt/homebrew`, `/usr/local`). Every external + dylib reachable from the release's NIFs is copied into `Contents/Frameworks` + and rewritten to `@loader_path`-relative install names. The build runs + `BDS.MacBundle.verify_standalone/1` after relocation and **fails** if any + external reference survives — so a bundle that would break on a Mac without + Homebrew never ships. """ use Mix.Task diff --git a/test/bds/mac_bundle_test.exs b/test/bds/mac_bundle_test.exs index 59f8d4e..41a8526 100644 --- a/test/bds/mac_bundle_test.exs +++ b/test/bds/mac_bundle_test.exs @@ -145,6 +145,58 @@ defmodule BDS.MacBundleTest do end end + describe "Dylibs.relink_changes/2" do + test "rewrites every external dep to /, independent of source spelling" do + deps = [ + # opt/symlink spelling (as a NIF references it) + "/opt/homebrew/opt/wxwidgets@3.2/lib/libwx_osx_cocoau_core-3.2.dylib", + # Cellar/versioned spelling (as inter-lib deps reference the same file) + "/opt/homebrew/Cellar/wxwidgets@3.2/3.2.10/lib/libwx_baseu-3.2.0.4.3.dylib", + "/usr/local/lib/libpcre2-32.0.dylib", + # left untouched: macOS-provided and already-relocatable + "/usr/lib/libSystem.B.dylib", + "@rpath/libalready.dylib" + ] + + assert Dylibs.relink_changes(deps, "@loader_path") == [ + {"/opt/homebrew/opt/wxwidgets@3.2/lib/libwx_osx_cocoau_core-3.2.dylib", + "@loader_path/libwx_osx_cocoau_core-3.2.dylib"}, + {"/opt/homebrew/Cellar/wxwidgets@3.2/3.2.10/lib/libwx_baseu-3.2.0.4.3.dylib", + "@loader_path/libwx_baseu-3.2.0.4.3.dylib"}, + {"/usr/local/lib/libpcre2-32.0.dylib", "@loader_path/libpcre2-32.0.dylib"} + ] + end + + test "honors a NIF-relative loader prefix" do + assert Dylibs.relink_changes(["/opt/homebrew/lib/libfoo.dylib"], "@loader_path/../../Frameworks") == + [{"/opt/homebrew/lib/libfoo.dylib", "@loader_path/../../Frameworks/libfoo.dylib"}] + end + end + + describe "Dylibs.parse_rpaths/1" do + test "extracts LC_RPATH paths from otool -l output, ignoring other load commands" do + output = """ + Load command 12 + cmd LC_LOAD_DYLIB + cmdsize 56 + name /usr/lib/libSystem.B.dylib (offset 24) + Load command 13 + cmd LC_RPATH + cmdsize 48 + path /opt/homebrew/Cellar/jpeg-turbo/3.1.4.1/lib (offset 12) + Load command 14 + cmd LC_RPATH + cmdsize 32 + path @loader_path/../lib (offset 12) + """ + + assert Dylibs.parse_rpaths(output) == [ + "/opt/homebrew/Cellar/jpeg-turbo/3.1.4.1/lib", + "@loader_path/../lib" + ] + end + end + describe "Dylibs install_name_tool arg builders" do test "id_args/2" do assert Dylibs.id_args("/Frameworks/libfoo.dylib", "@rpath/libfoo.dylib") == @@ -174,6 +226,53 @@ defmodule BDS.MacBundleTest do end end + describe "Dylibs.bundle/3 + MacBundle.verify_standalone/1 (real otool/install_name_tool)" do + @describetag :macos_tools + + test "relocates the wx NIF's dylibs and leaves zero Homebrew/local references" do + src_nif = Path.join(to_string(:code.priv_dir(:wx)), "wxe_driver.so") + + # Only meaningful when the host's wx NIF links Homebrew dylibs (the normal + # macOS+Homebrew dev setup). Skip otherwise so a non-Homebrew host passes. + {out, 0} = System.cmd("otool", ["-L", src_nif]) + external = out |> Dylibs.parse_otool() |> Enum.filter(&Dylibs.external?/1) + + if external == [] do + :ok + else + tmp = Path.join(System.tmp_dir!(), "bds-relink-#{System.unique_integer([:positive])}") + on_exit(fn -> File.rm_rf!(tmp) end) + + nif_dir = Path.join(tmp, "rel/lib/wx-2.4/priv") + File.mkdir_p!(nif_dir) + File.cp!(src_nif, Path.join(nif_dir, "wxe_driver.so")) + + frameworks = Path.join(tmp, "Frameworks") + nifs = MacBundle.macho_files(Path.join(tmp, "rel")) + + prefix_for = fn nif -> + "@loader_path/" <> MacBundle.relative_up(Path.dirname(nif), frameworks) + end + + assert {:ok, reals} = Dylibs.bundle(nifs, frameworks, prefix_for) + assert reals != [] + + # Hard requirement: nothing in the tree still points at Homebrew/local. + assert MacBundle.verify_standalone(tmp) == :ok + + # Same physical lib referenced under two names (e.g. core-3.2.dylib and + # core-3.2.0.4.3.dylib) collapses to one real file + a symlink, so dyld + # loads a single image — no duplicate Objective-C classes. + symlinks = + frameworks + |> File.ls!() + |> Enum.filter(&(File.lstat!(Path.join(frameworks, &1)).type == :symlink)) + + assert symlinks != [], "expected symlink dedup for versioned dylib names" + end + end + end + describe "MacBundle.macho_files/1" do test "finds Mach-O binaries anywhere in the tree by magic number, ignoring other files" do tmp = Path.join(System.tmp_dir!(), "bds-macho-#{System.unique_integer([:positive])}")