From 7afb49b50990e79efd21a14575f9e9147c254c46 Mon Sep 17 00:00:00 2001 From: jhugman Date: Thu, 10 Oct 2024 01:07:58 +0100 Subject: [PATCH] Build for selected Android targets (#119) Fixes #106 According to [The Big O of Code Reviews](https://www.egorand.dev/the-big-o-of-code-reviews/), this is a O(_n_) change. This issue is conceptually simple: we need to add one line to the `build.gradle` template. Driving that line in the file is data about the targets being built. `targets` comes from the `ProjectConfig` (a.k.a. `ubrn.config.yaml`), which makes this straightforward. HOWEVER, `--targets` can be used when building! SO, we can mutate the `ProjectConfig` while we're building. BUT we don't want the second command overwriting the `build.gradle` so carefully specified in the first. ```sh ubrn build android --and-generate --config ubrn.config.yaml --targets aarch64-linux-android,armv7-linux-androideabi ubrn build ios --and-generate --config ubrn.config.yaml ``` THIS MEANT: adding platform specificity to the each of the generated files. Drive-by Review also requested from @Johennes , if interested. --- crates/ubrn_cli/src/android.rs | 31 +++------ crates/ubrn_cli/src/building.rs | 27 ++++++-- crates/ubrn_cli/src/codegen/mod.rs | 68 +++++++++++++++++-- .../src/codegen/templates/build.gradle | 7 ++ crates/ubrn_cli/src/config/mod.rs | 6 +- crates/ubrn_cli/src/generate.rs | 55 +++++++++++++-- crates/ubrn_cli/src/ios.rs | 22 +++--- crates/ubrn_cli/src/main.rs | 6 ++ crates/ubrn_cli/src/rust.rs | 6 +- crates/ubrn_common/src/files.rs | 3 + docs/src/getting-started/guide.md | 8 +++ docs/src/reference/commandline.md | 8 ++- 12 files changed, 188 insertions(+), 59 deletions(-) diff --git a/crates/ubrn_cli/src/android.rs b/crates/ubrn_cli/src/android.rs index 2d809975..2b300e19 100644 --- a/crates/ubrn_cli/src/android.rs +++ b/crates/ubrn_cli/src/android.rs @@ -124,16 +124,10 @@ pub(crate) struct AndroidArgs { /// `config.yaml` file. /// /// Android: - /// aarch64-linux-android, - /// armv7-linux-androideabi, - /// x86_64-linux-android - /// i686-linux-android, + /// aarch64-linux-android,armv7-linux-androideabi,x86_64-linux-android,i686-linux-android, /// /// Synonyms for: - /// arm64-v8a, - /// armeabi-v7a, - /// x86_64, - /// x86 + /// arm64-v8a,armeabi-v7a,x86_64,x86 #[clap(short, long, value_parser, num_args = 1.., value_delimiter = ',')] pub(crate) targets: Vec, @@ -148,15 +142,9 @@ pub(crate) struct AndroidArgs { impl AndroidArgs { pub(crate) fn build(&self) -> Result> { let config: ProjectConfig = self.project_config()?; - - let crate_ = &config.crate_; - let android = &config.android; - let target_list = if !self.targets.is_empty() { - &self.targets - } else { - &android.targets - }; + let target_list = &android.targets; + let crate_ = &config.crate_; let target_files = if self.common_args.no_cargo { let files = self.find_existing(&crate_.metadata()?, target_list); if !files.is_empty() { @@ -280,11 +268,12 @@ impl AndroidArgs { } pub(crate) fn project_config(&self) -> Result { - self.config.clone().try_into() - } - - pub(crate) fn config(&self) -> Utf8PathBuf { - self.config.clone() + let mut config: ProjectConfig = self.config.clone().try_into()?; + let android = &mut config.android; + if !self.targets.is_empty() { + android.targets = self.targets.clone(); + } + Ok(config) } } diff --git a/crates/ubrn_cli/src/building.rs b/crates/ubrn_cli/src/building.rs index 7eae50e4..738c3463 100644 --- a/crates/ubrn_cli/src/building.rs +++ b/crates/ubrn_cli/src/building.rs @@ -10,7 +10,10 @@ use clap::{Args, Subcommand}; use serde::Deserialize; use ubrn_common::CrateMetadata; -use crate::{android::AndroidArgs, generate::GenerateAllArgs, ios::IOsArgs}; +use crate::{ + android::AndroidArgs, config::ProjectConfig, generate::GenerateAllCommand, ios::IOsArgs, + Platform, +}; #[derive(Args, Debug)] pub(crate) struct BuildArgs { @@ -37,7 +40,12 @@ impl BuildArgs { } fn generate(&self, lib_file: Utf8PathBuf) -> Result<()> { - GenerateAllArgs::new(lib_file, self.cmd.config()).run() + GenerateAllCommand::platform_specific( + lib_file, + self.cmd.project_config()?, + Platform::from(&self.cmd), + ) + .run() } } @@ -54,10 +62,10 @@ impl BuildCmd { .ok_or_else(|| anyhow!("No targets were specified")) } - fn config(&self) -> Utf8PathBuf { + fn project_config(&self) -> Result { match self { - Self::Android(a) => a.config(), - Self::Ios(a) => a.config(), + Self::Android(a) => a.project_config(), + Self::Ios(a) => a.project_config(), } } @@ -133,3 +141,12 @@ impl From<&[&str]> for ExtraArgs { ExtraArgs::AsList(vec) } } + +impl From<&BuildCmd> for Platform { + fn from(value: &BuildCmd) -> Self { + match value { + BuildCmd::Android(..) => Self::Android, + BuildCmd::Ios(..) => Self::Ios, + } + } +} diff --git a/crates/ubrn_cli/src/codegen/mod.rs b/crates/ubrn_cli/src/codegen/mod.rs index f6116c01..a1fc8376 100644 --- a/crates/ubrn_cli/src/codegen/mod.rs +++ b/crates/ubrn_cli/src/codegen/mod.rs @@ -12,7 +12,7 @@ use std::{collections::BTreeMap, rc::Rc}; use ubrn_bindgen::ModuleMetadata; use ubrn_common::{mk_dir, CrateMetadata}; -use crate::config::ProjectConfig; +use crate::{config::ProjectConfig, Platform}; #[derive(Args, Debug)] pub(crate) struct TurboModuleArgs { @@ -33,7 +33,7 @@ impl TurboModuleArgs { .map(|s| ModuleMetadata::new(s)) .collect(); let rust_crate = project.crate_.metadata()?; - render_files(project, rust_crate, modules)?; + render_files(None, project, rust_crate, modules)?; Ok(()) } } @@ -41,6 +41,8 @@ impl TurboModuleArgs { pub(crate) trait RenderedFile: DynTemplate { fn path(&self, project_root: &Utf8Path) -> Utf8PathBuf; fn project_root(&self) -> Utf8PathBuf { + // This provides a convenience for templates to calculate + // relative paths between one another. Utf8PathBuf::new() } fn relative_to(&self, project_root: &Utf8Path, to: &Utf8Path) -> Utf8PathBuf { @@ -50,6 +52,19 @@ pub(crate) trait RenderedFile: DynTemplate { .expect("Expected this file to have a directory"); pathdiff::diff_utf8_paths(to, from).expect("Should be able to find a relative path") } + fn platform(&self) -> Option { + None + } + fn filter_by(&self, platform: &Option) -> bool { + platforms_match(platform, &self.platform()) + } +} + +fn platforms_match(platform: &Option, this_file_wants: &Option) -> bool { + match (platform, this_file_wants) { + (Some(building_for), Some(this_file)) => building_for == this_file, + (_, _) => true, + } } pub(crate) struct TemplateConfig { @@ -75,12 +90,14 @@ impl TemplateConfig { } pub(crate) fn render_files( + platform: Option, project: ProjectConfig, rust_crate: CrateMetadata, modules: Vec, ) -> Result<()> { let config = Rc::new(TemplateConfig::new(project, rust_crate, modules)); let files = files::get_files(config.clone()); + let files = files.iter().filter(|f| f.filter_by(&platform)).cloned(); let project_root = config.project.project_root(); let map = render_templates(project_root, files)?; @@ -104,10 +121,10 @@ pub(crate) fn render_files( fn render_templates( project_root: &Utf8Path, - files: Vec>, + files: impl Iterator>, ) -> Result> { let mut map = BTreeMap::default(); - for f in &files { + for f in files { let text = f.dyn_render()?; let path = f.path(project_root); map.insert(path, text); @@ -141,6 +158,7 @@ macro_rules! templated_file { mod files { use super::RenderedFile; use super::TemplateConfig; + use crate::Platform; use camino::Utf8Path; use camino::Utf8PathBuf; use std::rc::Rc; @@ -192,6 +210,9 @@ mod files { .codegen_package_dir(project_root) .join(filename) } + fn platform(&self) -> Option { + Some(Platform::Android) + } } templated_file!(BuildGradle, "build.gradle"); @@ -204,6 +225,9 @@ mod files { .directory(project_root) .join(filename) } + fn platform(&self) -> Option { + Some(Platform::Android) + } } templated_file!(CMakeLists, "CMakeLists.txt"); @@ -216,6 +240,9 @@ mod files { .directory(project_root) .join(filename) } + fn platform(&self) -> Option { + Some(Platform::Android) + } } templated_file!(TMHeader, "TurboModuleTemplate.h"); @@ -260,6 +287,9 @@ mod files { .directory(project_root) .join(filename) } + fn platform(&self) -> Option { + Some(Platform::Android) + } } templated_file!(ModuleTemplateH, "ModuleTemplate.h"); @@ -273,6 +303,9 @@ mod files { .directory(project_root) .join(filename) } + fn platform(&self) -> Option { + Some(Platform::Ios) + } } templated_file!(ModuleTemplateMm, "ModuleTemplate.mm"); @@ -286,6 +319,9 @@ mod files { .directory(project_root) .join(filename) } + fn platform(&self) -> Option { + Some(Platform::Ios) + } } templated_file!(PodspecTemplate, "module-template.podspec"); @@ -295,6 +331,9 @@ mod files { let filename = format!("{name}.podspec"); project_root.join(filename) } + fn platform(&self) -> Option { + Some(Platform::Ios) + } } templated_file!(AndroidManifest, "AndroidManifest.xml"); @@ -307,6 +346,9 @@ mod files { .src_main_dir(project_root) .join(filename) } + fn platform(&self) -> Option { + Some(Platform::Android) + } } } @@ -419,4 +461,22 @@ mod tests { assert!(s.contains("list of modules = ['NativeAlice', 'NativeBob', 'NativeCharlie']")); Ok(()) } + + #[test] + fn test_platform_matching() { + let not_specfied: Option = None; + let android_only = Some(Platform::Android); + let ios_only = Some(Platform::Ios); + assert!(platforms_match(¬_specfied, ¬_specfied)); + assert!(platforms_match(¬_specfied, &android_only)); + assert!(platforms_match(¬_specfied, &ios_only)); + + assert!(platforms_match(&android_only, ¬_specfied)); + assert!(platforms_match(&android_only, &android_only)); + assert!(!platforms_match(&android_only, &ios_only)); + + assert!(platforms_match(&ios_only, ¬_specfied)); + assert!(!platforms_match(&ios_only, &android_only)); + assert!(platforms_match(&ios_only, &ios_only)); + } } diff --git a/crates/ubrn_cli/src/codegen/templates/build.gradle b/crates/ubrn_cli/src/codegen/templates/build.gradle index b043ea3f..63ff7619 100644 --- a/crates/ubrn_cli/src/codegen/templates/build.gradle +++ b/crates/ubrn_cli/src/codegen/templates/build.gradle @@ -67,6 +67,13 @@ android { arguments '-DANDROID_STL=c++_shared' } } + ndk { + abiFilters {# space #} + {%- for t in self.config.project.android.targets -%} + "{{ t }}" + {%- if !loop.last %}, {% endif %} + {%- endfor %} + } } externalNativeBuild { diff --git a/crates/ubrn_cli/src/config/mod.rs b/crates/ubrn_cli/src/config/mod.rs index a1dc090c..f3339e47 100644 --- a/crates/ubrn_cli/src/config/mod.rs +++ b/crates/ubrn_cli/src/config/mod.rs @@ -13,7 +13,7 @@ use serde::Deserialize; use crate::{android::AndroidConfig, ios::IOsConfig, rust::CrateConfig, workspace}; -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub(crate) struct ProjectConfig { #[serde(default = "ProjectConfig::default_name")] @@ -111,7 +111,7 @@ impl ProjectConfig { } } -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub(crate) struct BindingsConfig { #[serde(default = "BindingsConfig::default_cpp_dir")] @@ -153,7 +153,7 @@ impl BindingsConfig { } } -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub(crate) struct TurboModulesConfig { #[serde(default = "TurboModulesConfig::default_cpp_dir")] diff --git a/crates/ubrn_cli/src/generate.rs b/crates/ubrn_cli/src/generate.rs index 00a4aa08..efecddd3 100644 --- a/crates/ubrn_cli/src/generate.rs +++ b/crates/ubrn_cli/src/generate.rs @@ -5,10 +5,11 @@ */ use anyhow::Result; use camino::Utf8PathBuf; -use clap::{Args, Subcommand}; +use clap::{self, Args, Subcommand}; +use std::convert::TryFrom; use ubrn_bindgen::{BindingsArgs, OutputArgs, SourceArgs}; -use crate::{codegen::TurboModuleArgs, config::ProjectConfig}; +use crate::{codegen::TurboModuleArgs, config::ProjectConfig, Platform}; #[derive(Args, Debug)] pub(crate) struct GenerateArgs { @@ -47,6 +48,7 @@ impl GenerateCmd { Ok(()) } Self::All(t) => { + let t = GenerateAllCommand::try_from(t)?; t.run()?; Ok(()) } @@ -64,9 +66,36 @@ pub(crate) struct GenerateAllArgs { lib_file: Utf8PathBuf, } -impl GenerateAllArgs { - pub(crate) fn new(lib_file: Utf8PathBuf, config: Utf8PathBuf) -> Self { - Self { lib_file, config } +#[derive(Debug)] +pub(crate) struct GenerateAllCommand { + /// The configuration file for this project + project_config: ProjectConfig, + + /// A path to staticlib file. + lib_file: Utf8PathBuf, + + platform: Option, +} + +impl GenerateAllCommand { + pub(crate) fn new(lib_file: Utf8PathBuf, project_config: ProjectConfig) -> Self { + Self { + lib_file, + project_config, + platform: None, + } + } + + pub(crate) fn platform_specific( + lib_file: Utf8PathBuf, + project_config: ProjectConfig, + platform: Platform, + ) -> Self { + Self { + lib_file, + project_config, + platform: Some(platform), + } } pub(crate) fn run(&self) -> Result<()> { @@ -93,11 +122,23 @@ impl GenerateAllArgs { }; ubrn_common::cd(&pwd)?; let rust_crate = project.crate_.metadata()?; - crate::codegen::render_files(project, rust_crate, modules)?; + crate::codegen::render_files(self.platform.clone(), project, rust_crate, modules)?; Ok(()) } fn project_config(&self) -> Result { - self.config.clone().try_into() + Ok(self.project_config.clone()) + } +} + +impl TryFrom<&GenerateAllArgs> for GenerateAllCommand { + type Error = anyhow::Error; + + fn try_from(value: &GenerateAllArgs) -> Result { + let project_config = value.config.clone().try_into()?; + Ok(GenerateAllCommand::new( + value.lib_file.clone(), + project_config, + )) } } diff --git a/crates/ubrn_cli/src/ios.rs b/crates/ubrn_cli/src/ios.rs index db5be7e1..ca0dfbb1 100644 --- a/crates/ubrn_cli/src/ios.rs +++ b/crates/ubrn_cli/src/ios.rs @@ -109,9 +109,7 @@ pub(crate) struct IOsArgs { /// the `config.yaml` file. /// /// iOS: - /// aarch64-apple-ios, - /// aarch64-apple-ios-sim, - /// x86_64-apple-ios + /// aarch64-apple-ios,aarch64-apple-ios-sim,x86_64-apple-ios #[clap(short, long, value_parser, num_args = 1.., value_delimiter = ',')] pub(crate) targets: Vec, @@ -124,12 +122,7 @@ impl IOsArgs { let config = self.project_config()?; let crate_ = &config.crate_; let ios = &config.ios; - - let target_list = if !self.targets.is_empty() { - &self.targets - } else { - &ios.targets - }; + let target_list = &ios.targets; let targets = target_list .iter() @@ -291,11 +284,12 @@ impl IOsArgs { } pub(crate) fn project_config(&self) -> Result { - self.config.clone().try_into() - } - - pub(crate) fn config(&self) -> Utf8PathBuf { - self.config.clone() + let mut config: ProjectConfig = self.config.clone().try_into()?; + let ios = &mut config.ios; + if !self.targets.is_empty() { + ios.targets = self.targets.clone(); + } + Ok(config) } } diff --git a/crates/ubrn_cli/src/main.rs b/crates/ubrn_cli/src/main.rs index b29d502c..d66bd354 100644 --- a/crates/ubrn_cli/src/main.rs +++ b/crates/ubrn_cli/src/main.rs @@ -50,3 +50,9 @@ impl TryFrom for ProjectConfig { ubrn_common::read_from_file(value) } } + +#[derive(Clone, Debug, PartialEq)] +pub(crate) enum Platform { + Android, + Ios, +} diff --git a/crates/ubrn_cli/src/rust.rs b/crates/ubrn_cli/src/rust.rs index d3858025..3feb6bec 100644 --- a/crates/ubrn_cli/src/rust.rs +++ b/crates/ubrn_cli/src/rust.rs @@ -12,7 +12,7 @@ use ubrn_common::CrateMetadata; use crate::{repo::GitRepoArgs, workspace}; -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] #[serde(rename_all = "camelCase")] pub(crate) struct CrateConfig { #[serde(default = "CrateConfig::default_project_root", skip)] @@ -54,14 +54,14 @@ impl CrateConfig { } } -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] #[serde(untagged)] pub(crate) enum RustSource { OnDisk(OnDiskArgs), GitRepo(GitRepoArgs), } -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] pub(crate) struct OnDiskArgs { #[serde(alias = "rust", alias = "directory")] pub(crate) src: String, diff --git a/crates/ubrn_common/src/files.rs b/crates/ubrn_common/src/files.rs index 335f3ca0..19c92987 100644 --- a/crates/ubrn_common/src/files.rs +++ b/crates/ubrn_common/src/files.rs @@ -92,6 +92,9 @@ where for<'a> T: Deserialize<'a>, { let file = file.as_ref(); + if !file.exists() { + anyhow::bail!("File {file} does not exist"); + } let s = std::fs::read_to_string(file).with_context(|| format!("Failed to read from {file:?}"))?; Ok(if is_yaml(file) { diff --git a/docs/src/getting-started/guide.md b/docs/src/getting-started/guide.md index 5a52d0bb..d4eeaca2 100644 --- a/docs/src/getting-started/guide.md +++ b/docs/src/getting-started/guide.md @@ -191,6 +191,14 @@ Building for Android will: yarn ubrn:android ``` +```admonish hint +You can change the targets that get built by adding a comma separated list to the `ubrn build android` and `ubrn build ios` commands. +``` + +```sh +yarn ubrn:android --targets aarch64-linux-android,armv7-linux-androideabi +``` + ## Step 5: Write an example app exercising the Rust API Here, we're editing the app file at `example/src/App.tsx`. diff --git a/docs/src/reference/commandline.md b/docs/src/reference/commandline.md index 9eb0b1c7..98b4b7a7 100644 --- a/docs/src/reference/commandline.md +++ b/docs/src/reference/commandline.md @@ -101,7 +101,9 @@ Options: `--release` sets the release profile for `cargo`. -`--and-generate` is a convenience option to pass the built library file to `generate bindings` and `generate turbo-module`. +`--and-generate` is a convenience option to pass the built library file to `generate bindings` and `generate turbo-module` for Android and common files. + +This is useful as some generated files use the targets specified in this command. Once the library files (one for each target) are created, they are copied into the `jniLibs` specified by the YAML configuration. @@ -174,7 +176,9 @@ The configuration file refers to [the YAML configuration][config]. `--sim-only` and `--no-sim` restricts the targets to targets with/without `sim` in the target triple. -`--and-generate` is a convenience option to pass the built library file to `generate bindings` and `generate turbo-module`. +`--and-generate` is a convenience option to pass the built library file to `generate bindings` and `generate turbo-module` for iOS and common files. + +This is useful as some generated files use the targets specified in this command. Once the target libraries are compiled, and a config file is specified, they are passed to `xcodebuild -create-xcframework` to generate an `xcframework`.