Skip to content

Commit

Permalink
cargo-apk: Inherit package configuration (version) from workspace root
Browse files Browse the repository at this point in the history
Starting with [Rust 1.64] common `[package]` parameters (and
dependencies) can now be specified in the workspace root manifest by
setting `<field>.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
  • Loading branch information
MarijnS95 committed Nov 23, 2022
1 parent 419df14 commit 4d9101a
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 15 deletions.
1 change: 1 addition & 0 deletions cargo-apk/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.)

Expand Down
2 changes: 1 addition & 1 deletion cargo-apk/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
40 changes: 37 additions & 3 deletions cargo-apk/src/apk.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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() {
Expand All @@ -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");
Expand All @@ -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");
Expand Down
6 changes: 6 additions & 0 deletions cargo-apk/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
52 changes: 41 additions & 11 deletions cargo-apk/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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,
Expand All @@ -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>,
}

Expand Down

0 comments on commit 4d9101a

Please sign in to comment.