From f8e6e552078a66a70e958372cb075e9444d63e39 Mon Sep 17 00:00:00 2001 From: Lucas Nogueira Date: Thu, 9 Nov 2023 08:35:11 -0300 Subject: [PATCH 1/2] fix: conflict between Apple and Android targets The TargetTrait used a static value to determine the `name_list` which caused a conflict on the Android vs Apple implementation. Moving the static to each type fixes it. --- .changes/fix-targets.md | 5 +++++ src/android/target.rs | 8 ++++++++ src/apple/target.rs | 8 ++++++++ src/target.rs | 7 +------ 4 files changed, 22 insertions(+), 6 deletions(-) create mode 100644 .changes/fix-targets.md diff --git a/.changes/fix-targets.md b/.changes/fix-targets.md new file mode 100644 index 00000000..f9b41043 --- /dev/null +++ b/.changes/fix-targets.md @@ -0,0 +1,5 @@ +--- +"cargo-mobile2": patch +--- + +Fixes conflicts between Apple and Android targets. diff --git a/src/android/target.rs b/src/android/target.rs index 34875817..14bb9739 100644 --- a/src/android/target.rs +++ b/src/android/target.rs @@ -158,6 +158,14 @@ impl<'a> TargetTrait<'a> for Target<'a> { }) } + fn name_list() -> &'static [&'a str] + where + Self: 'static, + { + static INSTANCE: OnceCell> = OnceCell::new(); + INSTANCE.get_or_init(|| Self::all().keys().copied().collect::>()) + } + fn triple(&'a self) -> &'a str { self.triple } diff --git a/src/apple/target.rs b/src/apple/target.rs index ee0ca17e..cb9c388d 100644 --- a/src/apple/target.rs +++ b/src/apple/target.rs @@ -188,6 +188,14 @@ impl<'a> TargetTrait<'a> for Target<'a> { }) } + fn name_list() -> &'static [&'a str] + where + Self: 'static, + { + static INSTANCE: OnceCell> = OnceCell::new(); + INSTANCE.get_or_init(|| Self::all().keys().copied().collect::>()) + } + fn triple(&'a self) -> &'a str { self.triple } diff --git a/src/target.rs b/src/target.rs index 7123406c..e98b9bb0 100644 --- a/src/target.rs +++ b/src/target.rs @@ -1,5 +1,4 @@ use crate::util; -use once_cell_regex::exports::once_cell::sync::OnceCell; use std::{ collections::BTreeMap, fmt::{self, Debug, Display}, @@ -13,11 +12,7 @@ pub trait TargetTrait<'a>: Debug + Sized { fn name_list() -> &'static [&'a str] where - Self: 'static, - { - static INSTANCE: OnceCell> = OnceCell::new(); - INSTANCE.get_or_init(|| Self::all().keys().copied().collect::>()) - } + Self: 'static; fn default_ref() -> &'a Self { Self::all() From 26c72e6e30cc01a185239fe3722ef5bea2191a62 Mon Sep 17 00:00:00 2001 From: Lucas Nogueira Date: Mon, 13 Nov 2023 21:10:06 -0300 Subject: [PATCH 2/2] code review suggestions --- .changes/fix-targets.md | 4 ++-- src/android/cli.rs | 8 ++++---- src/android/target.rs | 8 ++------ src/apple/cli.rs | 6 +++--- src/apple/device/devicectl/device_list.rs | 2 +- src/apple/device/mod.rs | 2 +- src/apple/target.rs | 8 ++------ src/apple/version_number.rs | 2 +- src/target.rs | 4 +--- 9 files changed, 17 insertions(+), 27 deletions(-) diff --git a/.changes/fix-targets.md b/.changes/fix-targets.md index f9b41043..fdbf532e 100644 --- a/.changes/fix-targets.md +++ b/.changes/fix-targets.md @@ -1,5 +1,5 @@ --- -"cargo-mobile2": patch +"cargo-mobile2": minor --- -Fixes conflicts between Apple and Android targets. +Fixes conflicts between Apple and Android targets. `Target::name_list` now returns `Vec<&str>`. diff --git a/src/android/cli.rs b/src/android/cli.rs index f6c64f47..9e79c3ab 100644 --- a/src/android/cli.rs +++ b/src/android/cli.rs @@ -52,12 +52,12 @@ pub enum Command { Open, #[structopt(name = "check", about = "Checks if code compiles for target(s)")] Check { - #[structopt(name = "targets", default_value = Target::DEFAULT_KEY, possible_values = Target::name_list())] + #[structopt(name = "targets", default_value = Target::DEFAULT_KEY, possible_values = &Target::name_list())] targets: Vec, }, #[structopt(name = "build", about = "Builds dynamic libraries for target(s)")] Build { - #[structopt(name = "targets", default_value = Target::DEFAULT_KEY, possible_values = Target::name_list())] + #[structopt(name = "targets", default_value = Target::DEFAULT_KEY, possible_values = &Target::name_list())] targets: Vec, #[structopt(flatten)] profile: cli::Profile, @@ -97,7 +97,7 @@ pub enum Command { pub enum ApkSubcommand { #[structopt(about = "build APKs (Android Package Kit)")] Build { - #[structopt(name = "targets", possible_values = Target::name_list())] + #[structopt(name = "targets", possible_values = &Target::name_list())] /// Which targets to build (all by default). targets: Vec, #[structopt(flatten)] @@ -110,7 +110,7 @@ pub enum ApkSubcommand { pub enum AabSubcommand { #[structopt(about = "build AABs (Android App Bundle)")] Build { - #[structopt(name = "targets", possible_values = Target::name_list())] + #[structopt(name = "targets", possible_values = &Target::name_list())] /// Which targets to build (all by default). targets: Vec, #[structopt(flatten)] diff --git a/src/android/target.rs b/src/android/target.rs index 14bb9739..e0379c82 100644 --- a/src/android/target.rs +++ b/src/android/target.rs @@ -158,12 +158,8 @@ impl<'a> TargetTrait<'a> for Target<'a> { }) } - fn name_list() -> &'static [&'a str] - where - Self: 'static, - { - static INSTANCE: OnceCell> = OnceCell::new(); - INSTANCE.get_or_init(|| Self::all().keys().copied().collect::>()) + fn name_list() -> Vec<&'a str> { + Self::all().keys().copied().collect::>() } fn triple(&'a self) -> &'a str { diff --git a/src/apple/cli.rs b/src/apple/cli.rs index 4d2aeb80..acb1244d 100644 --- a/src/apple/cli.rs +++ b/src/apple/cli.rs @@ -65,12 +65,12 @@ pub enum Command { Open, #[structopt(name = "check", about = "Checks if code compiles for target(s)")] Check { - #[structopt(name = "targets", default_value = Target::DEFAULT_KEY, possible_values = Target::name_list())] + #[structopt(name = "targets", default_value = Target::DEFAULT_KEY, possible_values = &Target::name_list())] targets: Vec, }, #[structopt(name = "build", about = "Builds static libraries for target(s)")] Build { - #[structopt(name = "targets", default_value = Target::DEFAULT_KEY, possible_values = Target::name_list())] + #[structopt(name = "targets", default_value = Target::DEFAULT_KEY, possible_values = &Target::name_list())] targets: Vec, #[structopt(flatten)] profile: cli::Profile, @@ -79,7 +79,7 @@ pub enum Command { Archive { #[structopt(long = "build-number")] build_number: Option, - #[structopt(name = "targets", default_value = Target::DEFAULT_KEY, possible_values = Target::name_list())] + #[structopt(name = "targets", default_value = Target::DEFAULT_KEY, possible_values = &Target::name_list())] targets: Vec, #[structopt(flatten)] profile: cli::Profile, diff --git a/src/apple/device/devicectl/device_list.rs b/src/apple/device/devicectl/device_list.rs index 406ddf4b..723d7023 100644 --- a/src/apple/device/devicectl/device_list.rs +++ b/src/apple/device/devicectl/device_list.rs @@ -122,6 +122,6 @@ pub fn device_list<'a>(env: &Env) -> Result>, DeviceListErro .run() .map_err(DeviceListError::DetectionFailed)?; - let contents = read_to_string(&json_output_path_)?; + let contents = read_to_string(json_output_path_)?; parse_device_list(contents) } diff --git a/src/apple/device/mod.rs b/src/apple/device/mod.rs index 75302ab1..7ab4a88b 100644 --- a/src/apple/device/mod.rs +++ b/src/apple/device/mod.rs @@ -197,6 +197,6 @@ pub fn list_devices<'a>(env: &Env) -> Result>, String> { } } -pub fn list_simulators<'a>(env: &Env) -> Result, String> { +pub fn list_simulators(env: &Env) -> Result, String> { simctl::device_list(env).map_err(|e| e.to_string()) } diff --git a/src/apple/target.rs b/src/apple/target.rs index cb9c388d..824c9280 100644 --- a/src/apple/target.rs +++ b/src/apple/target.rs @@ -188,12 +188,8 @@ impl<'a> TargetTrait<'a> for Target<'a> { }) } - fn name_list() -> &'static [&'a str] - where - Self: 'static, - { - static INSTANCE: OnceCell> = OnceCell::new(); - INSTANCE.get_or_init(|| Self::all().keys().copied().collect::>()) + fn name_list() -> Vec<&'a str> { + Self::all().keys().copied().collect::>() } fn triple(&'a self) -> &'a str { diff --git a/src/apple/version_number.rs b/src/apple/version_number.rs index c70fa836..d47e2b0b 100644 --- a/src/apple/version_number.rs +++ b/src/apple/version_number.rs @@ -49,7 +49,7 @@ impl FromStr for VersionNumber { fn from_str(v: &str) -> Result { match v.split('.').count() { - 1 | 2 | 3 => { + 1..=3 => { let triple = VersionTriple::from_str(v)?; Ok(Self { triple, diff --git a/src/target.rs b/src/target.rs index e98b9bb0..a0892b85 100644 --- a/src/target.rs +++ b/src/target.rs @@ -10,9 +10,7 @@ pub trait TargetTrait<'a>: Debug + Sized { fn all() -> &'a BTreeMap<&'a str, Self>; - fn name_list() -> &'static [&'a str] - where - Self: 'static; + fn name_list() -> Vec<&'a str>; fn default_ref() -> &'a Self { Self::all()