cargo-apk: Inherit package configuration (version) from workspace root

cargo-apk/CHANGELOG.md

@@ -2,6 +2,7 @@
 
 - Profile signing information can now be specified via the `CARGO_APK_<PROFILE>_KEYSTORE` and `CARGO_APK_<PROFILE>_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`.)
 

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"

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<String>,
     ) -> Result<Self, Error> {
+        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<Root> = 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");

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 {

cargo-apk/src/manifest.rs

@@ -8,8 +8,15 @@ use std::{
     path::{Path, PathBuf},
 };
 
+#[derive(Debug, Clone, Deserialize)]
+#[serde(untagged)]
+pub enum Inheritable<T> {
+    Value(T),
+    Inherited { workspace: bool },
+}
+
 pub(crate) struct Manifest {
-    pub(crate) version: String,
+    pub(crate) version: Inheritable<String>,
     pub(crate) apk_name: Option<String>,
     pub(crate) android_manifest: AndroidManifest,
     pub(crate) build_targets: Vec<Target>,
@@ -24,16 +31,19 @@ pub(crate) struct Manifest {
 
 impl Manifest {
     pub(crate) fn parse_from_toml(path: &Path) -> Result<Self, Error> {
-        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<Package>,
+    pub(crate) workspace: Option<Workspace>,
+}
+
+impl Root {
+    pub(crate) fn parse_from_toml(path: &Path) -> Result<Self, Error> {
+        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<PackageMetadata>,
+pub(crate) struct Package {
+    pub(crate) version: Inheritable<String>,
+    pub(crate) metadata: Option<PackageMetadata>,
+}
+
+#[derive(Clone, Debug, Deserialize)]
+pub(crate) struct Workspace {
+    pub(crate) package: Option<WorkspacePackage>,
+}
+
+/// 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<String>,
 }
 
 #[derive(Clone, Debug, Default, Deserialize)]
-struct PackageMetadata {
+pub(crate) struct PackageMetadata {
     android: Option<AndroidMetadata>,
 }