From c20ac244ff530511401d8fc1ff970856a20db6bd Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Thu, 24 Mar 2022 15:08:15 +0100 Subject: [PATCH] Support `autobins` and target renaming `[lib]` and `[[bin]]` --- Cargo.toml | 2 +- README.md | 6 +- src/args.rs | 9 +++ src/artifact.rs | 70 +++++++++-------- src/error.rs | 6 ++ src/lib.rs | 3 +- src/manifest.rs | 48 ++++++++++++ src/subcommand.rs | 188 ++++++++++++++++++++++++++++++++++++++++------ src/utils.rs | 7 +- 9 files changed, 277 insertions(+), 62 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 2884c36..904fa17 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -2,7 +2,7 @@ name = "cargo-subcommand" version = "0.11.0" authors = ["David Craven ", "Marijn Suijten "] -edition = "2018" +edition = "2021" description = "Library for creating cargo subcommands." repository = "https://github.com/dvc94ch/cargo-subcommand" license = "ISC" diff --git a/README.md b/README.md index 6193f8d..bdf05f9 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,9 @@ -# cargo-subcommand library for building subcommands -Is used by `cargo-apk` and `cargo-flutter`. +# `cargo-subcommand` library for building subcommands + +Is used by [`cargo-apk`](https://crates.io/crates/cargo-apk) and `cargo-flutter`. # License + Copyright 2020 David Craven Permission is hereby granted, free of charge, to any person obtaining a copy of diff --git a/src/args.rs b/src/args.rs index 009720a..ba70325 100644 --- a/src/args.rs +++ b/src/args.rs @@ -130,4 +130,13 @@ impl Args { Profile::Dev } } + + /// Returns [`true`] when one or more target selection options are active. + /// This is generally used to deduce whether to default to binary and + /// library targets, in accordance with [`cargo build`]. + /// + /// [`cargo build`]: https://doc.rust-lang.org/cargo/commands/cargo-build.html#target-selection + pub fn specific_target_selected(&self) -> bool { + self.lib || self.bins || self.examples || !self.bin.is_empty() || !self.example.is_empty() + } } diff --git a/src/artifact.rs b/src/artifact.rs index 9e4c692..65679ec 100644 --- a/src/artifact.rs +++ b/src/artifact.rs @@ -1,50 +1,54 @@ -use std::path::Path; +use std::path::{Path, PathBuf}; -#[derive(Clone, Debug, Eq, Hash, PartialEq)] -pub enum Artifact { - Root(String), - Example(String), +use crate::manifest::CrateType; + +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] +pub enum ArtifactType { + Lib, + Bin, + Example, + // Bench, + // Test, } -impl AsRef for Artifact { - fn as_ref(&self) -> &Path { - Path::new(match self { - Self::Root(_) => "", - Self::Example(_) => "examples", - }) - } +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +pub struct Artifact { + pub name: String, + pub path: PathBuf, + pub r#type: ArtifactType, } impl Artifact { - pub fn name(&self) -> &str { - match self { - Self::Root(name) => name, - Self::Example(name) => name, - } + pub fn build_dir(&self) -> &'static Path { + Path::new(match self.r#type { + ArtifactType::Lib | ArtifactType::Bin => "", + ArtifactType::Example => "examples", + }) } + // TODO: CrateType should be read from the manifest' crate-type array, + // and validated that the requested format is in that array pub fn file_name(&self, ty: CrateType, target: &str) -> String { - match ty { - CrateType::Bin => { + match (self.r#type, ty) { + (ArtifactType::Bin | ArtifactType::Example, CrateType::Bin) => { if target.contains("windows") { - format!("{}.exe", self.name()) + format!("{}.exe", self.name) } else if target.contains("wasm") { - format!("{}.wasm", self.name()) + format!("{}.wasm", self.name) } else { - self.name().to_string() + self.name.to_string() } } - CrateType::Lib => format!("lib{}.rlib", self.name().replace('-', "_")), - CrateType::Staticlib => format!("lib{}.a", self.name().replace('-', "_")), - CrateType::Cdylib => format!("lib{}.so", self.name().replace('-', "_")), + (ArtifactType::Lib | ArtifactType::Example, CrateType::Lib) => { + format!("lib{}.rlib", self.name.replace('-', "_")) + } + (ArtifactType::Lib | ArtifactType::Example, CrateType::Staticlib) => { + format!("lib{}.a", self.name.replace('-', "_")) + } + (ArtifactType::Lib | ArtifactType::Example, CrateType::Cdylib) => { + format!("lib{}.so", self.name.replace('-', "_")) + } + (a, c) => panic!("{a:?} is not compatible with {c:?}"), } } } - -#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] -pub enum CrateType { - Bin, - Lib, - Staticlib, - Cdylib, -} diff --git a/src/error.rs b/src/error.rs index 809e1b4..de76907 100644 --- a/src/error.rs +++ b/src/error.rs @@ -22,6 +22,9 @@ pub enum Error { }, Io(PathBuf, IoError), Toml(PathBuf, TomlError), + BinNotFound(String), + DuplicateBin(String), + DuplicateExample(String), } pub type Result = std::result::Result; @@ -74,6 +77,9 @@ Alternatively, to keep it out of the workspace, add an empty `[workspace]` table }, Self::Io(path, error) => return write!(f, "{}: {}", path.display(), error), Self::Toml(file, error) => return write!(f, "{}: {}", file.display(), error), + Self::BinNotFound(name) => return write!(f, "Can't find `{name}` bin at `src/bin/{name}.rs` or `src/bin/{name}/main.rs`. Please specify bin.path if you want to use a non-default path.", name = name), + Self::DuplicateBin(name) => return write!(f, "found duplicate binary name {name}, but all binary targets must have a unique name"), + Self::DuplicateExample(name) => return write!(f, "found duplicate example name {name}, but all example targets must have a unique name"), }) } } diff --git a/src/lib.rs b/src/lib.rs index b4f0681..b50f122 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -8,8 +8,9 @@ mod subcommand; mod utils; pub use args::Args; -pub use artifact::{Artifact, CrateType}; +pub use artifact::{Artifact, ArtifactType}; pub use config::{EnvError, EnvOption, LocalizedConfig}; pub use error::Error; +pub use manifest::CrateType; pub use profile::Profile; pub use subcommand::Subcommand; diff --git a/src/manifest.rs b/src/manifest.rs index f25aec7..5c15f5f 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -11,6 +11,11 @@ use crate::utils; pub struct Manifest { pub workspace: Option, pub package: Option, + pub lib: Option, + #[serde(default, rename = "bin")] + pub bins: Vec, + #[serde(default, rename = "example")] + pub examples: Vec, } impl Manifest { @@ -90,7 +95,50 @@ pub struct Workspace { pub members: Vec, } +const fn default_true() -> bool { + true +} + #[derive(Clone, Debug, Deserialize)] pub struct Package { pub name: String, + + // https://doc.rust-lang.org/cargo/reference/cargo-targets.html#target-auto-discovery + #[serde(default = "default_true")] + pub autobins: bool, + #[serde(default = "default_true")] + pub autoexamples: bool, + // #[serde(default = "default_true")] + // pub autotests: bool, + // #[serde(default = "default_true")] + // pub autobenches: bool, +} + +#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, Deserialize)] +pub enum CrateType { + Bin, + Lib, + Staticlib, + Cdylib, +} + +#[derive(Clone, Debug, Deserialize)] +pub struct Lib { + pub name: Option, + pub path: Option, + // pub crate_type: Vec, +} + +#[derive(Clone, Debug, Deserialize)] +pub struct Bin { + pub name: String, + pub path: Option, + // pub crate_type: Vec, +} + +#[derive(Clone, Debug, Deserialize)] +pub struct Example { + pub name: String, + pub path: Option, + // pub crate_type: Vec, } diff --git a/src/subcommand.rs b/src/subcommand.rs index 3d9638d..88859c1 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -1,8 +1,10 @@ use crate::args::Args; -use crate::artifact::{Artifact, CrateType}; +use crate::artifact::{Artifact, ArtifactType}; use crate::error::{Error, Result}; +use crate::manifest::Manifest; use crate::profile::Profile; -use crate::{utils, LocalizedConfig}; +use crate::{utils, CrateType, LocalizedConfig}; +use std::collections::HashMap; use std::ffi::OsStr; use std::path::{Path, PathBuf}; @@ -15,7 +17,9 @@ pub struct Subcommand { target_dir: PathBuf, host_triple: String, profile: Profile, - artifacts: Vec, + lib_artifact: Option, + bin_artifacts: Vec, + example_artifacts: Vec, config: Option, } @@ -83,6 +87,8 @@ impl Subcommand { config.set_env_vars().unwrap(); } + let parsed_manifest = Manifest::parse_from_toml(&manifest_path)?; + let target_dir = args .target_dir .clone() @@ -109,28 +115,161 @@ impl Subcommand { .join(utils::get_target_dir_name(config.as_deref()).unwrap()) }); - let mut artifacts = vec![]; - if args.examples { - for file in utils::list_rust_files(&root_dir.join("examples"))? { - artifacts.push(Artifact::Example(file)); + // https://doc.rust-lang.org/cargo/reference/cargo-targets.html#target-auto-discovery + + let main_bin_path = Path::new("src/main.rs"); + let main_lib_path = Path::new("src/lib.rs"); + + let mut bin_artifacts = HashMap::new(); + let mut example_artifacts = HashMap::new(); + + fn find_main_file(dir: &Path, name: &str) -> Option { + let alt_path = dir.join(format!("{}.rs", name)); + alt_path.is_file().then_some(alt_path).or_else(|| { + let alt_path = dir.join(name).join("main.rs"); + alt_path.is_file().then_some(alt_path) + }) + } + + // Add all explicitly configured binaries + for bin in &parsed_manifest.bins { + let path = bin + .path + .clone() + .or_else(|| find_main_file(&root_dir.join("src/bin"), &bin.name)) + .ok_or_else(|| Error::BinNotFound(bin.name.clone()))?; + + let prev = bin_artifacts.insert( + bin.name.clone(), + Artifact { + name: bin.name.clone(), + path, + r#type: ArtifactType::Bin, + }, + ); + if prev.is_some() { + return Err(Error::DuplicateBin(bin.name.clone())); } - } else { - for example in &args.example { - artifacts.push(Artifact::Example(example.into())); + } + + // Add all explicitly configured examples + for example in &parsed_manifest.examples { + let path = example + .path + .clone() + .or_else(|| find_main_file(&root_dir.join("examples"), &example.name)) + .ok_or_else(|| Error::BinNotFound(example.name.clone()))?; + + let prev = example_artifacts.insert( + example.name.clone(), + Artifact { + name: example.name.clone(), + path, + r#type: ArtifactType::Example, + }, + ); + if prev.is_some() { + return Err(Error::DuplicateExample(example.name.clone())); + } + } + + /// Name is typically the [`Path::file_stem()`], except for `src/main.rs` where it is the package name + fn insert_if_unconfigured( + name: Option, + path: &Path, + r#type: ArtifactType, + artifacts: &mut HashMap, + ) { + // Only insert the detected binary if there isn't another artifact already configuring this file path + if artifacts.values().any(|bin| bin.path == path) { + println!("Already configuring {path:?}"); + return; } + + let name = + name.unwrap_or_else(|| path.file_stem().unwrap().to_str().unwrap().to_owned()); + + // Only insert the detected binary if an artifact with the same name wasn't yet configured + artifacts.entry(name.clone()).or_insert(Artifact { + name, + path: path.to_owned(), + r#type, + }); } - if args.bins { + + // Parse all autobins + if parsed_manifest + .package + .as_ref() + .map_or(true, |p| p.autobins) + { + // Special-case for the main binary of a package + if root_dir.join(main_bin_path).is_file() { + insert_if_unconfigured( + Some(package.clone()), + main_bin_path, + ArtifactType::Bin, + &mut bin_artifacts, + ); + } + for file in utils::list_rust_files(&root_dir.join("src").join("bin"))? { - artifacts.push(Artifact::Root(file)); + let file = file.strip_prefix(root_dir).unwrap(); + + insert_if_unconfigured(None, file, ArtifactType::Bin, &mut bin_artifacts); } - } else { - for bin in &args.bin { - artifacts.push(Artifact::Root(bin.into())); + } + + // Parse all autoexamples + if parsed_manifest + .package + .as_ref() + .map_or(true, |p| p.autoexamples) + { + for file in utils::list_rust_files(&root_dir.join("examples"))? { + let file = file.strip_prefix(root_dir).unwrap(); + + insert_if_unconfigured(None, file, ArtifactType::Example, &mut example_artifacts); } } - if artifacts.is_empty() { - artifacts.push(Artifact::Root(package.clone())); + + let mut lib_artifact = parsed_manifest + .lib + .as_ref() + .map(|lib| Artifact { + // The library is either configured with sensible defaults + name: lib.name.as_ref().unwrap_or(package).clone(), + path: lib.path.as_deref().unwrap_or(main_lib_path).to_owned(), + r#type: ArtifactType::Lib, + }) + .or_else(|| { + // Or autodetected with the same defaults, if that default path exists + root_dir.join(main_lib_path).is_file().then(|| Artifact { + name: package.clone(), + path: main_lib_path.to_owned(), + r#type: ArtifactType::Lib, + }) + }); + + // Filtering based on arguments + // https://doc.rust-lang.org/cargo/reference/cargo-targets.html#binaries + + let specific_target_selected = args.specific_target_selected(); + + if specific_target_selected { + if !args.lib { + lib_artifact = None; + } + + if !args.bins { + bin_artifacts.retain(|a, _| args.bin.contains(a)); + } + + if !args.examples { + example_artifacts.retain(|a, _| args.example.contains(a)); + } } + let host_triple = current_platform::CURRENT_PLATFORM.to_owned(); let profile = args.profile(); Ok(Self { @@ -141,7 +280,9 @@ impl Subcommand { target_dir, host_triple, profile, - artifacts, + lib_artifact, + bin_artifacts: bin_artifacts.into_values().collect(), + example_artifacts: example_artifacts.into_values().collect(), config, }) } @@ -170,8 +311,11 @@ impl Subcommand { &self.profile } - pub fn artifacts(&self) -> &[Artifact] { - &self.artifacts + pub fn artifacts(&self) -> impl Iterator { + self.lib_artifact + .iter() + .chain(&self.bin_artifacts) + .chain(&self.example_artifacts) } pub fn target_dir(&self) -> &Path { @@ -208,6 +352,8 @@ impl Subcommand { ) -> PathBuf { let triple = target.unwrap_or_else(|| self.host_triple()); let file_name = artifact.file_name(crate_type, triple); - self.build_dir(target).join(artifact).join(file_name) + self.build_dir(target) + .join(artifact.build_dir()) + .join(file_name) } } diff --git a/src/utils.rs b/src/utils.rs index adc5a18..97eadd0 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -4,15 +4,14 @@ use crate::manifest::Manifest; use std::ffi::OsStr; use std::path::{Path, PathBuf}; -pub fn list_rust_files(dir: &Path) -> Result> { - let mut files = Vec::new(); +pub fn list_rust_files(dir: &Path) -> Result> { + let mut files = vec![]; if dir.exists() && dir.is_dir() { let entries = std::fs::read_dir(dir).map_err(|e| Error::Io(dir.to_owned(), e))?; for entry in entries { let path = entry.map_err(|e| Error::Io(dir.to_owned(), e))?.path(); if path.is_file() && path.extension() == Some(OsStr::new("rs")) { - let name = path.file_stem().unwrap().to_str().unwrap(); - files.push(name.to_string()); + files.push(path); } } }