diff --git a/CHANGELOG.md b/CHANGELOG.md index 6da5b5f..f83169d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,32 @@ +## [Unreleased] + +### Fixed + +- **Two plugins declaring the same Info.plist key no longer block the build + when the project's own `ios/Info.plist` sets that key (MOB-387).** Plugin + plist keys only fill gaps (PlistBuddy `Add` never overrides), so the host's + value always wins and the collision is moot. Activating `mob_bluetooth` and + `mob_midi` together (both declare `NSBluetoothAlwaysUsageDescription`) + raised "activated plugins conflict"; now `Validator.cross_validate/2` takes + `:host_plist_keys` and the native build passes the top-level keys of + `ios/Info.plist` (`Validator.host_plist_keys/1`, parsed with xmerl; a binary + plist via `plutil` where it exists). Keys the host doesn't set, and keys any + plugin declares as an array (merged into the host's array rather than + yielding to it), still conflict. +- **A static Rust NIF a dependency ships is built (MOB-392).** A + `:static_nifs` entry whose crate lives in a dependency at + `native//Cargo.toml` (e.g. `mob_rapier`'s `lab_physics`) was + classified as Elixir-only and never cross-compiled, so the app crashed at + launch with `cannot locate symbol lab_physics_nif_init`. When the project + ships no C, Rust or Zig source for the NIF, every dependency + `Mix.Project.deps_paths/0` resolves (hex, git, path) is searched (not for + plugin-contributed NIFs, which the plugin build compiles); two dependencies + shipping the same crate name is a build error. Every Rust NIF now builds + into `_build//mob_rust_nifs/` (`cargo --target-dir`) instead of + the crate's `target/`, so a workspace member or `CARGO_TARGET_DIR` no longer + misplaces the archive (existing `native//target/` dirs can be deleted). + `mix mob.doctor` checks the Rust Android targets for such crates too. + ## [0.7.13] - 2026-10-03 ### Added diff --git a/lib/mix/tasks/mob.doctor.ex b/lib/mix/tasks/mob.doctor.ex index a39a74d..643b774 100644 --- a/lib/mix/tasks/mob.doctor.ex +++ b/lib/mix/tasks/mob.doctor.ex @@ -425,7 +425,8 @@ defmodule Mix.Tasks.Mob.Doctor do ] ++ maybe_check_rust_android_targets() end - # If the project has any Rust NIFs (native/*/Cargo.toml), make sure both + # If the project builds any Rust NIFs (its own native/*/Cargo.toml, or a + # crate a dependency ships for a :static_nifs entry), make sure both # Android rustup targets are installed. Without these, `cargo build # --target=aarch64-linux-android` (or `armv7-linux-androideabi`) fails # with "error: toolchain '' is not installed". @@ -437,7 +438,19 @@ defmodule Mix.Tasks.Mob.Doctor do end end - defp has_rust_nif?, do: Path.wildcard("native/*/Cargo.toml") != [] + defp has_rust_nif?, do: Path.wildcard("native/*/Cargo.toml") != [] or dep_rust_nif?() + + defp dep_rust_nif? do + dep_dirs_for = MobDev.NativeBuild.nif_dep_dirs() + root = File.cwd!() + + Enum.any?(MobDev.NativeBuild.project_nif_user_entries(), fn entry -> + match?( + {:rust, _}, + MobDev.NativeBuild.classify_project_nif(entry, root, dep_dirs_for.(entry)) + ) + end) + end defp check_rust_android_targets do case System.cmd("rustup", ["target", "list", "--installed"], stderr_to_stdout: true) do diff --git a/lib/mob_dev/ios_layout_plist.ex b/lib/mob_dev/ios_layout_plist.ex index a028918..5a4443f 100644 --- a/lib/mob_dev/ios_layout_plist.ex +++ b/lib/mob_dev/ios_layout_plist.ex @@ -237,6 +237,15 @@ defmodule MobDev.IosLayoutPlist do end end + @doc """ + The keys of an XML property list's top-level ``, parsed structurally + (comments, CDATA and nested dictionaries contribute nothing). + """ + @spec top_level_keys(String.t()) :: {:ok, [String.t()]} | {:error, String.t()} + def top_level_keys(xml) do + with {:ok, dict} <- top_level_dict(xml), do: {:ok, Map.keys(dict)} + end + defp top_level_dict(xml) do {doc, _rest} = :xmerl_scan.string(:binary.bin_to_list(xml), diff --git a/lib/mob_dev/native_build.ex b/lib/mob_dev/native_build.ex index f440735..7170d39 100644 --- a/lib/mob_dev/native_build.ex +++ b/lib/mob_dev/native_build.ex @@ -3759,18 +3759,81 @@ defmodule MobDev.NativeBuild do def classify_project_nif(entry), do: classify_project_nif(entry, File.cwd!()) @doc false - @spec classify_project_nif(MobDev.StaticNifs.nif_entry(), Path.t()) :: + # `dep_dirs` are the dependency directories searched, last, for a crate the + # project ships no source for; they default to `nif_dep_dirs/0`'s answer for + # the entry (every dependency `Mix.Project.deps_paths/0` resolves, or none + # for a plugin-contributed NIF). Callers classifying many entries should + # compute `nif_dep_dirs/0` once and pass its answer. + @spec classify_project_nif(MobDev.StaticNifs.nif_entry(), Path.t(), [Path.t()] | nil) :: {:c, Path.t()} | {:rust, Path.t()} | {:zig, atom()} | :elixir_only - def classify_project_nif(entry, project_root) do + def classify_project_nif(entry, project_root, dep_dirs \\ nil) do name = to_string(entry.module) c_src = Path.join(project_root, "c_src/#{name}.c") rust_manifest = Path.join(project_root, "native/#{name}/Cargo.toml") cond do # C wins if both exist — the user has explicitly written C. - File.exists?(c_src) -> {:c, c_src} - File.exists?(rust_manifest) -> {:rust, rust_manifest} - true -> classify_via_zig_stub(name, project_root) + File.exists?(c_src) -> + {:c, c_src} + + File.exists?(rust_manifest) -> + {:rust, rust_manifest} + + true -> + with :elixir_only <- classify_via_zig_stub(name, project_root) do + case dep_rust_manifest(name, dep_dirs || nif_dep_dirs().(entry)) do + nil -> :elixir_only + manifest -> {:rust, manifest} + end + end + end + end + + @doc false + # The dep dirs to search per static-NIF entry. Builds call it once per + # target, not per entry: activating plugins verifies every manifest. A + # plugin's NIF belongs to the plugin's own build; searching deps for it + # could link a second _nif_init, so it gets none. + @spec nif_dep_dirs() :: (MobDev.StaticNifs.nif_entry() -> [Path.t()]) + def nif_dep_dirs do + nif_dep_dirs( + MobDev.Plugin.Merge.nifs(MobDev.Plugin.activated()), + Map.values(Mix.Project.deps_paths()) + ) + end + + @doc false + @spec nif_dep_dirs([map()], [Path.t()]) :: (MobDev.StaticNifs.nif_entry() -> [Path.t()]) + def nif_dep_dirs(plugin_nifs, dep_dirs) do + plugin_modules = MapSet.new(plugin_nifs, & &1.module) + fn entry -> if MapSet.member?(plugin_modules, entry.module), do: [], else: dep_dirs end + end + + # A Rustler NIF a dependency ships (e.g. mob_rapier's + # deps/mob_rapier/native/lab_physics): the host registers it in + # :static_nifs and the crate is cross-compiled from the dep, so the host + # needn't copy or symlink it into its own native/. :static_nifs names only + # the module, so two deps shipping the same crate name is ambiguous. + defp dep_rust_manifest(name, dep_dirs) do + candidates = + for dir <- dep_dirs, + manifest = Path.join(dir, "native/#{name}/Cargo.toml"), + File.exists?(manifest), + do: manifest + + case Enum.sort(candidates) do + [] -> + nil + + [manifest] -> + manifest + + many -> + Mix.raise( + "static NIF #{name}: more than one dependency ships native/#{name}/Cargo.toml:\n" <> + Enum.map_join(many, "\n", &" - #{&1}") <> + "\nRemove one of those dependencies or rename one crate." + ) end end @@ -3871,9 +3934,11 @@ defmodule MobDev.NativeBuild do project_nif_user_entries() |> Enum.filter(&MobDev.StaticNifs.on_platform?(&1, target_arch)) + dep_dirs_for = nif_dep_dirs() + {c_sources, rust_manifests, zig_modules} = Enum.reduce(entries, {[], [], []}, fn entry, {c_acc, rust_acc, zig_acc} -> - case classify_project_nif(entry, project_root) do + case classify_project_nif(entry, project_root, dep_dirs_for.(entry)) do {:c, path} -> {[{to_string(entry.module), path} | c_acc], rust_acc, zig_acc} @@ -3938,12 +4003,14 @@ defmodule MobDev.NativeBuild do "--crate-type", "staticlib", "--manifest-path", - manifest + manifest, + "--target-dir", + rust_nif_target_dir(name) ] case System.cmd("cargo", args, stderr_to_stdout: true, into: IO.stream()) do {_, 0} -> - a = Path.expand("native/#{name}/target/#{target}/release/lib#{name}.a") + a = rust_nif_archive(rust_nif_target_dir(name), name, target) if File.exists?(a) do {:ok, a} @@ -3963,6 +4030,20 @@ defmodule MobDev.NativeBuild do end end + # Cargo builds every Rust NIF into the host's own build dir, not next to the + # crate (which for a dependency is inside deps/): a crate that is a + # workspace member or a set CARGO_TARGET_DIR would otherwise put the archive + # somewhere this build doesn't look. + defp rust_nif_target_dir(name), + do: Path.expand(Path.join(Mix.Project.build_path(), "mob_rust_nifs/#{name}")) + + @doc false + # The static archive `cargo rustc --target-dir ` writes. + @spec rust_nif_archive(Path.t(), String.t(), String.t()) :: Path.t() + def rust_nif_archive(target_dir, name, target) do + Path.join(target_dir, "#{target}/release/lib#{name}.a") + end + # Apple toolchains differ between simulator and device targets. Both # are arm64 on Apple Silicon Macs; sim has the `-sim` suffix because # the SDK headers differ. Android splits per-ABI: arm64-v8a uses diff --git a/lib/mob_dev/plugin/validator.ex b/lib/mob_dev/plugin/validator.ex index 2d2a9e4..8e730e7 100644 --- a/lib/mob_dev/plugin/validator.ex +++ b/lib/mob_dev/plugin/validator.ex @@ -202,28 +202,59 @@ defmodule MobDev.Plugin.Validator do ignored). The first element identifies the plugin in error messages: a plugin directory (as `MobDev.Plugin.activated/0` returns) is reported as its `priv/mob_plugin.exs` path, anything else via `inspect/1`. + + Options: + * `:host_plist_keys` — the keys the host's own `ios/Info.plist` sets. For a + scalar value the host's wins over every plugin's (plugin keys only fill + gaps), so two plugins declaring one of these keys is not a conflict: + neither value lands. A key any plugin declares as an array still + conflicts: arrays merge into the host's array instead of yielding to it. """ - @spec cross_validate([{atom() | Path.t(), map() | nil}]) :: result() - def cross_validate(plugins) do + @spec cross_validate([{atom() | Path.t(), map() | nil}], keyword()) :: result() + def cross_validate(plugins, opts \\ []) do owned = for {owner, m} <- plugins, is_map(m), do: {manifest_location(owner), m} + exempt = exempt_plist_keys(owned, Keyword.get(opts, :host_plist_keys, [])) errors = - for {_gatherer, {:collision, checks}} <- conflict_surface(), + for {gatherer, {:collision, checks}} <- conflict_surface(), {label, extractor} <- checks, + extractor = exempting(gatherer, extractor, exempt), error <- collisions(owned, extractor, label), do: error %{errors: errors, warnings: []} end + # Host keys every plugin declares as a scalar. One array declaration anywhere + # keeps the key a collision: arrays merge into the host's array, so a later + # plugin's value (array or scalar) would silently drop an earlier one's array. + defp exempt_plist_keys(owned, host_keys) do + array_keys = + for {_, m} <- owned, + {key, value} <- get_in(m, [:ios, :plist_keys]) || %{}, + is_list(value), + into: MapSet.new(), + do: to_string(key) + + host_keys |> MapSet.new(&to_string/1) |> MapSet.difference(array_keys) + end + + defp exempting(:plist_keys, extractor, exempt) do + fn manifest -> Enum.reject(extractor.(manifest), &(to_string(&1) in exempt)) end + end + + defp exempting(_gatherer, extractor, _host_keys), do: extractor + @doc """ - Runs `cross_validate/1` over the activated `{plugin_dir, manifest}` pairs and + Runs `cross_validate/2` over the activated `{plugin_dir, manifest}` pairs and `Mix.raise/1`s on any collision, so a native build stops before codegen instead of letting one plugin's registration silently overwrite another's. + Info.plist keys the project's own `ios/Info.plist` already sets are exempt + (the project's value wins; see `cross_validate/2`). """ @spec raise_on_cross_plugin_conflicts!([{Path.t(), map() | nil}]) :: :ok def raise_on_cross_plugin_conflicts!(plugins) do - case cross_validate(plugins) do + case cross_validate(plugins, host_plist_keys: host_plist_keys("ios/Info.plist")) do %{errors: []} -> :ok @@ -235,6 +266,34 @@ defmodule MobDev.Plugin.Validator do end end + @doc """ + The top-level keys an Info.plist sets (`[]` when the file is absent or + unparseable, which keeps every collision an error). The XML is parsed + structurally (`MobDev.IosLayoutPlist.top_level_keys/1`); a binary plist is + converted with `plutil` first when it is available. + """ + @spec host_plist_keys(Path.t()) :: [String.t()] + def host_plist_keys(path) do + with {:ok, raw} <- File.read(path), + {:ok, xml} <- plist_xml(raw, path), + {:ok, keys} <- MobDev.IosLayoutPlist.top_level_keys(xml) do + keys + else + _ -> [] + end + end + + defp plist_xml("bplist" <> _, path) do + with exe when is_binary(exe) <- System.find_executable("plutil"), + {xml, 0} <- System.cmd(exe, ["-convert", "xml1", "-o", "-", path]) do + {:ok, xml} + else + _ -> :error + end + end + + defp plist_xml(xml, _path), do: {:ok, xml} + defp manifest_location(dir) when is_binary(dir), do: Path.join([dir, "priv", "mob_plugin.exs"]) defp manifest_location(other), do: inspect(other) diff --git a/test/mob_dev/native_build_test.exs b/test/mob_dev/native_build_test.exs index c143e2d..f837a4b 100644 --- a/test/mob_dev/native_build_test.exs +++ b/test/mob_dev/native_build_test.exs @@ -1540,7 +1540,7 @@ defmodule MobDev.NativeBuildTest do end end - describe "classify_project_nif/2" do + describe "classify_project_nif/2,3" do # Pins the source-classification logic that decides whether a # project-side NIF gets the C wiring path, the Rust cross-compile + # link path, or no native wiring at all (Elixir-only stub). Issue #18. @@ -1572,6 +1572,101 @@ defmodule MobDev.NativeBuildTest do assert {:rust, ^cargo_path} = NativeBuild.classify_project_nif(%{module: :foo}, tmp) end + test "finds a Rust crate one of the given dependencies ships", %{tmp: tmp} do + # A path dep (or a custom MIX_DEPS_PATH) is not under /deps. + dep = Path.join(tmp, "elsewhere/some_dep") + cargo_path = Path.join(dep, "native/foo/Cargo.toml") + File.mkdir_p!(Path.dirname(cargo_path)) + File.write!(cargo_path, "") + other_dep = Path.join(tmp, "elsewhere/other_dep") + File.mkdir_p!(other_dep) + + assert {:rust, ^cargo_path} = + NativeBuild.classify_project_nif(%{module: :foo}, tmp, [other_dep, dep]) + + assert NativeBuild.rust_nif_archive("/b/mob_rust_nifs/foo", "foo", "aarch64-apple-ios") == + "/b/mob_rust_nifs/foo/aarch64-apple-ios/release/libfoo.a" + + # The project's own crate wins over a dep's. + own = Path.join(tmp, "native/foo/Cargo.toml") + File.mkdir_p!(Path.dirname(own)) + File.write!(own, "") + assert {:rust, ^own} = NativeBuild.classify_project_nif(%{module: :foo}, tmp, [dep]) + end + + test "two dependencies shipping the same crate name is an error", %{tmp: tmp} do + deps = + for d <- ~w(a b) do + dir = Path.join(tmp, "deps/#{d}") + File.mkdir_p!(Path.join(dir, "native/foo")) + File.write!(Path.join(dir, "native/foo/Cargo.toml"), "") + dir + end + + assert_raise Mix.Error, ~r/more than one dependency ships native\/foo/, fn -> + NativeBuild.classify_project_nif(%{module: :foo}, tmp, deps) + end + end + + test "the 2-arity form searches the project's resolved path deps", %{tmp: tmp} do + # A path dep lives outside /deps; only Mix.Project.deps_paths/0 + # knows where it is. + host = Path.join(tmp, "host") + dep = Path.join(tmp, "elsewhere/rdep") + cargo_path = Path.join(dep, "native/foo/Cargo.toml") + File.mkdir_p!(Path.dirname(cargo_path)) + File.write!(cargo_path, "") + suffix = System.unique_integer([:positive]) + + File.write!(Path.join(dep, "mix.exs"), """ + defmodule RDep#{suffix}.MixProject do + use Mix.Project + def project, do: [app: :rdep, version: "0.1.0"] + end + """) + + File.mkdir_p!(host) + + File.write!(Path.join(host, "mix.exs"), """ + defmodule Host#{suffix}.MixProject do + use Mix.Project + def project, do: [app: :host, version: "0.1.0", deps: [{:rdep, path: "../elsewhere/rdep"}]] + end + """) + + # on_clean_slate makes the fixture the top-level project; nested under + # mob_dev's own project, Mix resolves its deps from mob_dev's cache. + result = + Mix.ProjectStack.on_clean_slate(fn -> + Mix.Project.in_project(:"host#{suffix}", host, fn _ -> + NativeBuild.classify_project_nif(%{module: :foo}, host) + end) + end) + + assert {:rust, found} = result + # deps_paths resolves symlinks (macOS /var → /private/var). + assert String.ends_with?(found, Path.relative_to(cargo_path, Path.dirname(tmp))) + end + + test "nif_dep_dirs/2 searches no deps for a plugin's own NIF" do + dirs_for = NativeBuild.nif_dep_dirs([%{module: :plugin_nif}], ["/deps/a"]) + assert dirs_for.(%{module: :plugin_nif}) == [] + assert dirs_for.(%{module: :host_nif}) == ["/deps/a"] + end + + test "the project's own Zig NIF wins over a dependency's crate", %{tmp: tmp} do + dep = Path.join(tmp, "deps/d") + File.mkdir_p!(Path.join(dep, "native/foo")) + File.write!(Path.join(dep, "native/foo/Cargo.toml"), "") + Application.put_env(:mob_dev, :__app_name__, :zapp) + on_exit(fn -> Application.delete_env(:mob_dev, :__app_name__) end) + stub = Path.join(tmp, "lib/zapp/nifs/foo.ex") + File.mkdir_p!(Path.dirname(stub)) + File.write!(stub, "defmodule Foo do\n use Zig, otp_app: :zapp\nend\n") + + assert {:zig, _} = NativeBuild.classify_project_nif(%{module: :foo}, tmp, [dep]) + end + test "C wins if both exist (user has explicitly written C)", %{tmp: tmp} do File.mkdir_p!(Path.join(tmp, "c_src")) File.mkdir_p!(Path.join(tmp, "native/foo")) diff --git a/test/mob_dev/plugin/conflict_surface_test.exs b/test/mob_dev/plugin/conflict_surface_test.exs index 65a023f..e6e46ce 100644 --- a/test/mob_dev/plugin/conflict_surface_test.exs +++ b/test/mob_dev/plugin/conflict_surface_test.exs @@ -99,6 +99,76 @@ defmodule MobDev.Plugin.ConflictSurfaceTest do assert Enum.any?(errs, &(&1 =~ "Info.plist key")) end + test "a duplicate Info.plist key the host's own Info.plist sets is no conflict" do + plugins = + two( + %{ + ios: %{ + plist_keys: %{NSBluetoothAlwaysUsageDescription: "A", NSCameraUsageDescription: "A"} + } + }, + %{ + ios: %{ + plist_keys: %{NSBluetoothAlwaysUsageDescription: "B", NSCameraUsageDescription: "B"} + } + } + ) + + assert %{errors: [err]} = + Validator.cross_validate(plugins, + host_plist_keys: ["NSBluetoothAlwaysUsageDescription"] + ) + + assert err =~ "NSCameraUsageDescription" + end + + test "a host-set Info.plist key any plugin declares as an array still conflicts" do + # Plugin arrays are merged into the host's array, and between plugins the + # later value wins, so the other plugin's array would be silently lost. + array = %{ios: %{plist_keys: %{"UIBackgroundModes" => ["bluetooth-central"]}}} + + for other <- [["location"], "scalar"], {a, b} <- [{:array, :other}, {:other, :array}] do + pick = %{array: array, other: %{ios: %{plist_keys: %{"UIBackgroundModes" => other}}}} + + assert %{errors: [err]} = + Validator.cross_validate(two(pick[a], pick[b]), + host_plist_keys: ["UIBackgroundModes"] + ) + + assert err =~ "UIBackgroundModes" + end + end + + @tag :tmp_dir + test "host_plist_keys/1 lists the root dictionary's keys only", %{tmp_dir: dir} do + path = Path.join(dir, "Info.plist") + + File.write!(path, """ + + + + NSCameraUsageDescription + camera + UIApplicationSceneManifest + + UISceneConfigurations + Innerx + + + Real + Fake text]]> + NFCReaderUsageDescription + nfc + + + """) + + assert Enum.sort(Validator.host_plist_keys(path)) == + ~w(NFCReaderUsageDescription NSCameraUsageDescription Real UIApplicationSceneManifest) + + assert Validator.host_plist_keys(Path.join(dir, "missing.plist")) == [] + end + test "duplicate AndroidManifest component name across plugins" do plugins = same(%{ diff --git a/test/mob_dev/plugin/host_plist_conflicts_test.exs b/test/mob_dev/plugin/host_plist_conflicts_test.exs new file mode 100644 index 0000000..3850bb1 --- /dev/null +++ b/test/mob_dev/plugin/host_plist_conflicts_test.exs @@ -0,0 +1,56 @@ +defmodule MobDev.Plugin.HostPlistConflictsTest do + # Changes the cwd, so not async. + use ExUnit.Case, async: false + + alias MobDev.Plugin.Validator + + @base %{name: :p, mob_version: "~> 0.6", plugin_spec_version: 1} + + @plugins [ + {:a, Map.put(@base, :ios, %{plist_keys: %{"NSBluetoothAlwaysUsageDescription" => "A"}})}, + {:b, + Map.put(%{@base | name: :b}, :ios, %{ + plist_keys: %{"NSBluetoothAlwaysUsageDescription" => "B"} + })} + ] + + @host_plist """ + + + + + NSBluetoothAlwaysUsageDescription + host + + + """ + + @tag :tmp_dir + test "the native build's gate reads the project's ios/Info.plist", %{tmp_dir: dir} do + File.cd!(dir, fn -> + assert_raise Mix.Error, ~r/NSBluetoothAlwaysUsageDescription/, fn -> + Validator.raise_on_cross_plugin_conflicts!(@plugins) + end + + File.mkdir_p!("ios") + File.write!("ios/Info.plist", @host_plist) + assert Validator.raise_on_cross_plugin_conflicts!(@plugins) == :ok + end) + end + + @tag :tmp_dir + test "host_plist_keys/1 reads a binary plist via plutil", %{tmp_dir: dir} do + plutil = System.find_executable("plutil") + path = Path.join(dir, "Info.plist") + File.write!(path, @host_plist) + + if plutil do + {_, 0} = System.cmd(plutil, ["-convert", "binary1", path]) + assert File.read!(path) =~ ~r/\Abplist/ + assert Validator.host_plist_keys(path) == ["NSBluetoothAlwaysUsageDescription"] + else + File.write!(path, "bplist00garbage") + assert Validator.host_plist_keys(path) == [] + end + end +end