From 4d9101a048a240edb16d8b0a4b9b60b4e6782f79 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Sat, 5 Nov 2022 18:34:13 +0100 Subject: [PATCH] cargo-apk: Inherit package configuration (`version`) from workspace root Starting with [Rust 1.64] common `[package]` parameters (and dependencies) can now be specified in the workspace root manifest by setting `.workspace=true` in a `[package]` and specifying its value in the workspace root manifest under [`[workspace.package]`]. Since `cargo-apk` reads the `version` field from `[package]` it has to support this new format and pull the value from the root manifest instead, in order to support projects utilizing this new format. This is currently implemented ad-hoc for the version field, but could be done in a cleaner way if/when more fields are needed. [Rust 1.64]: https://blog.rust-lang.org/2022/09/22/Rust-1.64.0.html#cargo-improvements-workspace-inheritance-and-multi-target-builds [`[workspace.package]`]: https://doc.rust-lang.org/cargo/reference/workspaces.html#the-workspacepackage-table --- cargo-apk/CHANGELOG.md | 1 + cargo-apk/Cargo.toml | 2 +- cargo-apk/src/apk.rs | 40 +++++++++++++++++++++++++++--- cargo-apk/src/error.rs | 6 +++++ cargo-apk/src/manifest.rs | 52 ++++++++++++++++++++++++++++++--------- 5 files changed, 86 insertions(+), 15 deletions(-) diff --git a/cargo-apk/CHANGELOG.md b/cargo-apk/CHANGELOG.md index 8be438c6..8ec4cd1d 100644 --- a/cargo-apk/CHANGELOG.md +++ b/cargo-apk/CHANGELOG.md @@ -2,6 +2,7 @@ - Profile signing information can now be specified via the `CARGO_APK__KEYSTORE` and `CARGO_APK__KEYSTORE_PASSWORD` environment variables. The environment variables take precedence over signing information in the cargo manifest. Both environment variables are required except in the case of the `dev` profile, which will fall back to the default password if `CARGO_APK_DEV_KEYSTORE_PASSWORD` is not set. ([#358](https://github.com/rust-windowing/android-ndk-rs/pull/358)) - Add `strip` option to `android` metadata, allowing a user to specify how they want debug symbols treated after cargo has finished building, but before the shared object is copied into the APK. ([#356](https://github.com/rust-windowing/android-ndk-rs/pull/356)) +- Support [`[workspace.package]` inheritance](https://doc.rust-lang.org/cargo/reference/workspaces.html#the-workspacepackage-table) from a workspace root manifest for the `version` field under `[package]`. ([#360](https://github.com/rust-windowing/android-ndk-rs/pull/360)) (0.9.5, released on 2022-10-14, was yanked due to unintentionally bumping MSRV through the `quick-xml` crate, and breaking `cargo apk --` parsing after switching to `clap`.) diff --git a/cargo-apk/Cargo.toml b/cargo-apk/Cargo.toml index 90689cde..737800f1 100644 --- a/cargo-apk/Cargo.toml +++ b/cargo-apk/Cargo.toml @@ -13,7 +13,7 @@ rust-version = "1.60" [dependencies] anyhow = "1.0.57" -cargo-subcommand = "0.9" +cargo-subcommand = "0.10" clap = { version = "4", features = ["derive"] } dunce = "1" env_logger = "0.9" diff --git a/cargo-apk/src/apk.rs b/cargo-apk/src/apk.rs index ceb7cb65..a807135f 100644 --- a/cargo-apk/src/apk.rs +++ b/cargo-apk/src/apk.rs @@ -1,5 +1,5 @@ use crate::error::Error; -use crate::manifest::Manifest; +use crate::manifest::{Inheritable, Manifest, Root}; use cargo_subcommand::{Artifact, CrateType, Profile, Subcommand}; use ndk_build::apk::{Apk, ApkConfig}; use ndk_build::cargo::{cargo_ndk, VersionCode}; @@ -24,8 +24,17 @@ impl<'a> ApkBuilder<'a> { cmd: &'a Subcommand, device_serial: Option, ) -> Result { + println!( + "Using package `{}` in `{}`", + cmd.package(), + cmd.manifest().display() + ); let ndk = Ndk::from_env()?; let mut manifest = Manifest::parse_from_toml(cmd.manifest())?; + let workspace_manifest: Option = cmd + .workspace_manifest() + .map(Root::parse_from_toml) + .transpose()?; let build_targets = if let Some(target) = cmd.target() { vec![Target::from_rust_triple(target)?] } else if !manifest.build_targets.is_empty() { @@ -39,11 +48,36 @@ impl<'a> ApkBuilder<'a> { .join(cmd.profile()) .join("apk"); + let package_version = match &manifest.version { + Inheritable::Value(v) => v.clone(), + Inheritable::Inherited { workspace: true } => { + let workspace = workspace_manifest + .ok_or(Error::InheritanceMissingWorkspace)? + .workspace + .unwrap_or_else(|| { + // Unlikely to fail as cargo-subcommand should give us + // a `Cargo.toml` containing a `[workspace]` table + panic!( + "Manifest `{:?}` must contain a `[workspace]` table", + cmd.workspace_manifest().unwrap() + ) + }); + + workspace + .package + .ok_or(Error::WorkspaceMissingInheritedField("package"))? + .version + .ok_or(Error::WorkspaceMissingInheritedField("package.version"))? + } + Inheritable::Inherited { workspace: false } => return Err(Error::InheritedFalse), + }; + let version_code = VersionCode::from_semver(&package_version)?.to_code(1); + // Set default Android manifest values if manifest .android_manifest .version_name - .replace(manifest.version.clone()) + .replace(package_version) .is_some() { panic!("version_name should not be set in TOML"); @@ -52,7 +86,7 @@ impl<'a> ApkBuilder<'a> { if manifest .android_manifest .version_code - .replace(VersionCode::from_semver(&manifest.version)?.to_code(1)) + .replace(version_code) .is_some() { panic!("version_code should not be set in TOML"); diff --git a/cargo-apk/src/error.rs b/cargo-apk/src/error.rs index 8dc864f9..95228655 100644 --- a/cargo-apk/src/error.rs +++ b/cargo-apk/src/error.rs @@ -16,6 +16,12 @@ pub enum Error { Io(#[from] IoError), #[error("Configure a release keystore via `[package.metadata.android.signing.{0}]`")] MissingReleaseKey(String), + #[error("`workspace=false` is unsupported")] + InheritedFalse, + #[error("`workspace=true` requires a workspace")] + InheritanceMissingWorkspace, + #[error("Failed to inherit field: `workspace.{0}` was not defined in workspace root manifest")] + WorkspaceMissingInheritedField(&'static str), } impl Error { diff --git a/cargo-apk/src/manifest.rs b/cargo-apk/src/manifest.rs index 4e561a78..de4460a5 100644 --- a/cargo-apk/src/manifest.rs +++ b/cargo-apk/src/manifest.rs @@ -8,8 +8,15 @@ use std::{ path::{Path, PathBuf}, }; +#[derive(Debug, Clone, Deserialize)] +#[serde(untagged)] +pub enum Inheritable { + Value(T), + Inherited { workspace: bool }, +} + pub(crate) struct Manifest { - pub(crate) version: String, + pub(crate) version: Inheritable, pub(crate) apk_name: Option, pub(crate) android_manifest: AndroidManifest, pub(crate) build_targets: Vec, @@ -24,16 +31,19 @@ pub(crate) struct Manifest { impl Manifest { pub(crate) fn parse_from_toml(path: &Path) -> Result { - let contents = std::fs::read_to_string(path)?; - let toml: Root = toml::from_str(&contents)?; - let metadata = toml + let toml = Root::parse_from_toml(path)?; + // Unlikely to fail as cargo-subcommand should give us a `Cargo.toml` containing + // a `[package]` table (with a matching `name` when requested by the user) + let package = toml .package + .unwrap_or_else(|| panic!("Manifest `{:?}` must contain a `[package]`", path)); + let metadata = package .metadata .unwrap_or_default() .android .unwrap_or_default(); Ok(Self { - version: toml.package.version, + version: package.version, apk_name: metadata.apk_name, android_manifest: metadata.android_manifest, build_targets: metadata.build_targets, @@ -48,18 +58,38 @@ impl Manifest { } #[derive(Debug, Clone, Deserialize)] -struct Root { - package: Package, +pub(crate) struct Root { + pub(crate) package: Option, + pub(crate) workspace: Option, +} + +impl Root { + pub(crate) fn parse_from_toml(path: &Path) -> Result { + let contents = std::fs::read_to_string(path)?; + toml::from_str(&contents).map_err(|e| e.into()) + } } #[derive(Debug, Clone, Deserialize)] -struct Package { - version: String, - metadata: Option, +pub(crate) struct Package { + pub(crate) version: Inheritable, + pub(crate) metadata: Option, +} + +#[derive(Clone, Debug, Deserialize)] +pub(crate) struct Workspace { + pub(crate) package: Option, +} + +/// Almost the same as [`Package`], except that this must provide +/// root values instead of possibly inheritable values +#[derive(Clone, Debug, Deserialize)] +pub(crate) struct WorkspacePackage { + pub(crate) version: Option, } #[derive(Clone, Debug, Default, Deserialize)] -struct PackageMetadata { +pub(crate) struct PackageMetadata { android: Option, }