From da4d1f697e7fd91b86e0a21999e3476d4ec3fe9a Mon Sep 17 00:00:00 2001 From: GenericJam Date: Fri, 11 Sep 2026 17:53:07 -0600 Subject: [PATCH 1/2] MOB-169: mob.deploy freezes explicit targets before doing work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discovery used to fan out to every reachable device during compatibility checks, native installs, and the final BEAM push independently. A bare `mix mob.deploy --native` installed on every Android device from `adb devices` and tried every connected iPhone, ignoring ANDROID_SERIAL. The set could change during a long native build, so even reading the initial output did not define which devices the command would later modify — a phone plugged in for unrelated work would receive the deploy. The task now discovers once and resolves an immutable target list before compile / build / install / push begins. Every downstream stage consumes that list. See `decisions/2026-09-11-deploy-freezes-explicit-targets.md`. New shared `MobDev.TaskTargets` policy: - `--device ` selects one named device, including a physical device. - `--all-devices` selects every emulator and simulator. - `--all-physical` selects every physical device. - Combined broad flags select every connected device. - No flag = exactly one emulator or simulator, auto-selected. Physical devices are never implicit; multiple development devices are an error demanding an explicit selection. - On Android-only runs, a non-empty `ANDROID_SERIAL` is a named target when no CLI scope was supplied. Explicit CLI wins. `MobDev.Uninstaller` delegates through the same selector so device- changing tasks cannot drift. ## Reviews before commit Adopted this branch from a prior agent's session (they left the work uncommitted, no PR, ticket sitting In Progress with no comment). Rebased onto current master (which now carries the 0.7.1 release), verified the design against the decision record, ran gates: - 2447 mix tests pass (2 new, added while addressing review findings). Every changed test flips when the corresponding fix is reverted. - `mix format --check-formatted`, `mix credo --strict`, `mix compile --warnings-as-errors --force`, `mix mob.security_scan` all clean. Adversarial subagent review of the ~1000-line diff (verdict SOUND WITH FIXES) caught: - **F1 blocker**: `ANDROID_SERIAL` hijacked mixed-platform default runs. On macOS `mix mob.deploy` runs on [:android, :ios], and a stale ANDROID_SERIAL from another tool would turn a bare deploy into `--device xyz` and raise :no_matching_devices against any connected iOS simulator. Fixed by narrowing the ANDROID_SERIAL rule to Android-only runs (matches the decision record's exact wording), plus a `String.trim/1` so a whitespace-only value is treated as unset. Two revert-verified tests added. - **F2 should-fix**: CHANGELOG did not name the multi-emulator break as a behaviour change. Added a paragraph. - **Also caught by me**: the prior agent had added the MOB-169 CHANGELOG entry inside the [0.7.0] block, which shipped to Hex a week ago without any of this work. Moved to a fresh [Unreleased] block at the top. Not device-verified (mob_dev is host-side tooling, no device to verify against). Ready for release when a next mob_dev version warrants one. Co-Authored-By: Claude Opus 4.7 --- AGENTS.md | 15 +- CHANGELOG.md | 28 ++ README.md | 21 +- ...6-09-11-deploy-freezes-explicit-targets.md | 55 +++ lib/mix/tasks/mob.deploy.ex | 240 +++++++++---- lib/mob_dev/deployer.ex | 57 +++- lib/mob_dev/native_build.ex | 317 +++++++++++------- lib/mob_dev/task_targets.ex | 114 +++++++ lib/mob_dev/uninstaller.ex | 96 +----- test/mix/tasks/mob_deploy_beam_flags_test.exs | 11 + test/mix/tasks/mob_deploy_parsing_test.exs | 101 ++++++ test/mob_dev/deployer_test.exs | 37 +- test/mob_dev/native_build_test.exs | 29 ++ test/mob_dev/task_targets_test.exs | 46 +++ test/mob_dev/wiring_test.exs | 12 +- 15 files changed, 857 insertions(+), 322 deletions(-) create mode 100644 decisions/2026-09-11-deploy-freezes-explicit-targets.md create mode 100644 lib/mob_dev/task_targets.ex create mode 100644 test/mob_dev/task_targets_test.exs diff --git a/AGENTS.md b/AGENTS.md index 8358cef..3a2d0a9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -219,7 +219,7 @@ you'll lose the ability to evolve the parsers safely. ## Destructive-task conventions Apply consistently to every Mix task that mutates device state -(`mix mob.uninstall` today; `mix mob.deploy --all-devices`, +(`mix mob.uninstall`, `mix mob.deploy`, `mix mob.connect`, future ones). **Emulator vs physical safety pattern (from `mix mob.uninstall`):** @@ -237,19 +237,12 @@ Apply consistently to every Mix task that mutates device state flags → error with a hint pointing at `--all-physical` or `--device`. -The predicate to route on is `MobDev.Device.physical?/1`. The -selection logic lives in `MobDev.Uninstaller.select_devices/3` -(public for testing); same shape should appear in any new task -needing the same fan-out behavior. Pin the headline guarantee in +The predicate to route on is `MobDev.Device.physical?/1`. Shared +selection logic lives in `MobDev.TaskTargets`; task-specific planning +and error messages wrap it. Pin the headline guarantee in each task's tests — "personal iPhone + dev emulators + `--all-devices` must leave the iPhone alone." -**TODO:** apply this pattern to `mix mob.deploy` (today's `--all-devices` -deploy can push BEAMs to a personal phone). When that fan-out exists -or grows, factor `select_devices/3` plus the flag plumbing into a -shared `MobDev.TaskTargets` (or similar) module so the rules don't -drift between tasks. - ## Naming gotcha: `mix mob.install` vs `mix mob.uninstall` These look like inverses but aren't. Future agents touching either diff --git a/CHANGELOG.md b/CHANGELOG.md index ee8a709..3d7f288 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,31 @@ +## [Unreleased] + +### Changed + +- **`mix mob.deploy` now freezes an explicit target set before doing work** + (MOB-169). A bare deploy automatically targets exactly one emulator or + simulator and never a physical phone. `--device`, `--all-devices`, and + `--all-physical` provide explicit single, development-device, and physical + scopes; `ANDROID_SERIAL` acts as the Android single-device selector on + Android-only runs. + + Compatibility checks, native installs, and the final BEAM push all consume + the same snapshot, so a device appearing during the build cannot join the + operation. App-not-installed skips on physical iOS also no longer inherit + the incomplete-override warning used for real mid-copy failures. + + **Behaviour change** — a bare `mix mob.deploy` with **two or more emulators + or simulators running for parallel testing** used to fan out to both, and + now refuses with an ambiguity error demanding an explicit selection + (`--device ` or `--all-devices`). This is the loudest new failure mode: + scripts and shell aliases that relied on the fan-out will need one of the + new flags. Physical devices also no longer receive an implicit deploy — + name one with `--device` or pass `--all-physical` to include them. + + See `decisions/2026-09-11-deploy-freezes-explicit-targets.md`. + +--- + ## [0.7.1] - 2026-09-11 ### Added diff --git a/README.md b/README.md index 6424b2a..a189689 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ end | `mix mob.new APP_NAME` | Generate a new Mob project (see `mob_new` archive) | | `mix mob.adopt` | Install Mob into an **existing** Phoenix project (Igniter-based; composes `mob.adopt.{deps,bridge,screen,mob_app,mob_exs,native,finalize}`). The install-into-existing counterpart to `mix mob.new` | | `mix mob.install` | First-run setup: download OTP runtime, generate icons, write `mob.exs` | -| `mix mob.deploy` | Compile and push BEAMs to all connected devices | +| `mix mob.deploy` | Compile and push BEAMs to one selected emulator/simulator | | `mix mob.deploy --native` | Also build and install the native APK/iOS app | | `mix mob.deploy --slim` | Same, but with the App Store strip pass applied (slow, lets you verify a slim build before TestFlight — see [`guides/slim_release.md`](guides/slim_release.md)) | | `mix mob.release` | Build a signed `.ipa` / `.aab` for App Store / TestFlight / Play Store (slim by default) | @@ -83,6 +83,14 @@ Watch events broadcast on `"watch"` PubSub topic: ## Hot-push transport (`mix mob.deploy`) +The task resolves its target set once before compiling. With no target flag it +automatically selects exactly one emulator or simulator and never selects a +physical device. Use `--device ` for one explicit target, +`--all-devices` for every emulator/simulator, or `--all-physical` for attached +phones. Combining the two broad flags selects every connected device. +`ANDROID_SERIAL` has the same single-target effect as `--device` for Android; +an explicit CLI scope takes precedence. + When Erlang distribution is reachable, `mix mob.deploy` hot-pushes changed BEAMs in-place via RPC — no `adb push`, no app restart. The running modules are replaced exactly like `nl/1` in IEx. ``` @@ -134,18 +142,15 @@ unloaded at any moment. `mix mob.deploy` exits non-zero when: - any device **failed**, including a partial success where others deployed; -- every device of a platform you **named** was skipped — `mix mob.deploy --ios` - where every iOS device lacked the app. A skip stays non-fatal when it is - incidental, so a plain `mix mob.deploy` with an unrelated phone attached - still exits 0, and one simulator deploying while a stale one is skipped is a - success; +- every selected device of a platform you **named** was skipped — + `mix mob.deploy --ios --device ` where the target lacked the app; - you named `--device X` and nothing was deployed to it, or no device matched; - `--native` built nothing for a platform you **named** — a missing `sdk.dir` in `android/local.properties` under `--android --native`, say. A plain `mix mob.deploy --native` that skips a platform nobody asked for still exits 0; -- you **named** a platform and no device of it was connected at all, which is - also what `--ios` on Linux does. +- you selected a broad scope and no device matched it. Naming a platform alone + still permits a native artifact-only build with no attached device. A `--native` run that built the artifact and found no device to push it to still exits 0: "build the APK now, attach the phone after" is a legitimate diff --git a/decisions/2026-09-11-deploy-freezes-explicit-targets.md b/decisions/2026-09-11-deploy-freezes-explicit-targets.md new file mode 100644 index 0000000..d133619 --- /dev/null +++ b/decisions/2026-09-11-deploy-freezes-explicit-targets.md @@ -0,0 +1,55 @@ +# mob.deploy freezes explicit targets before doing work + +Date: 2026-09-11 +Status: accepted +Ticket: MOB-169 + +## Context + +`mix mob.deploy --native` discovered devices independently during compatibility +checks, native installation, and the final BEAM push. A bare run therefore +installed on every Android device returned by `adb devices` and tried every +connected iPhone. It also ignored `ANDROID_SERIAL`. The set could change during +a long native build, so even checking the initial output did not define which +devices the command would later modify. + +Physical phones may contain personal data and are often attached for unrelated +work. A command that changes them needs a deliberate target choice. Emulators +and simulators are safer development defaults, but selecting several of them +implicitly is still ambiguous. + +## Decision + +The deploy task discovers once and resolves one immutable list before compile, +build, install, or push begins. Every later stage consumes that list. + +The shared `MobDev.TaskTargets` policy is: + +- `--device ` selects one named device, including a physical device. +- `--all-devices` selects every emulator and simulator. +- `--all-physical` selects every physical device. +- Combining the broad flags selects every connected device. +- With no flag, exactly one emulator or simulator is selected automatically. + Physical devices are never implicit, and multiple development devices are an + error. + +For Android, a non-empty `ANDROID_SERIAL` is a named target when no CLI target +scope was supplied. Explicit CLI selection takes precedence. + +Native artifact-only builds remain valid with no connected device. An empty +frozen list means install and push nothing; it never means rediscover and fan +out. + +## Consequences + +- Users with several emulators or simulators must choose one or pass + `--all-devices`. +- Users deploying to a phone must name it or pass `--all-physical`. +- Android install and OTP delivery intersect later `adb devices` output with + the frozen serials. Newly connected devices cannot join, and a selected + device disappearing stops the native delivery instead of widening scope. +- An iOS simulator build is shared across selected simulators and installed + only on their frozen UDIDs. Physical iOS builds remain per-device because + signing and installation are device-specific. +- `MobDev.Uninstaller` delegates to the same selector so the two device-changing + tasks cannot drift independently. diff --git a/lib/mix/tasks/mob.deploy.ex b/lib/mix/tasks/mob.deploy.ex index 7158f9b..071f377 100644 --- a/lib/mix/tasks/mob.deploy.ex +++ b/lib/mix/tasks/mob.deploy.ex @@ -1,11 +1,11 @@ defmodule Mix.Tasks.Mob.Deploy do use Mix.Task - @shortdoc "Build and deploy to all connected mob devices" + @shortdoc "Build and deploy to explicitly selected mob devices" @moduledoc """ - Compiles the project then pushes BEAM files to all connected - Android devices and iOS simulators. + Compiles the project then pushes BEAM files to a frozen set of selected + Android and iOS devices. ## Modes @@ -19,11 +19,19 @@ defmodule Mix.Tasks.Mob.Deploy do mix mob.deploy --native + Device selection is resolved once before any build or push starts. With no + selection flag, one emulator or simulator is selected automatically; physical + devices always require an explicit `--device` or `--all-physical`. The + `ANDROID_SERIAL` environment variable is treated like `--device` for Android. + ## Options * `--native` — build native binaries before pushing BEAMs * `--no-restart` — push BEAMs but don't restart the app * `-d`, `--device ` — target a specific device; use `mix mob.devices` to find IDs + * `--all-devices` — target all emulators and simulators + * `--all-physical` — target all physical devices; combine with + `--all-devices` to target every connected device * `--dist-port ` — pin the BEAM dist listen port (default: auto-allocated per device, `9100 + index`). Use to resolve EPMD collisions when multiple sims/emulators are running the same app concurrently @@ -126,15 +134,15 @@ defmodule Mix.Tasks.Mob.Deploy do bucket — including a partial success where other devices deployed fine. Devices under `Skipped on N device(s)` (app not installed for that platform) - do not fail the run — *unless you named that platform*. A skip means "this - device is not a target for this app", which is ordinary when it is a phone - that happens to be attached, and a failure when the run asked for it: - - * `mix mob.deploy` with an unrelated phone attached — exit 0. - * `mix mob.deploy --ios` where every iOS device was skipped — exit 1. - * `mix mob.deploy --ios` where one simulator deployed and a stale one was - skipped — exit 0. A partial success is a success; the rule is per - platform, not per device. + do not fail an implicit single-emulator run. They do fail when an explicit + device, broad scope, or platform was requested and nothing reached that + target platform: + + * `mix mob.deploy --ios --device X` where X lacked the app — exit 1. + * `mix mob.deploy --all-devices` where every selected iOS simulator was + skipped — exit 1 for the requested iOS target set. + * `mix mob.deploy --ios --all-devices` where one simulator deployed and a + stale one was skipped — exit 0. The rule remains per platform. * `mix mob.deploy --device X` that reached X and deployed nothing — exit 1. * `mix mob.deploy --device NOPE` matching no device — exit 1. * `mix mob.deploy --android --native` that built the APK with no device @@ -144,7 +152,7 @@ defmodule Mix.Tasks.Mob.Deploy do is what a missing `sdk.dir` in `android/local.properties` produces. """ - alias MobDev.Device + alias MobDev.{Device, TaskTargets} @switches [ native: :boolean, @@ -157,6 +165,8 @@ defmodule Mix.Tasks.Mob.Deploy do android: :boolean, ios: :boolean, device: :string, + all_devices: :boolean, + all_physical: :boolean, schedulers: :integer, beam_flags: :string, # Manual overrides for the BEAM-distribution surface — useful when @@ -231,22 +241,23 @@ defmodule Mix.Tasks.Mob.Deploy do restart = Keyword.get(opts, :restart, true) native = Keyword.get(opts, :native, false) - device_id = opts[:device] platforms = resolve_platforms(opts) - # Narrow once at the task level so build_all and deploy_all both see the - # same platform list. Without this, the deployer iterates over the - # irrelevant platform and `filter_by_device_id` emits a misleading - # "No device matched" warning even when the targeted platform succeeded. - platforms = MobDev.NativeBuild.narrow_platforms_for_device(platforms, device_id) - beam_flags = resolve_beam_flags(opts) + android_serial = System.get_env("ANDROID_SERIAL") + target_reference = explicit_target_reference(platforms, opts, android_serial) + discovered = discover_devices(platforms) + + devices = + case resolve_targets(discovered, platforms, opts, android_serial) do + {:ok, selected} -> selected + {:error, message} -> Mix.raise(message) + end - # When no --device is given and we're doing a native iOS build, auto-detect - # a connected physical device now so both the native build and the BEAM push - # target the same device (not all simulators + the phone). - effective_device_id = - device_id || - if native and :ios in platforms, - do: MobDev.NativeBuild.detect_physical_ios() + # The selected snapshot is the authority for the rest of this run. Narrow + # the build platforms to it when devices exist, then pass the same structs + # through compatibility checks, native installation, and the final push. + platforms = selected_platforms(platforms, devices) + required_platforms = required_platforms(opts, platforms) + beam_flags = resolve_beam_flags(opts) # Validate every targeted device against the project's enabled # features (Pythonx, etc.) BEFORE we waste time on a multi-minute @@ -262,7 +273,7 @@ defmodule Mix.Tasks.Mob.Deploy do # only as good as the data it's based on, and an escape hatch is # how we keep that data honest. if System.get_env("MOB_FORCE_DEPLOY") in [nil, ""] do - validate_device_compatibility!(platforms, effective_device_id) + validate_device_compatibility!(devices) else IO.puts( " #{IO.ANSI.yellow()}MOB_FORCE_DEPLOY set — skipping device compatibility check#{IO.ANSI.reset()}" @@ -291,9 +302,9 @@ defmodule Mix.Tasks.Mob.Deploy do if native do MobDev.NativeBuild.build_all( platforms: platforms, - device: effective_device_id, + devices: devices, slim: slim, - requested: requested_platforms(opts) + requested: required_platforms ) end @@ -314,9 +325,8 @@ defmodule Mix.Tasks.Mob.Deploy do MobDev.Deployer.deploy_all( restart: restart, platforms: platforms, + devices: devices, force_fs: native, - device: device_id, - ios_device: effective_device_id, beam_flags: beam_flags, # nil → auto-allocation (per-device port + auto-derived suffix). # Set → all targeted devices use these values verbatim. @@ -329,12 +339,12 @@ defmodule Mix.Tasks.Mob.Deploy do # The full summary is printed first, then the status code is set — the # fan-out across devices is unchanged, only the exit code is. message = - missing_device_message(device_id, deployed, failed, skipped) || + missing_device_message(target_reference, deployed, failed, skipped) || failure_message( deployed, failed, skipped, - requested_platforms(opts), + required_platforms, native and native_ok == true ) @@ -545,21 +555,29 @@ defmodule Mix.Tasks.Mob.Deploy do Only fires when a device was named: with no `--device`, an empty run is the ordinary "nothing is plugged in" case and stays non-fatal. """ - @spec missing_device_message(String.t() | nil, [Device.t()], [Device.t()], [Device.t()]) :: - String.t() | nil + @spec missing_device_message( + String.t() | {:device | :android_serial, String.t()} | nil, + [Device.t()], + [Device.t()], + [Device.t()] + ) :: String.t() | nil def missing_device_message(nil, _deployed, _failed, _skipped), do: nil - def missing_device_message(device_id, [], [], []), - do: "No device matched --device #{device_id} — nothing was deployed" + def missing_device_message(reference, [], [], []), + do: "No device matched #{target_reference_label(reference)} — nothing was deployed" # Found, but nothing landed on it. Naming a device by id is at least as # explicit as naming a platform, so a run that shipped nowhere must say so. # `failed` is left to `failure_message/5`, which reports the actual error. - def missing_device_message(device_id, [], [], skipped) when skipped != [], - do: "--device #{device_id} was skipped — nothing was deployed to it" + def missing_device_message(reference, [], [], skipped) when skipped != [], + do: "#{target_reference_label(reference)} was skipped — nothing was deployed to it" def missing_device_message(_device_id, _deployed, _failed, _skipped), do: nil + defp target_reference_label({:device, id}), do: "--device #{id}" + defp target_reference_label({:android_serial, id}), do: "ANDROID_SERIAL=#{id}" + defp target_reference_label(id) when is_binary(id), do: "--device #{id}" + @doc """ Build the per-deploy summary lines from the three device buckets. @@ -644,6 +662,117 @@ defmodule Mix.Tasks.Mob.Deploy do Enum.filter([:android, :ios], &(opts[&1] == true)) end + @doc false + @spec required_platforms(keyword(), [:android | :ios]) :: [:android | :ios] + def required_platforms(opts, selected_platforms) do + requested = requested_platforms(opts) + + if requested == [] and (opts[:all_devices] == true or opts[:all_physical] == true), + do: selected_platforms, + else: requested + end + + @doc false + @spec resolve_targets([Device.t()], [:android | :ios], keyword(), String.t() | nil) :: + {:ok, [Device.t()]} | {:error, String.t()} + def resolve_targets(discovered, platforms, opts, android_serial) do + broad? = opts[:all_devices] == true or opts[:all_physical] == true + + ids = + case explicit_target_reference(platforms, opts, android_serial) do + {_source, id} -> [id] + nil -> [] + end + + cond do + discovered == [] and ids != [] -> + {:error, "No connected device matched #{inspect(hd(ids))}. Run `mix mob.devices`."} + + discovered == [] and broad? -> + {:error, "No connected devices matched the requested target scope."} + + discovered == [] -> + {:ok, []} + + true -> + case TaskTargets.resolve(discovered, ids, opts) do + {:ok, selected} -> {:ok, selected} + {:error, reason, context} -> {:error, target_error(reason, context, ids)} + end + end + end + + defp explicit_target_reference(platforms, opts, android_serial) do + broad? = opts[:all_devices] == true or opts[:all_physical] == true + + cond do + is_binary(opts[:device]) -> + {:device, opts[:device]} + + broad? -> + nil + + # ANDROID_SERIAL is honoured only on Android-only runs. The default + # macOS `mix mob.deploy` runs on `[:android, :ios]`, and a leftover + # ANDROID_SERIAL from another tool would otherwise turn every bare + # deploy into an explicit named target — a fresh regression that + # would raise :no_matching_devices for anyone with a valid iOS + # simulator connected. The decision record's exact phrasing — + # "For Android, a non-empty ANDROID_SERIAL is a named target" — + # explicitly scopes to Android; this is that scope. + platforms == [:android] and is_binary(android_serial) and + String.trim(android_serial) != "" -> + {:android_serial, String.trim(android_serial)} + + true -> + nil + end + end + + @doc false + @spec selected_platforms([:android | :ios], [Device.t()]) :: [:android | :ios] + def selected_platforms(platforms, []), do: platforms + + def selected_platforms(platforms, devices) do + Enum.filter(platforms, fn platform -> Enum.any?(devices, &(&1.platform == platform)) end) + end + + @doc false + @spec discover_devices([:android | :ios], keyword()) :: [Device.t()] + def discover_devices(platforms, opts \\ []) do + android_lister = Keyword.get(opts, :android_lister, &MobDev.Discovery.Android.list_devices/0) + ios_lister = Keyword.get(opts, :ios_lister, &MobDev.Discovery.IOS.list_devices/0) + + [] + |> maybe_concat(:android in platforms, android_lister) + |> maybe_concat(:ios in platforms, ios_lister) + |> Enum.reject(&(&1.status == :unauthorized)) + end + + defp target_error(:no_matching_devices, _context, ids), + do: + "No connected device matched #{Enum.map_join(ids, ", ", &inspect/1)}. Run `mix mob.devices`." + + defp target_error(:no_dev_devices, %{hint: hint}, _ids), do: hint + + defp target_error(:no_physical_devices, _context, _ids), + do: "No physical devices are connected. Run `mix mob.devices`." + + defp target_error(:ambiguous_devices, context, _ids) do + case context do + %{non_physical: 0, physical: physical} when physical > 0 -> + "Only physical devices are connected. Use `--device ` or `--all-physical`." + + %{non_physical: count} when count > 1 -> + "#{count} emulators/simulators are connected. Use `--device ` or `--all-devices`." + + _ -> + "Device selection is ambiguous. Run `mix mob.devices` and choose a target." + end + end + + defp target_error(:no_devices, _context, _ids), do: "No connected devices found." + defp resolve_platforms(opts) do android = opts[:android] ios = opts[:ios] @@ -692,15 +821,13 @@ defmodule Mix.Tasks.Mob.Deploy do # We deliberately don't filter — if any one of the targeted devices fails, # we halt and surface every device that fails. Skipping unsupported devices # silently would just regrow the silent-failure problem at a different layer. - defp validate_device_compatibility!(platforms, device_id) do + defp validate_device_compatibility!(devices) do project_dir = File.cwd!() features = MobDev.SupportMatrix.enabled_features(project_dir) if features == [] do :ok else - devices = candidate_devices(platforms, device_id) - issues = devices |> Enum.flat_map(fn device -> @@ -730,33 +857,6 @@ defmodule Mix.Tasks.Mob.Deploy do end end - # Returns the connected devices that mob.deploy would actually target. - # Mirrors what the deployer / build pipeline does internally — narrow by - # platform and (if given) by --device id. - defp candidate_devices(platforms, device_id) do - devices = - [] - |> maybe_concat(:android in platforms, fn -> - try do - MobDev.Discovery.Android.list_devices() - rescue - _ -> [] - end - end) - |> maybe_concat(:ios in platforms, fn -> - try do - MobDev.Discovery.IOS.list_simulators() - rescue - _ -> [] - end - end) - - case device_id do - nil -> devices - id -> Enum.filter(devices, &MobDev.Device.match_id?(&1, id)) - end - end - defp maybe_concat(list, true, fun), do: list ++ fun.() defp maybe_concat(list, false, _fun), do: list diff --git a/lib/mob_dev/deployer.ex b/lib/mob_dev/deployer.ex index 2345d3e..b2368dd 100644 --- a/lib/mob_dev/deployer.ex +++ b/lib/mob_dev/deployer.ex @@ -69,20 +69,7 @@ defmodule MobDev.Deployer do beam_flags = Keyword.get(opts, :beam_flags, nil) beam_dirs = collect_beam_dirs() - android = - if :android in platforms, - do: - Android.list_devices() - |> Enum.reject(&(&1.status == :unauthorized)) - |> filter_by_device_id(device_id), - else: [] - - ios = - if :ios in platforms, - do: IOS.list_devices() |> filter_by_device_id(ios_device_id || device_id), - else: [] - - all = android ++ ios + all = target_devices(opts, platforms, device_id, ios_device_id) if all == [] do IO.puts(" #{color(:yellow)}No devices found.#{color(:reset)}") @@ -162,6 +149,34 @@ defmodule MobDev.Deployer do end end + @doc false + @spec target_devices(keyword(), [atom()], String.t() | nil, String.t() | nil) :: [Device.t()] + def target_devices(opts, platforms, device_id \\ nil, ios_device_id \\ nil) do + if Keyword.has_key?(opts, :devices) do + opts + |> Keyword.fetch!(:devices) + |> Enum.filter(&(&1.platform in platforms)) + else + android_lister = Keyword.get(opts, :android_lister, &Android.list_devices/0) + ios_lister = Keyword.get(opts, :ios_lister, &IOS.list_devices/0) + + android = + if :android in platforms, + do: + android_lister.() + |> Enum.reject(&(&1.status == :unauthorized)) + |> filter_by_device_id(device_id), + else: [] + + ios = + if :ios in platforms, + do: ios_lister.() |> filter_by_device_id(ios_device_id || device_id), + else: [] + + android ++ ios + end + end + @doc """ Bucket a per-device results list into `{deployed, failed, skipped}`. @@ -1236,19 +1251,25 @@ defmodule MobDev.Deployer do Process.delete(:mob_ios_override_replaced) {:ok, device} catch - # Not annotated with override state: the copy never started, so nothing - # on the device was replaced and there is no partial override to warn about. {:skipped, reason} -> - {:skipped, reason} + finalize_ios_override_result({:skipped, reason}, app) {:error, reason} -> - {:error, annotate_override_state(reason, app)} + finalize_ios_override_result({:error, reason}, app) after Process.delete(:mob_ios_override_replaced) File.rm_rf!(staging_parent) end end + @doc false + @spec finalize_ios_override_result({:skipped | :error, String.t()}, String.t()) :: + {:skipped | :error, String.t()} + def finalize_ios_override_result({:skipped, reason}, _app), do: {:skipped, reason} + + def finalize_ios_override_result({:error, reason}, app), + do: {:error, annotate_override_state(reason, app)} + # If the destructive copy had already begun, the device is now in a worse # state than we found it: Documents/otp/ exists but is incomplete, and # mob_beam.m selects that override on directory existence alone. The app will diff --git a/lib/mob_dev/native_build.ex b/lib/mob_dev/native_build.ex index 272f047..4494a45 100644 --- a/lib/mob_dev/native_build.ex +++ b/lib/mob_dev/native_build.ex @@ -1,6 +1,5 @@ defmodule MobDev.NativeBuild do - alias MobDev.Release - alias MobDev.Toolchain + alias MobDev.{Device, Release, Toolchain} @moduledoc """ Builds native binaries (APK for Android, .app bundle for iOS simulator) @@ -25,16 +24,22 @@ defmodule MobDev.NativeBuild do Runs Android Gradle build if `android/` dir exists. Runs the Mix-driven iOS pipeline (delegating native compile + link to `ios/build.zig` for sim, `ios/build_device.zig` for device) when - `ios/build.zig` exists. Selection between sim and device is driven - by the `device:` opt. + `ios/build.zig` exists. `devices:` supplies the frozen target snapshot used + by `mix mob.deploy`; older direct callers may still use `device:`. """ - @spec build_all(keyword()) :: [:ok | {:error, term()}] + @spec build_all(keyword()) :: boolean() def build_all(opts \\ []) do cfg = load_config() platforms = Keyword.get(opts, :platforms, [:android, :ios]) device_id = Keyword.get(opts, :device, nil) + devices = Keyword.get(opts, :devices, :discover) slim = Keyword.get(opts, :slim, true) - platforms = narrow_platforms_for_device(platforms, device_id) + + platforms = + if devices == :discover, + do: narrow_platforms_for_device(platforms, device_id), + else: platforms + Process.put(:mob_slim, slim) # Always regenerate the runtime plugin manifest from the CURRENT activated @@ -92,47 +97,20 @@ defmodule MobDev.NativeBuild do results true -> - [build_android(cfg, device_id) | results] - end - - results = - if :ios in platforms do - physical_udid = - cond do - is_binary(device_id) and ios_physical_udid?(device_id) -> - device_id - - is_nil(device_id) -> - auto_detect_physical_ios() - - true -> - nil - end - - cond do - not ios_toolchain_available?() -> - warn_skipped_ios() - results - - physical_udid -> - [build_ios_physical(cfg, physical_udid) | results] - - File.exists?("ios/build.zig") -> - [build_ios(cfg, device_id) | results] - - true -> - if :ios in Keyword.get(opts, :requested, []) do - IO.puts( - " #{IO.ANSI.yellow()}⚠ Skipping iOS build — no ios/build.zig in this project#{IO.ANSI.reset()}" - ) + android_targets = + case devices do + :discover -> device_id + selected -> for d <- selected, d.platform == :android, do: d.serial end - results - end - else - results + [build_android(cfg, android_targets) | results] end + results = + if :ios in platforms, + do: build_ios_targets(results, cfg, devices, device_id, opts), + else: results + if results == [] do IO.puts( " #{IO.ANSI.yellow()}No native build targets found (missing android/ or ios/build.zig, or toolchains)#{IO.ANSI.reset()}" @@ -166,6 +144,65 @@ defmodule MobDev.NativeBuild do end end + defp build_ios_targets(results, cfg, :discover, device_id, opts) do + physical_udid = + cond do + is_binary(device_id) and ios_physical_udid?(device_id) -> device_id + is_nil(device_id) -> auto_detect_physical_ios() + true -> nil + end + + cond do + not ios_toolchain_available?() -> + warn_skipped_ios() + results + + physical_udid -> + [build_ios_physical(cfg, physical_udid) | results] + + File.exists?("ios/build.zig") -> + [build_ios(cfg, device_id) | results] + + true -> + warn_missing_ios_scaffold(opts) + results + end + end + + defp build_ios_targets(results, cfg, devices, _device_id, opts) when is_list(devices) do + physical_ids = for d <- devices, d.platform == :ios and Device.physical?(d), do: d.serial + + simulator_ids = + for d <- devices, d.platform == :ios and not Device.physical?(d), do: d.serial + + cond do + not ios_toolchain_available?() -> + warn_skipped_ios() + results + + physical_ids == [] and simulator_ids == [] and devices != [] -> + results + + true -> + results = Enum.reduce(physical_ids, results, &[build_ios_physical(cfg, &1) | &2]) + + if File.exists?("ios/build.zig") and (simulator_ids != [] or devices == []) do + [build_ios(cfg, simulator_ids) | results] + else + if simulator_ids != [], do: warn_missing_ios_scaffold(opts) + results + end + end + end + + defp warn_missing_ios_scaffold(opts) do + if :ios in Keyword.get(opts, :requested, []) do + IO.puts( + " #{IO.ANSI.yellow()}⚠ Skipping iOS build — no ios/build.zig in this project#{IO.ANSI.reset()}" + ) + end + end + @doc """ Whether a native build run succeeded, given what it produced and what the user explicitly asked for. @@ -1343,71 +1380,72 @@ defmodule MobDev.NativeBuild do defp adb_install_all(apk, bundle_id, device_id) do case System.cmd("adb", ["devices"], stderr_to_stdout: true) do {output, 0} -> - serials = + available = output |> String.split("\n") |> Enum.drop(1) |> Enum.filter(&String.contains?(&1, "\tdevice")) |> Enum.map(&hd(String.split(&1, "\t"))) - |> filter_serials(device_id) - Enum.each(serials, fn serial -> - IO.puts(" Installing APK on #{serial}...") + with {:ok, serials} <- resolve_frozen_adb_targets(available, device_id) do + Enum.each(serials, fn serial -> + IO.puts(" Installing APK on #{serial}...") - System.cmd("adb", ["-s", serial, "shell", "am", "force-stop", bundle_id], - stderr_to_stdout: true - ) + System.cmd("adb", ["-s", serial, "shell", "am", "force-stop", bundle_id], + stderr_to_stdout: true + ) - # Try an in-place reinstall first (`install -r`): it preserves app data - # (on-device identity, screen stores) when the signing key matches — - # the common case once an app pins a committed debug keystore. Only - # when the package can't be updated in place (signature mismatch, - # version downgrade) do we uninstall + install, which clears app data. - {first_out, first_rc} = - System.cmd("adb", ["-s", serial, "install", "-r", apk], stderr_to_stdout: true) - - {install_out, install_rc} = - if needs_clean_reinstall?(first_out, first_rc) do - # Distinguish a genuine package-state rejection (signature or - # version mismatch) from a transient adb error (e.g. device - # offline): a clean reinstall reliably clears app data only in the - # former case, so word the notice accordingly rather than always - # promising "app data will be cleared". - if String.contains?(first_out, "INSTALL_FAILED") do - IO.puts( - " #{IO.ANSI.yellow()}In-place update rejected (signature or version " <> - "mismatch), reinstalling clean (app data will be cleared)#{IO.ANSI.reset()}" - ) + # Try an in-place reinstall first (`install -r`): it preserves app data + # (on-device identity, screen stores) when the signing key matches — + # the common case once an app pins a committed debug keystore. Only + # when the package can't be updated in place (signature mismatch, + # version downgrade) do we uninstall + install, which clears app data. + {first_out, first_rc} = + System.cmd("adb", ["-s", serial, "install", "-r", apk], stderr_to_stdout: true) + + {install_out, install_rc} = + if needs_clean_reinstall?(first_out, first_rc) do + # Distinguish a genuine package-state rejection (signature or + # version mismatch) from a transient adb error (e.g. device + # offline): a clean reinstall reliably clears app data only in the + # former case, so word the notice accordingly rather than always + # promising "app data will be cleared". + if String.contains?(first_out, "INSTALL_FAILED") do + IO.puts( + " #{IO.ANSI.yellow()}In-place update rejected (signature or version " <> + "mismatch), reinstalling clean (app data will be cleared)#{IO.ANSI.reset()}" + ) + else + IO.puts( + " #{IO.ANSI.yellow()}In-place update failed (adb exit #{first_rc}), " <> + "retrying with a clean install#{IO.ANSI.reset()}" + ) + end + + System.cmd("adb", ["-s", serial, "uninstall", bundle_id], stderr_to_stdout: true) + System.cmd("adb", ["-s", serial, "install", apk], stderr_to_stdout: true) else - IO.puts( - " #{IO.ANSI.yellow()}In-place update failed (adb exit #{first_rc}), " <> - "retrying with a clean install#{IO.ANSI.reset()}" - ) + {first_out, first_rc} end - System.cmd("adb", ["-s", serial, "uninstall", bundle_id], stderr_to_stdout: true) - System.cmd("adb", ["-s", serial, "install", apk], stderr_to_stdout: true) - else - {first_out, first_rc} - end - - if install_rc != 0 or String.contains?(install_out, "INSTALL_FAILED") do - reason = - install_out - |> String.split("\n") - |> Enum.find(&String.contains?(&1, "INSTALL_FAILED")) || String.trim(install_out) + if install_rc != 0 or String.contains?(install_out, "INSTALL_FAILED") do + reason = + install_out + |> String.split("\n") + |> Enum.find(&String.contains?(&1, "INSTALL_FAILED")) || String.trim(install_out) - IO.puts( - " #{IO.ANSI.yellow()}⚠ #{serial}: APK install failed — #{reason}#{IO.ANSI.reset()}" - ) + IO.puts( + " #{IO.ANSI.yellow()}⚠ #{serial}: APK install failed — #{reason}#{IO.ANSI.reset()}" + ) - IO.puts(" (OTP push will be skipped for this device)") - else - fix_erts_helper_labels(serial, bundle_id) - end - end) + IO.puts(" (OTP push will be skipped for this device)") + else + fix_erts_helper_labels(serial, bundle_id) + end + end) - :ok + :ok + end {out, _} -> {:error, "adb devices failed: #{out}"} @@ -1460,24 +1498,26 @@ defmodule MobDev.NativeBuild do case System.cmd("adb", ["devices"], stderr_to_stdout: true) do {output, 0} -> - serials = parse_adb_serials(output) |> filter_serials(device_id) - if serials == [], do: IO.puts(" (no devices connected, skipping OTP push)") - - Enum.reduce_while(serials, :ok, fn serial, _ -> - otp_dir = device_otp_dir(serial, otp_arm64, otp_arm32, otp_x86_64) + with {:ok, serials} <- + output |> parse_adb_serials() |> resolve_frozen_adb_targets(device_id) do + if serials == [], do: IO.puts(" (no devices connected, skipping OTP push)") + + Enum.reduce_while(serials, :ok, fn serial, _ -> + otp_dir = device_otp_dir(serial, otp_arm64, otp_arm32, otp_x86_64) + + result = + try do + push_otp_to_device(serial, bundle_id, app_data, otp_dir, elixir_lib) + catch + {:skip, _} -> :ok + end - result = - try do - push_otp_to_device(serial, bundle_id, app_data, otp_dir, elixir_lib) - catch - {:skip, _} -> :ok + case result do + :ok -> {:cont, :ok} + {:error, reason} -> {:halt, {:error, reason}} end - - case result do - :ok -> {:cont, :ok} - {:error, reason} -> {:halt, {:error, reason}} - end - end) + end) + end {out, _} -> {:error, "adb devices failed: #{out}"} @@ -1655,14 +1695,17 @@ defmodule MobDev.NativeBuild do |> Enum.map(&hd(String.split(&1, "\t"))) end - # Filters a list of adb serials by `--device `. The id is matched against - # the serial directly, against an `IP:port` form (auto-strip `:5555`), and - # against a bare IP for WiFi-adb devices. Returns all serials when device_id - # is nil. Returns empty + warning if device_id matches no connected serial. + # Filters adb serials by one legacy `--device ` value or by the exact list + # frozen at task start. An empty frozen list means no installation; it must + # never widen back to every device. @doc false - @spec filter_serials([String.t()], String.t() | nil) :: [String.t()] + @spec filter_serials([String.t()], String.t() | [String.t()] | nil) :: [String.t()] def filter_serials(serials, nil), do: serials + def filter_serials(serials, ids) when is_list(ids) do + Enum.filter(serials, &(&1 in ids)) + end + def filter_serials(serials, id) when is_binary(id) do matches = Enum.filter(serials, fn s -> @@ -1678,6 +1721,23 @@ defmodule MobDev.NativeBuild do matches end + @doc false + @spec resolve_frozen_adb_targets([String.t()], String.t() | [String.t()] | nil) :: + {:ok, [String.t()]} | {:error, String.t()} + def resolve_frozen_adb_targets(serials, ids) when is_list(ids) do + selected = filter_serials(serials, ids) + missing = ids -- selected + + if missing == [] do + {:ok, selected} + else + {:error, "Selected Android device(s) disconnected: #{Enum.join(missing, ", ")}"} + end + end + + def resolve_frozen_adb_targets(serials, device_id), + do: {:ok, filter_serials(serials, device_id)} + defp strip_port(s) do case String.split(s, ":", parts: 2) do [host, _port] -> host @@ -1687,7 +1747,7 @@ defmodule MobDev.NativeBuild do # ── iOS ────────────────────────────────────────────────────────────────────── - defp build_ios(cfg, device_id) do + defp build_ios(cfg, device_ids) do with :ok <- check_path(cfg[:mob_dir], "mob_dir"), :ok <- check_path(cfg[:elixir_lib], "elixir_lib"), {:ok, otp_root} <- MobDev.OtpDownloader.ensure_ios_sim(), @@ -1736,7 +1796,7 @@ defmodule MobDev.NativeBuild do nxeigen_archive, tflite_build ), - {:ok, sim_id} <- pick_ios_sim(device_id), + {:ok, sim_ids} <- resolve_ios_sim_targets(device_ids), binary_path = "ios/zig-out/#{display_name}", :ok <- check_path(binary_path, "iOS binary"), {:ok, app_path} <- bundle_ios_app(binary_path, display_name, cfg), @@ -1746,7 +1806,7 @@ defmodule MobDev.NativeBuild do "ios-arm64_x86_64-simulator", Path.join(app_path, "Frameworks") ), - :ok <- install_ios_sim(sim_id, app_path) do + :ok <- install_ios_sims(sim_ids, app_path) do {:ok, "iOS"} else {:error, reason} -> {:error, "iOS", reason} @@ -1756,6 +1816,18 @@ defmodule MobDev.NativeBuild do end end + @doc false + @spec resolve_ios_sim_targets([String.t()] | String.t() | nil) :: + {:ok, [String.t()]} | {:error, String.t()} + def resolve_ios_sim_targets(ids) when is_list(ids), do: {:ok, ids} + + def resolve_ios_sim_targets(device_id) do + case pick_ios_sim(device_id) do + {:ok, sim_id} -> {:ok, [sim_id]} + {:error, reason} -> {:error, reason} + end + end + # Phase 2 iter 13b: iOS sim build pipeline ported out of build.sh. # Each helper mirrors a section of the prior shell script. @@ -4424,6 +4496,15 @@ defmodule MobDev.NativeBuild do end end + defp install_ios_sims(sim_ids, app_path) do + Enum.reduce_while(sim_ids, :ok, fn sim_id, :ok -> + case install_ios_sim(sim_id, app_path) do + :ok -> {:cont, :ok} + {:error, reason} -> {:halt, {:error, reason}} + end + end) + end + # Physical iOS: compile for device SDK, bundle OTP, sign, install via devicectl. # Mirrors the mob_qa build_device.sh approach but driven from mob.exs config. # diff --git a/lib/mob_dev/task_targets.ex b/lib/mob_dev/task_targets.ex new file mode 100644 index 0000000..a769f57 --- /dev/null +++ b/lib/mob_dev/task_targets.ex @@ -0,0 +1,114 @@ +defmodule MobDev.TaskTargets do + @moduledoc """ + Shared device-selection policy for tasks that can change device state. + + A named device is always explicit. Broad selection keeps physical devices + behind `:all_physical`; `:all_devices` means development emulators and + simulators. With no scope flags, exactly one non-physical device may be + selected automatically. + """ + + alias MobDev.Device + + @type selection_error :: + :no_devices + | :ambiguous_devices + | :no_matching_devices + | :no_dev_devices + | :no_physical_devices + + @doc """ + Resolve a device snapshot into the exact targets for one task run. + """ + @spec resolve([Device.t()], [String.t()], keyword()) :: + {:ok, [Device.t()]} | {:error, selection_error(), map()} + def resolve(all, device_ids, opts) do + selected = select(all, device_ids, opts) + + cond do + all == [] -> + {:error, :no_devices, %{detected: 0}} + + device_ids != [] and selected == [] -> + {:error, :no_matching_devices, %{requested: device_ids, detected: length(all)}} + + selected == [] -> + empty_selection_error(all, opts) + + true -> + {:ok, selected} + end + end + + @doc """ + Select devices from an already-discovered snapshot. + + Precedence is named devices, both broad flags, development devices, + physical devices, then safe single-development-device auto-selection. + """ + @spec select([Device.t()], [String.t()], keyword()) :: [Device.t()] + def select(all, device_ids, opts) do + all_devices? = Keyword.get(opts, :all_devices, false) == true + all_physical? = Keyword.get(opts, :all_physical, false) == true + + cond do + device_ids != [] -> + filter_by_id(all, device_ids) + + all_devices? and all_physical? -> + all + + all_devices? -> + Enum.reject(all, &Device.physical?/1) + + all_physical? -> + Enum.filter(all, &Device.physical?/1) + + true -> + non_physical = Enum.reject(all, &Device.physical?/1) + if length(non_physical) == 1, do: non_physical, else: [] + end + end + + @doc """ + Filter devices by the identifiers accepted by `MobDev.Device.match_id?/2`. + """ + @spec filter_by_id([Device.t()], [String.t()]) :: [Device.t()] + def filter_by_id(devices, []), do: devices + + def filter_by_id(devices, ids) do + Enum.filter(devices, fn device -> + Enum.any?(ids, &Device.match_id?(device, &1)) + end) + end + + defp empty_selection_error(all, opts) do + physical = Enum.filter(all, &Device.physical?/1) + non_physical = Enum.reject(all, &Device.physical?/1) + all_devices? = Keyword.get(opts, :all_devices, false) == true + all_physical? = Keyword.get(opts, :all_physical, false) == true + + cond do + all_devices? and non_physical == [] -> + {:error, :no_dev_devices, + %{ + physical_count: length(physical), + hint: + "Only physical devices are connected. `--all-devices` targets " <> + "emulators/simulators only; use `--all-physical` to include " <> + "physical devices, or `--device ` to target one explicitly." + }} + + all_physical? and physical == [] -> + {:error, :no_physical_devices, %{detected: length(all)}} + + true -> + {:error, :ambiguous_devices, + %{ + detected: length(all), + non_physical: length(non_physical), + physical: length(physical) + }} + end + end +end diff --git a/lib/mob_dev/uninstaller.ex b/lib/mob_dev/uninstaller.ex index 895f416..051ee18 100644 --- a/lib/mob_dev/uninstaller.ex +++ b/lib/mob_dev/uninstaller.ex @@ -41,7 +41,7 @@ defmodule MobDev.Uninstaller do """ alias MobDev.Discovery.{Android, IOS} - alias MobDev.Device + alias MobDev.{Device, TaskTargets} @type outcome :: :uninstalled | :skipped | :error @type result :: %{ @@ -58,12 +58,7 @@ defmodule MobDev.Uninstaller do @type plan :: [{Device.t(), [String.t()]}] @typedoc "Why `plan/1` couldn't build a matrix." - @type plan_error :: - :no_devices - | :ambiguous_devices - | :no_matching_devices - | :no_dev_devices - | :no_physical_devices + @type plan_error :: TaskTargets.selection_error() @doc """ Build the (devices × apps) plan without executing it. @@ -86,24 +81,10 @@ defmodule MobDev.Uninstaller do def plan(opts \\ []) do all = list_all_devices(opts) device_ids = opts[:device_ids] || [] - selected = select_devices(all, device_ids, opts) - cond do - all == [] -> - {:error, :no_devices, %{detected: 0}} - - device_ids != [] and selected == [] -> - {:error, :no_matching_devices, %{requested: device_ids, detected: length(all)}} - - selected == [] -> - # Could mean: --all-devices was set but no emulators/sims - # connected (only physical), OR no flags + multiple devices - # connected, OR no flags + zero non-physical devices. Use - # the available counts to pick the right error. - ambiguous_or_only_physical_error(all, opts) - - true -> - {:ok, build_plan(selected, opts)} + case TaskTargets.resolve(all, device_ids, opts) do + {:ok, selected} -> {:ok, build_plan(selected, opts)} + {:error, reason, context} -> {:error, reason, context} end end @@ -126,64 +107,7 @@ defmodule MobDev.Uninstaller do Public for testing — the precedence ladder is the safety contract. """ @spec select_devices([Device.t()], [String.t()], keyword()) :: [Device.t()] - def select_devices(all, device_ids, opts) do - # Coerce to explicit booleans — opts[:foo] is nil when the flag - # wasn't passed, and `nil and X` crashes under Elixir 1.20. - all_devices? = Keyword.get(opts, :all_devices, false) == true - all_physical? = Keyword.get(opts, :all_physical, false) == true - - cond do - device_ids != [] -> - filter_devices_by_id(all, device_ids) - - all_devices? and all_physical? -> - all - - all_devices? -> - Enum.reject(all, &Device.physical?/1) - - all_physical? -> - Enum.filter(all, &Device.physical?/1) - - true -> - non_physical = Enum.reject(all, &Device.physical?/1) - if length(non_physical) == 1, do: non_physical, else: [] - end - end - - defp ambiguous_or_only_physical_error(all, opts) do - physical = Enum.filter(all, &Device.physical?/1) - non_physical = Enum.reject(all, &Device.physical?/1) - # Same boolean-coercion guard as in select_devices/3. - all_devices? = Keyword.get(opts, :all_devices, false) == true - all_physical? = Keyword.get(opts, :all_physical, false) == true - - cond do - all_devices? and non_physical == [] -> - # --all-devices targets non-physical; user has only phones. - {:error, :no_dev_devices, - %{ - physical_count: length(physical), - hint: - "Only physical devices connected. `--all-devices` targets " <> - "emulators/simulators only — use `--all-physical` to also " <> - "uninstall on physical devices, or `--device ` to " <> - "target one explicitly." - }} - - all_physical? and physical == [] -> - {:error, :no_physical_devices, %{detected: length(all)}} - - true -> - # No flags + ambiguous (>1 non-physical) OR no non-physical at all. - {:error, :ambiguous_devices, - %{ - detected: length(all), - non_physical: length(non_physical), - physical: length(physical) - }} - end - end + defdelegate select_devices(all, device_ids, opts), to: TaskTargets, as: :select @doc """ Execute a `plan/0` against the connected devices. Returns @@ -251,13 +175,7 @@ defmodule MobDev.Uninstaller do Used by `resolve_devices/1` when the user passes `--device foo`. """ @spec filter_devices_by_id([Device.t()], [String.t()]) :: [Device.t()] - def filter_devices_by_id(devices, []), do: devices - - def filter_devices_by_id(devices, ids) do - Enum.filter(devices, fn d -> - Enum.any?(ids, &Device.match_id?(d, &1)) - end) - end + defdelegate filter_devices_by_id(devices, ids), to: TaskTargets, as: :filter_by_id @doc """ Parse `adb uninstall` output into an outcome. diff --git a/test/mix/tasks/mob_deploy_beam_flags_test.exs b/test/mix/tasks/mob_deploy_beam_flags_test.exs index 8dcb8a6..43824ef 100644 --- a/test/mix/tasks/mob_deploy_beam_flags_test.exs +++ b/test/mix/tasks/mob_deploy_beam_flags_test.exs @@ -417,6 +417,17 @@ defmodule Mix.Tasks.Mob.DeployBeamFlagsTest do assert Deploy.missing_device_message("emulator-5554", [], [], skipped) =~ "was skipped — nothing was deployed" end + + test "ANDROID_SERIAL has the same explicit-target exit semantics" do + skipped = [device("emulator-5554", "app not installed")] + + assert Deploy.missing_device_message( + {:android_serial, "emulator-5554"}, + [], + [], + skipped + ) =~ "ANDROID_SERIAL=emulator-5554 was skipped" + end end describe "json_result/4" do diff --git a/test/mix/tasks/mob_deploy_parsing_test.exs b/test/mix/tasks/mob_deploy_parsing_test.exs index d4fab47..90d77fe 100644 --- a/test/mix/tasks/mob_deploy_parsing_test.exs +++ b/test/mix/tasks/mob_deploy_parsing_test.exs @@ -11,6 +11,7 @@ defmodule Mix.Tasks.Mob.DeployParsingTest do """ use ExUnit.Case, async: true + alias MobDev.Device alias Mix.Tasks.Mob.Deploy defp parse(argv) do @@ -32,6 +33,8 @@ defmodule Mix.Tasks.Mob.DeployParsingTest do ~w(--native --android), ~w(--device ABC123), ~w(-d ABC123), + ~w(--all-devices), + ~w(--all-physical), ~w(--dist-port 9200), ~w(--node-suffix sim1), ~w(--schedulers 4), @@ -70,6 +73,104 @@ defmodule Mix.Tasks.Mob.DeployParsingTest do end end + describe "target resolution" do + defp device(serial, platform, type) do + %Device{serial: serial, name: serial, platform: platform, type: type, status: :discovered} + end + + test "ANDROID_SERIAL selects its exact Android target" do + emulator = device("emulator-5554", :android, :emulator) + phone = device("ZY22K6BSJM", :android, :physical) + + assert {:ok, [^phone]} = + Deploy.resolve_targets([emulator, phone], [:android], [], "ZY22K6BSJM") + end + + test "--device takes precedence over ANDROID_SERIAL" do + emulator = device("emulator-5554", :android, :emulator) + phone = device("ZY22K6BSJM", :android, :physical) + + assert {:ok, [^emulator]} = + Deploy.resolve_targets( + [emulator, phone], + [:android], + [device: "emulator-5554"], + "ZY22K6BSJM" + ) + end + + test "--all-devices takes precedence over ANDROID_SERIAL and excludes phones" do + emulator = device("emulator-5554", :android, :emulator) + phone = device("ZY22K6BSJM", :android, :physical) + + assert {:ok, [^emulator]} = + Deploy.resolve_targets( + [emulator, phone], + [:android], + [all_devices: true], + "ZY22K6BSJM" + ) + end + + test "an iOS-only run ignores ANDROID_SERIAL" do + simulator = device("SIM-UDID", :ios, :simulator) + + assert {:ok, [^simulator]} = + Deploy.resolve_targets([simulator], [:ios], [], "ZY22K6BSJM") + end + + test "a mixed-platform run ignores ANDROID_SERIAL and auto-picks the sole dev device" do + # The default macOS `mix mob.deploy` runs on `[:android, :ios]`. A + # leftover `ANDROID_SERIAL` in the shell (from adb-based tooling) must + # not hijack the default and turn every bare deploy into an explicit + # single-device selection — that would raise :no_matching_devices + # against any connected iOS simulator. The decision record narrows the + # ANDROID_SERIAL rule to Android-only runs on purpose. + simulator = device("SIM-UDID", :ios, :simulator) + + assert {:ok, [^simulator]} = + Deploy.resolve_targets([simulator], [:android, :ios], [], "some-stale-serial") + end + + test "whitespace-only ANDROID_SERIAL is treated as unset" do + # A shell that exports `ANDROID_SERIAL=""` or `ANDROID_SERIAL=" "` + # (some CI wrappers do this) must not raise a "no device matched" + # error looking for `" "` — the shape is functionally unset. + emulator = device("emulator-5554", :android, :emulator) + + assert {:ok, [^emulator]} = + Deploy.resolve_targets([emulator], [:android], [], " ") + end + + test "zero devices remains a valid native artifact-build input" do + assert {:ok, []} = + Deploy.resolve_targets([], [:android], [native: true, android: true], nil) + end + + test "explicit selection fails before work when it matches nothing" do + assert {:error, message} = + Deploy.resolve_targets([], [:android], [device: "missing"], nil) + + assert message =~ "missing" + end + + test "selected platforms come only from the frozen device snapshot" do + simulator = device("SIM-UDID", :ios, :simulator) + assert Deploy.selected_platforms([:android, :ios], [simulator]) == [:ios] + assert Deploy.selected_platforms([:android, :ios], []) == [:android, :ios] + end + + test "broad target scopes make their selected platforms required" do + assert Deploy.required_platforms([all_devices: true], [:android, :ios]) == [ + :android, + :ios + ] + + assert Deploy.required_platforms([ios: true], [:ios]) == [:ios] + assert Deploy.required_platforms([], [:android]) == [] + end + end + describe "what it still refuses" do test "an unrecognised flag" do assert {_, _, [{"--devcie", _}]} = parse(~w(--devcie X)) diff --git a/test/mob_dev/deployer_test.exs b/test/mob_dev/deployer_test.exs index cc50102..1219283 100644 --- a/test/mob_dev/deployer_test.exs +++ b/test/mob_dev/deployer_test.exs @@ -1,9 +1,44 @@ defmodule MobDev.DeployerTest do use ExUnit.Case, async: true - alias MobDev.Deployer + alias MobDev.{Deployer, Device} + + describe "frozen targets" do + test "an explicit device snapshot bypasses discovery and excludes unrelated devices" do + selected = %Device{platform: :android, serial: "emulator-5554", type: :emulator} + + fail_if_called = fn -> flunk("device discovery must not run after targets are frozen") end + + assert Deployer.target_devices( + [devices: [selected], android_lister: fail_if_called, ios_lister: fail_if_called], + [:android, :ios] + ) == [selected] + end + end describe "physical iOS override" do + test "app-not-installed skips never claim the override is incomplete" do + Process.put(:mob_ios_override_replaced, true) + on_exit(fn -> Process.delete(:mob_ios_override_replaced) end) + + assert {:skipped, "App is not installed"} = + Deployer.finalize_ios_override_result( + {:skipped, "App is not installed"}, + "sample_app" + ) + end + + test "real copy failures still explain an incomplete override" do + Process.put(:mob_ios_override_replaced, true) + on_exit(fn -> Process.delete(:mob_ios_override_replaced) end) + + assert {:error, message} = + Deployer.finalize_ios_override_result({:error, "copy failed"}, "sample_app") + + assert message =~ "copy failed" + assert message =~ "now incomplete" + end + test "requires staged bootstrap bytes to match the active compile output" do root = Path.join(System.tmp_dir!(), "mob_ios_override_#{System.unique_integer()}") compile_path = Path.join(root, "compile") diff --git a/test/mob_dev/native_build_test.exs b/test/mob_dev/native_build_test.exs index 57e94ea..7c22ac4 100644 --- a/test/mob_dev/native_build_test.exs +++ b/test/mob_dev/native_build_test.exs @@ -331,6 +331,27 @@ defmodule MobDev.NativeBuildTest do assert NativeBuild.filter_serials(@serials, nil) == @serials end + test "an empty frozen target list selects nothing" do + assert NativeBuild.filter_serials(@serials, []) == [] + end + + test "a frozen target list cannot grow when another adb device appears" do + assert NativeBuild.filter_serials(@serials ++ ["UNRELATED"], ["emulator-5554"]) == [ + "emulator-5554" + ] + end + + test "a selected device disappearing fails instead of widening the target set" do + assert {:error, message} = + NativeBuild.resolve_frozen_adb_targets( + ["emulator-5554", "UNRELATED"], + ["emulator-5554", "PHONE"] + ) + + assert message =~ "PHONE" + refute message =~ "UNRELATED" + end + test "exact serial match" do assert NativeBuild.filter_serials(@serials, "ZY22K6BSJM") == ["ZY22K6BSJM"] end @@ -1336,6 +1357,14 @@ defmodule MobDev.NativeBuildTest do File.write!(Path.join(pkg, "_ext.so"), <<0xCA, 0xFE, 0xBA, 0xBE>>) end + describe "resolve_ios_sim_targets/1" do + test "a frozen simulator list is accepted without discovery" do + ids = ["SIM-A", "SIM-B"] + assert NativeBuild.resolve_ios_sim_targets(ids) == {:ok, ids} + assert NativeBuild.resolve_ios_sim_targets([]) == {:ok, []} + end + end + # ── resolve_booted_udid/2 ─────────────────────────────────────────────── # # Regression: `mix mob.deploy --native --device defd4bdc` failed at diff --git a/test/mob_dev/task_targets_test.exs b/test/mob_dev/task_targets_test.exs new file mode 100644 index 0000000..9087d96 --- /dev/null +++ b/test/mob_dev/task_targets_test.exs @@ -0,0 +1,46 @@ +defmodule MobDev.TaskTargetsTest do + use ExUnit.Case, async: true + + alias MobDev.{Device, TaskTargets} + + defp device(serial, platform, type) do + %Device{serial: serial, name: serial, platform: platform, type: type, status: :discovered} + end + + test "default selection chooses the sole development device and leaves phones alone" do + emulator = device("emulator-5554", :android, :emulator) + android_phone = device("PHONE", :android, :physical) + iphone = device("IPHONE", :ios, :physical) + + assert {:ok, [^emulator]} = TaskTargets.resolve([emulator, android_phone, iphone], [], []) + end + + test "default selection refuses multiple development devices" do + devices = [ + device("emulator-5554", :android, :emulator), + device("SIM-UDID", :ios, :simulator) + ] + + assert {:error, :ambiguous_devices, %{non_physical: 2}} = + TaskTargets.resolve(devices, [], []) + end + + test "broad scopes keep physical and development targets separate" do + emulator = device("emulator-5554", :android, :emulator) + simulator = device("SIM-UDID", :ios, :simulator) + phone = device("PHONE", :android, :physical) + all = [emulator, simulator, phone] + + assert TaskTargets.select(all, [], all_devices: true) == [emulator, simulator] + assert TaskTargets.select(all, [], all_physical: true) == [phone] + + assert TaskTargets.select(all, [], all_devices: true, all_physical: true) == all + end + + test "an explicit identifier can select one physical device" do + emulator = device("emulator-5554", :android, :emulator) + phone = device("PHONE", :android, :physical) + + assert {:ok, [^phone]} = TaskTargets.resolve([emulator, phone], ["phone"], []) + end +end diff --git a/test/mob_dev/wiring_test.exs b/test/mob_dev/wiring_test.exs index f68e2e9..7535c6c 100644 --- a/test/mob_dev/wiring_test.exs +++ b/test/mob_dev/wiring_test.exs @@ -98,10 +98,8 @@ defmodule MobDev.WiringTest do test "the catch clause returns it as skipped without override annotation" do # The copy never began, so nothing on the device was replaced and there # is no partial-override warning to attach. - body = region(@deployer, " catch\n # Not annotated", "\n after") - - assert body =~ "{:skipped, reason} ->" - refute body =~ "{:skipped, reason} ->\n {:error," + assert @deployer =~ + "{:skipped, reason} ->\n finalize_ios_override_result({:skipped, reason}, app)" end end @@ -114,7 +112,7 @@ defmodule MobDev.WiringTest do body = region(@deploy_task, "Enum.each(format_summary(", "\n end") assert body =~ "failure_message(" - assert body =~ "missing_device_message(device_id, deployed, failed, skipped)" + assert body =~ "missing_device_message(target_reference, deployed, failed, skipped)" assert body =~ "message -> Mix.raise(message)" # Order matters: raising before the summary loses the per-device detail @@ -122,7 +120,7 @@ defmodule MobDev.WiringTest do # against the whole file — anchoring the region ON the summary made this # `assert 0 < raise_at`, which cannot fail. assert index_of(@deploy_task, "Enum.each(format_summary(") < - index_of(@deploy_task, "Mix.raise(message)") + index_of(@deploy_task, "case message do") end end @@ -135,7 +133,7 @@ defmodule MobDev.WiringTest do test "the task tells the build which platforms were asked for" do body = region(@deploy_task, "MobDev.NativeBuild.build_all(", "\n )") - assert body =~ "requested: requested_platforms(opts)" + assert body =~ "requested: required_platforms" end test "the build reads that back and runs it through build_outcome/2" do From 983687d4922170bf7166e799aaf2784b19f965e7 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Fri, 11 Sep 2026 18:01:15 -0600 Subject: [PATCH 2/2] Address pre-merge review notes on MOB-169 - CHANGELOG: name the actual iOS refactor (extracted finalize_ios_override_result/2 so :skipped can no longer inherit the incomplete-override wording) instead of implying a mid-copy fix that the diff does not make. - TaskTargets moduledoc: spell out that combining --all-devices and --all-physical selects every connected device, and that multiple development devices with no scope flag is an ambiguity error. Behaviour unchanged; docs only. Co-Authored-By: Claude Opus 4.7 --- CHANGELOG.md | 6 ++++-- lib/mob_dev/task_targets.ex | 10 ++++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d7f288..dd7a99d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,10 @@ Compatibility checks, native installs, and the final BEAM push all consume the same snapshot, so a device appearing during the build cannot join the - operation. App-not-installed skips on physical iOS also no longer inherit - the incomplete-override warning used for real mid-copy failures. + operation. On the iOS path, the diagnostic that annotates a mid-copy + failure is now extracted (`Deployer.finalize_ios_override_result/2`) so + a `:skipped` result stays `:skipped` and only real `:error` finalizations + can inherit the incomplete-override wording. **Behaviour change** — a bare `mix mob.deploy` with **two or more emulators or simulators running for parallel testing** used to fan out to both, and diff --git a/lib/mob_dev/task_targets.ex b/lib/mob_dev/task_targets.ex index a769f57..7382c8d 100644 --- a/lib/mob_dev/task_targets.ex +++ b/lib/mob_dev/task_targets.ex @@ -2,10 +2,12 @@ defmodule MobDev.TaskTargets do @moduledoc """ Shared device-selection policy for tasks that can change device state. - A named device is always explicit. Broad selection keeps physical devices - behind `:all_physical`; `:all_devices` means development emulators and - simulators. With no scope flags, exactly one non-physical device may be - selected automatically. + A named device is always explicit. Broad selection keeps physical + devices behind `:all_physical`; `:all_devices` means development + emulators and simulators. **Combined `:all_devices` and + `:all_physical` selects every connected device.** With no scope + flags, exactly one non-physical device may be selected automatically; + multiple development devices are an ambiguity error. """ alias MobDev.Device