Skip to content

Commit

Permalink
Build for selected Android targets (#119)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jhugman authored Oct 10, 2024
1 parent 4cda9a3 commit 7afb49b
Show file tree
Hide file tree
Showing 12 changed files with 188 additions and 59 deletions.
31 changes: 10 additions & 21 deletions crates/ubrn_cli/src/android.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Target>,

Expand All @@ -148,15 +142,9 @@ pub(crate) struct AndroidArgs {
impl AndroidArgs {
pub(crate) fn build(&self) -> Result<Vec<Utf8PathBuf>> {
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() {
Expand Down Expand Up @@ -280,11 +268,12 @@ impl AndroidArgs {
}

pub(crate) fn project_config(&self) -> Result<ProjectConfig> {
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)
}
}

Expand Down
27 changes: 22 additions & 5 deletions crates/ubrn_cli/src/building.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()
}
}

Expand All @@ -54,10 +62,10 @@ impl BuildCmd {
.ok_or_else(|| anyhow!("No targets were specified"))
}

fn config(&self) -> Utf8PathBuf {
fn project_config(&self) -> Result<ProjectConfig> {
match self {
Self::Android(a) => a.config(),
Self::Ios(a) => a.config(),
Self::Android(a) => a.project_config(),
Self::Ios(a) => a.project_config(),
}
}

Expand Down Expand Up @@ -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,
}
}
}
68 changes: 64 additions & 4 deletions crates/ubrn_cli/src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -33,14 +33,16 @@ 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(())
}
}

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 {
Expand All @@ -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<Platform> {
None
}
fn filter_by(&self, platform: &Option<Platform>) -> bool {
platforms_match(platform, &self.platform())
}
}

fn platforms_match(platform: &Option<Platform>, this_file_wants: &Option<Platform>) -> bool {
match (platform, this_file_wants) {
(Some(building_for), Some(this_file)) => building_for == this_file,
(_, _) => true,
}
}

pub(crate) struct TemplateConfig {
Expand All @@ -75,12 +90,14 @@ impl TemplateConfig {
}

pub(crate) fn render_files(
platform: Option<Platform>,
project: ProjectConfig,
rust_crate: CrateMetadata,
modules: Vec<ModuleMetadata>,
) -> 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)?;
Expand All @@ -104,10 +121,10 @@ pub(crate) fn render_files(

fn render_templates(
project_root: &Utf8Path,
files: Vec<Rc<dyn RenderedFile>>,
files: impl Iterator<Item = Rc<dyn RenderedFile>>,
) -> Result<BTreeMap<Utf8PathBuf, String>> {
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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -192,6 +210,9 @@ mod files {
.codegen_package_dir(project_root)
.join(filename)
}
fn platform(&self) -> Option<Platform> {
Some(Platform::Android)
}
}

templated_file!(BuildGradle, "build.gradle");
Expand All @@ -204,6 +225,9 @@ mod files {
.directory(project_root)
.join(filename)
}
fn platform(&self) -> Option<Platform> {
Some(Platform::Android)
}
}

templated_file!(CMakeLists, "CMakeLists.txt");
Expand All @@ -216,6 +240,9 @@ mod files {
.directory(project_root)
.join(filename)
}
fn platform(&self) -> Option<Platform> {
Some(Platform::Android)
}
}

templated_file!(TMHeader, "TurboModuleTemplate.h");
Expand Down Expand Up @@ -260,6 +287,9 @@ mod files {
.directory(project_root)
.join(filename)
}
fn platform(&self) -> Option<Platform> {
Some(Platform::Android)
}
}

templated_file!(ModuleTemplateH, "ModuleTemplate.h");
Expand All @@ -273,6 +303,9 @@ mod files {
.directory(project_root)
.join(filename)
}
fn platform(&self) -> Option<Platform> {
Some(Platform::Ios)
}
}

templated_file!(ModuleTemplateMm, "ModuleTemplate.mm");
Expand All @@ -286,6 +319,9 @@ mod files {
.directory(project_root)
.join(filename)
}
fn platform(&self) -> Option<Platform> {
Some(Platform::Ios)
}
}

templated_file!(PodspecTemplate, "module-template.podspec");
Expand All @@ -295,6 +331,9 @@ mod files {
let filename = format!("{name}.podspec");
project_root.join(filename)
}
fn platform(&self) -> Option<Platform> {
Some(Platform::Ios)
}
}

templated_file!(AndroidManifest, "AndroidManifest.xml");
Expand All @@ -307,6 +346,9 @@ mod files {
.src_main_dir(project_root)
.join(filename)
}
fn platform(&self) -> Option<Platform> {
Some(Platform::Android)
}
}
}

Expand Down Expand Up @@ -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<Platform> = None;
let android_only = Some(Platform::Android);
let ios_only = Some(Platform::Ios);
assert!(platforms_match(&not_specfied, &not_specfied));
assert!(platforms_match(&not_specfied, &android_only));
assert!(platforms_match(&not_specfied, &ios_only));

assert!(platforms_match(&android_only, &not_specfied));
assert!(platforms_match(&android_only, &android_only));
assert!(!platforms_match(&android_only, &ios_only));

assert!(platforms_match(&ios_only, &not_specfied));
assert!(!platforms_match(&ios_only, &android_only));
assert!(platforms_match(&ios_only, &ios_only));
}
}
7 changes: 7 additions & 0 deletions crates/ubrn_cli/src/codegen/templates/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions crates/ubrn_cli/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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")]
Expand Down Expand Up @@ -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")]
Expand Down
Loading

0 comments on commit 7afb49b

Please sign in to comment.