From f5159b8c718282036f60cd424c146f7245d4f9b3 Mon Sep 17 00:00:00 2001 From: GenericJam Date: Sun, 13 Sep 2026 22:59:25 -0600 Subject: [PATCH] fix(MOB-225): make native deploy outcomes honest --- AGENTS.md | 5 +- CHANGELOG.md | 8 +++ README.md | 10 ++-- lib/mix/tasks/mob.deploy.ex | 45 ++++++++++++--- lib/mix/tasks/mob.install.ex | 66 ++++++++++++++++------ lib/mob_dev/native_build.ex | 13 +++-- test/mix/tasks/mob_deploy_parsing_test.exs | 18 ++++++ test/mix/tasks/mob_install_test.exs | 16 +++++- test/mob_dev/wiring_test.exs | 14 +++++ 9 files changed, 157 insertions(+), 38 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3a2d0a9..31237fc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -250,7 +250,10 @@ should know: - **`mix mob.install`** — first-run **project setup**. Downloads the OTP runtime, generates icons, writes `mob.exs`. Per-project, runs - once. Doesn't touch any device. + once. Doesn't touch any device. A later `mix mob.deploy --native` also + repairs or creates `android/local.properties` when it can detect the SDK; + fresh worktrees therefore do not need another interactive install just to + restore that gitignored machine-local file. - **`mix mob.uninstall`** — per-**device** app removal. Sweeps connected devices and removes installed `.app` / `.apk` bundles. Doesn't undo `mix mob.install`'s project setup. diff --git a/CHANGELOG.md b/CHANGELOG.md index cc54a78..5ed686a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ### Fixed +- **`mix mob.deploy --native --device ` can no longer report success after + skipping the named device's native build** (MOB-225). The selected device now + makes its platform a required build target even without a redundant + `--android` / `--ios` flag. Android native deploys also create the gitignored + `android/local.properties` when the SDK is detectable, sharing + `mix mob.install`'s path writer; when it is not detectable, the skipped + requested build exits non-zero. + - **`mix mob.connect --name` is now honored end-to-end** (MOB-69). The option was parsed at the task layer, but `Connector.connect_all/1` called `ensure_local_dist/1` which hardcoded `Node.start(:"mob_dev@127.0.0.1", ...)`. diff --git a/README.md b/README.md index a189689..1ddfa6c 100644 --- a/README.md +++ b/README.md @@ -145,10 +145,12 @@ unloaded at any moment. - 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; +- `--native` built nothing for a platform you **named**, directly or through + `--device` — a non-detectable Android SDK under either + `--android --native` or `--native --device `, say. When the SDK + is detectable, deploy creates the gitignored `android/local.properties` + automatically. A plain `mix mob.deploy --native` that skips a platform + nobody asked for still exits 0; - 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. diff --git a/lib/mix/tasks/mob.deploy.ex b/lib/mix/tasks/mob.deploy.ex index 071f377..e1edf5f 100644 --- a/lib/mix/tasks/mob.deploy.ex +++ b/lib/mix/tasks/mob.deploy.ex @@ -127,6 +127,12 @@ defmodule Mix.Tasks.Mob.Deploy do xcodebuild -scheme -destination 'platform=iOS Simulator,...' build xcrun simctl install booted .app + A named `--device` supplies the platform when `--native` is used, so + `mix mob.deploy --native --device ` does not also need `--android` or + `--ios`. Before an Android build, the task creates + `android/local.properties` when the SDK is detectable, just as + `mix mob.install` does during first-run setup. + ## Exit status Every targeted device is attempted and the full summary printed, then the @@ -148,8 +154,10 @@ defmodule Mix.Tasks.Mob.Deploy do * `mix mob.deploy --android --native` that built the APK with no device attached — exit 0. The artifact is what was asked for. - `--native` fails the run when a platform you named built nothing at all, which - is what a missing `sdk.dir` in `android/local.properties` produces. + `--native` fails the run when a platform you named — directly or through + `--device` — built nothing at all. If the Android SDK cannot be detected, + a missing `sdk.dir` therefore produces a non-zero exit instead of a warning + followed by success. """ alias MobDev.{Device, TaskTargets} @@ -256,7 +264,7 @@ defmodule Mix.Tasks.Mob.Deploy do # 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) + required_platforms = required_platforms(opts, platforms, target_reference) beam_flags = resolve_beam_flags(opts) # Validate every targeted device against the project's enabled @@ -300,6 +308,13 @@ defmodule Mix.Tasks.Mob.Deploy do native_ok = if native do + if :android in platforms do + Mix.Tasks.Mob.Install.write_local_properties( + File.cwd!(), + MobDev.Config.load_mob_config() + ) + end + MobDev.NativeBuild.build_all( platforms: platforms, devices: devices, @@ -663,13 +678,27 @@ defmodule Mix.Tasks.Mob.Deploy do end @doc false - @spec required_platforms(keyword(), [:android | :ios]) :: [:android | :ios] - def required_platforms(opts, selected_platforms) do + @spec required_platforms( + keyword(), + [:android | :ios], + String.t() | {:device | :android_serial, String.t()} | nil + ) :: [:android | :ios] + def required_platforms(opts, selected_platforms, target_reference \\ nil) do requested = requested_platforms(opts) - if requested == [] and (opts[:all_devices] == true or opts[:all_physical] == true), - do: selected_platforms, - else: requested + cond do + requested != [] -> + requested + + opts[:all_devices] == true or opts[:all_physical] == true -> + selected_platforms + + opts[:native] == true and target_reference != nil -> + selected_platforms + + true -> + [] + end end @doc false diff --git a/lib/mix/tasks/mob.install.ex b/lib/mix/tasks/mob.install.ex index 43b580d..79edd02 100644 --- a/lib/mix/tasks/mob.install.ex +++ b/lib/mix/tasks/mob.install.ex @@ -295,33 +295,63 @@ defmodule Mix.Tasks.Mob.Install do @doc false @spec write_local_properties(String.t(), keyword()) :: :ok | nil def write_local_properties(project_dir, cfg) do - props = Path.join(project_dir, "android/local.properties") + write_local_properties(project_dir, cfg, detect_android_sdk()) + end - if File.exists?(props) do - content = File.read!(props) + @doc false + @spec write_local_properties(String.t(), keyword(), String.t() | nil) :: :ok | nil + def write_local_properties(project_dir, cfg, detected_sdk) do + props = Path.join(project_dir, "android/local.properties") - needs_mob_paths? = String.contains?(content, "/path/to/") - needs_sdk_dir? = not has_active_sdk_dir?(content) + cond do + File.exists?(props) -> + content = File.read!(props) - if needs_mob_paths? or needs_sdk_dir? do - otp_dir = MobDev.OtpDownloader.android_otp_dir("arm64-v8a") - otp_dir_arm32 = MobDev.OtpDownloader.android_otp_dir("armeabi-v7a") - otp_dir_x86_64 = MobDev.OtpDownloader.android_otp_dir("x86_64") + needs_mob_paths? = String.contains?(content, "/path/to/") + needs_sdk_dir? = not has_active_sdk_dir?(content) - new_content = - content - |> replace_prop("mob.otp_release", otp_dir) - |> replace_prop("mob.otp_release_arm32", otp_dir_arm32) - |> replace_prop("mob.otp_release_x86_64", otp_dir_x86_64) - |> replace_prop("mob.mob_dir", cfg[:mob_dir]) - |> ensure_sdk_dir(detect_android_sdk()) + if needs_mob_paths? or needs_sdk_dir? do + new_content = configure_local_properties(content, cfg, detected_sdk) + File.write!(props, new_content) + Mix.shell().info([:green, "* android/local.properties configured", :reset]) + end - File.write!(props, new_content) + File.dir?(Path.join(project_dir, "android")) and is_binary(detected_sdk) -> + content = configure_local_properties(local_properties_template(), cfg, detected_sdk) + File.write!(props, content) Mix.shell().info([:green, "* android/local.properties configured", :reset]) - end + + true -> + nil end end + defp configure_local_properties(content, cfg, detected_sdk) do + content + |> replace_prop("mob.otp_release", MobDev.OtpDownloader.android_otp_dir("arm64-v8a")) + |> replace_prop( + "mob.otp_release_arm32", + MobDev.OtpDownloader.android_otp_dir("armeabi-v7a") + ) + |> replace_prop( + "mob.otp_release_x86_64", + MobDev.OtpDownloader.android_otp_dir("x86_64") + ) + |> replace_prop("mob.mob_dir", cfg[:mob_dir]) + |> ensure_sdk_dir(detected_sdk) + end + + defp local_properties_template do + """ + # Machine-local Android and Mob paths. Generated by mix mob.install or mix mob.deploy. + # sdk.dir=/path/to/android/sdk + mob.otp_release=/path/to/otp + mob.otp_release_arm32=/path/to/otp-arm32 + mob.otp_release_x86_64=/path/to/otp-x86_64 + mob.mob_dir=/path/to/mob + """ + end + # Returns true iff local.properties has an *uncommented* `sdk.dir=...` line. # The mob_new template ships with a commented `# sdk.dir=...` placeholder; we # treat that as "not set" so the auto-detection writes a real value over it. diff --git a/lib/mob_dev/native_build.ex b/lib/mob_dev/native_build.ex index 4eac5d0..34454d1 100644 --- a/lib/mob_dev/native_build.ex +++ b/lib/mob_dev/native_build.ex @@ -210,14 +210,17 @@ defmodule MobDev.NativeBuild do `results` entries are `{:ok, label}` / `{:error, label, reason}` where label is the display name ("Android", "iOS", "iOS (device)"). - `requested` is the platforms named by an explicit `--android` / `--ios` - flag — NOT the resolved platform list, which collapses "no flag given" into - every platform and would make an ordinary skip fatal. + `requested` is the platforms required by an explicit `--android` / `--ios` + flag, broad target scope, or named `--device` under `--native` — NOT the + complete resolved platform list, which collapses "no flag given" into every + platform and would make an ordinary skip fatal. The rule this exists for: `ok_count == length(results)` is `0 == 0` for a run that built nothing, so `mix mob.deploy --android --native` with no `sdk.dir` - printed a warning, built nothing, and reported success. A skip is fine when - nobody asked for that platform; it is a failure when they did. + printed a warning, built nothing, and reported success. The same failure + occurred for `--native --device ` because the device-resolved + platform was not considered requested. A skip is fine when nobody asked for + that platform; it is a failure when they did. """ @spec build_outcome([{:ok, String.t()} | {:error, String.t(), term()}], [atom()]) :: :ok | {:error, String.t()} diff --git a/test/mix/tasks/mob_deploy_parsing_test.exs b/test/mix/tasks/mob_deploy_parsing_test.exs index 90d77fe..f734e63 100644 --- a/test/mix/tasks/mob_deploy_parsing_test.exs +++ b/test/mix/tasks/mob_deploy_parsing_test.exs @@ -169,6 +169,24 @@ defmodule Mix.Tasks.Mob.DeployParsingTest do assert Deploy.required_platforms([ios: true], [:ios]) == [:ios] assert Deploy.required_platforms([], [:android]) == [] end + + test "--native makes the named device platform a required build" do + assert Deploy.required_platforms( + [native: true, device: "ZY22K6BSJM"], + [:android], + {:device, "ZY22K6BSJM"} + ) == [:android] + + assert Deploy.required_platforms( + [native: true, device: "SIM-UDID"], + [:ios], + {:device, "SIM-UDID"} + ) == [:ios] + end + + test "a bare --native artifact build does not require every scaffolded platform" do + assert Deploy.required_platforms([native: true], [:android, :ios], nil) == [] + end end describe "what it still refuses" do diff --git a/test/mix/tasks/mob_install_test.exs b/test/mix/tasks/mob_install_test.exs index d5c5b78..b840a18 100644 --- a/test/mix/tasks/mob_install_test.exs +++ b/test/mix/tasks/mob_install_test.exs @@ -102,8 +102,20 @@ defmodule Mix.Tasks.Mob.InstallTest do assert File.read!(props_path(dir)) == original end - test "does nothing when local.properties does not exist", %{dir: dir} do - Install.write_local_properties(dir, mob_dir: dir) + test "creates local.properties when the Android SDK is detectable", %{dir: dir} do + sdk = Path.join(dir, "android-sdk") + File.mkdir_p!(sdk) + + Install.write_local_properties(dir, [mob_dir: dir], sdk) + + content = File.read!(props_path(dir)) + assert content =~ "sdk.dir=#{sdk}" + assert content =~ "mob.otp_release=#{OtpDownloader.android_otp_dir("arm64-v8a")}" + assert content =~ "mob.mob_dir=#{dir}" + end + + test "does not create local.properties when no Android SDK is detectable", %{dir: dir} do + Install.write_local_properties(dir, [mob_dir: dir], nil) refute File.exists?(props_path(dir)) end end diff --git a/test/mob_dev/wiring_test.exs b/test/mob_dev/wiring_test.exs index 7535c6c..391d3c2 100644 --- a/test/mob_dev/wiring_test.exs +++ b/test/mob_dev/wiring_test.exs @@ -134,6 +134,20 @@ defmodule MobDev.WiringTest do body = region(@deploy_task, "MobDev.NativeBuild.build_all(", "\n )") assert body =~ "requested: required_platforms" + + assert @deploy_task =~ + "required_platforms(opts, platforms, target_reference)" + end + + test "the task prepares only Android local.properties before checking the toolchain" do + prepare_at = index_of(@deploy_task, "Mix.Tasks.Mob.Install.write_local_properties(") + build_at = index_of(@deploy_task, "MobDev.NativeBuild.build_all(") + native_build = region(@deploy_task, "native_ok =", "MobDev.NativeBuild.build_all(") + + assert prepare_at < build_at + assert prepare_at > 0 + assert native_build =~ "if :android in platforms do" + assert native_build =~ "Mix.Tasks.Mob.Install.write_local_properties(" end test "the build reads that back and runs it through build_outcome/2" do