Skip to content

Commit bf24eb6

Browse files
authored
fix: cargo build/install -p resolves target to shader crate (#122)
* added logging * remove -p as alias for --shader-crate * add --package and -p as its own clap option * resolve package to shader crate * cache cargo metadata * fix or allow clippy lints
1 parent eb5f8db commit bf24eb6

File tree

7 files changed

+136
-24
lines changed

7 files changed

+136
-24
lines changed

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,4 @@ partial_pub_fields = { level = "allow", priority = 1 }
5959
pattern_type_mismatch = { level = "allow", priority = 1 }
6060
std_instead_of_alloc = { level = "allow", priority = 1 }
6161
arbitrary_source_item_ordering = { level = "allow", priority = 1 }
62+
missing_inline_in_public_items = { level = "allow", priority = 1 }

crates/cargo-gpu/src/build.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ impl Build {
5959
/// Entrypoint
6060
pub fn run(&mut self) -> anyhow::Result<()> {
6161
let installed_backend = self.install.run()?;
62+
let mut metadata = crate::metadata::MetadataCache::default();
63+
64+
if let Some(package) = self.install.package.as_ref() {
65+
self.install.shader_crate = metadata.resolve_package_to_shader_crate(package)?;
66+
}
6267

6368
let _lockfile_mismatch_handler = LockfileMismatchHandler::new(
6469
&self.install.shader_crate,

crates/cargo-gpu/src/config.rs

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
use anyhow::Context as _;
33
use clap::Parser as _;
44

5+
use crate::metadata::MetadataCache;
6+
57
/// Config
68
pub struct Config;
79

@@ -23,10 +25,11 @@ impl Config {
2325
/// `Cargo.toml`, so here we load that config first as the base config, and the CLI arguments can
2426
/// then later override it.
2527
pub fn clap_command_with_cargo_config(
26-
shader_crate_path: &std::path::PathBuf,
28+
shader_crate_path: &std::path::Path,
2729
mut env_args: Vec<String>,
30+
metadata: &mut MetadataCache,
2831
) -> anyhow::Result<crate::build::Build> {
29-
let mut config = crate::metadata::Metadata::as_json(shader_crate_path)?;
32+
let mut config = metadata.as_json(shader_crate_path)?;
3033

3134
env_args.retain(|arg| !(arg == "build" || arg == "install"));
3235
let cli_args_json = Self::cli_args_to_json(env_args)?;
@@ -102,6 +105,7 @@ mod test {
102105
"--debug".to_owned(),
103106
"--auto-install-rust-toolchain".to_owned(),
104107
],
108+
&mut MetadataCache::default(),
105109
)
106110
.unwrap();
107111
assert!(!args.build.spirv_builder.release);
@@ -126,7 +130,12 @@ mod test {
126130
})
127131
.unwrap();
128132

129-
let args = Config::clap_command_with_cargo_config(&shader_crate_path, vec![]).unwrap();
133+
let args = Config::clap_command_with_cargo_config(
134+
&shader_crate_path,
135+
vec![],
136+
&mut MetadataCache::default(),
137+
)
138+
.unwrap();
130139
assert!(!args.build.spirv_builder.release);
131140
assert!(args.install.auto_install_rust_toolchain);
132141
}
@@ -150,7 +159,12 @@ mod test {
150159
let _env = TestEnv::new();
151160
let shader_crate_path = update_cargo_output_dir(&_env);
152161

153-
let args = Config::clap_command_with_cargo_config(&shader_crate_path, vec![]).unwrap();
162+
let args = Config::clap_command_with_cargo_config(
163+
&shader_crate_path,
164+
vec![],
165+
&mut MetadataCache::default(),
166+
)
167+
.unwrap();
154168
if cfg!(target_os = "windows") {
155169
assert!(
156170
args.build
@@ -179,6 +193,7 @@ mod test {
179193
"--output-dir".to_owned(),
180194
"/the/river".to_owned(),
181195
],
196+
&mut MetadataCache::default(),
182197
)
183198
.unwrap();
184199
assert_eq!(args.build.output_dir, std::path::Path::new("/the/river"));
@@ -200,7 +215,12 @@ mod test {
200215
})
201216
.unwrap();
202217

203-
let args = Config::clap_command_with_cargo_config(&shader_crate_path, vec![]).unwrap();
218+
let args = Config::clap_command_with_cargo_config(
219+
&shader_crate_path,
220+
vec![],
221+
&mut MetadataCache::default(),
222+
)
223+
.unwrap();
204224
assert_eq!(
205225
args.build.spirv_builder.capabilities,
206226
vec![
@@ -223,6 +243,7 @@ mod test {
223243
"--manifest-file".to_owned(),
224244
"mymanifest".to_owned(),
225245
],
246+
&mut MetadataCache::default(),
226247
)
227248
.unwrap();
228249
assert_eq!(args.build.manifest_file, "mymanifest".to_owned());

crates/cargo-gpu/src/install.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,16 @@ impl InstalledBackend {
6464
#[derive(clap::Parser, Debug, Clone, serde::Deserialize, serde::Serialize)]
6565
#[non_exhaustive]
6666
pub struct Install {
67+
/// Cargo target to compile.
68+
///
69+
/// Conflicts with `--shader-crate`.
70+
#[clap(short, long, conflicts_with("shader_crate"))]
71+
pub package: Option<String>,
72+
6773
/// Directory containing the shader crate to compile.
68-
#[clap(long, alias("package"), short_alias('p'), default_value = "./")]
69-
#[serde(alias = "package")]
74+
///
75+
/// Conflicts with `--package` or `-p`.
76+
#[clap(long, default_value = "./", conflicts_with("package"))]
7077
pub shader_crate: PathBuf,
7178

7279
#[expect(
@@ -132,6 +139,7 @@ impl Install {
132139
#[must_use]
133140
pub const fn from_shader_crate(shader_crate: PathBuf) -> Self {
134141
Self {
142+
package: None,
135143
shader_crate,
136144
spirv_builder_source: None,
137145
spirv_builder_version: None,

crates/cargo-gpu/src/lib.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ mod target_specs;
6868
mod test;
6969

7070
pub use install::*;
71+
pub use metadata::MetadataCache;
7172
pub use spirv_builder;
7273

7374
/// Central function to write to the user.
@@ -120,12 +121,19 @@ impl Command {
120121
/// # Errors
121122
/// Any errors during execution, usually printed to the user
122123
#[inline]
123-
pub fn run(&self, env_args: Vec<String>) -> anyhow::Result<()> {
124+
pub fn run(
125+
&self,
126+
env_args: Vec<String>,
127+
metadata_cache: &mut metadata::MetadataCache,
128+
) -> anyhow::Result<()> {
124129
match &self {
125130
Self::Install(install) => {
126131
let shader_crate_path = &install.shader_crate;
127-
let command =
128-
config::Config::clap_command_with_cargo_config(shader_crate_path, env_args)?;
132+
let command = config::Config::clap_command_with_cargo_config(
133+
shader_crate_path,
134+
env_args,
135+
metadata_cache,
136+
)?;
129137
log::debug!(
130138
"installing with final merged arguments: {:#?}",
131139
command.install
@@ -134,8 +142,11 @@ impl Command {
134142
}
135143
Self::Build(build) => {
136144
let shader_crate_path = &build.install.shader_crate;
137-
let mut command =
138-
config::Config::clap_command_with_cargo_config(shader_crate_path, env_args)?;
145+
let mut command = config::Config::clap_command_with_cargo_config(
146+
shader_crate_path,
147+
env_args,
148+
metadata_cache,
149+
)?;
139150
log::debug!("building with final merged arguments: {command:#?}");
140151

141152
if command.build.watch {

crates/cargo-gpu/src/main.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,5 +37,6 @@ fn run() -> anyhow::Result<()> {
3737
.collect::<Vec<_>>();
3838
log::trace!("CLI args: {env_args:#?}");
3939
let cli = Cli::parse_from(&env_args);
40-
cli.command.run(env_args)
40+
let mut metadata_cache = cargo_gpu::MetadataCache::default();
41+
cli.command.run(env_args, &mut metadata_cache)
4142
}

crates/cargo-gpu/src/metadata.rs

Lines changed: 76 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,86 @@
11
//! Get config from the shader crate's `Cargo.toml` `[*.metadata.rust-gpu.*]`
22
3+
use std::collections::HashMap;
4+
5+
use anyhow::Context as _;
36
use cargo_metadata::MetadataCommand;
47
use serde_json::Value;
58

6-
/// `Metadata` refers to the `[metadata.*]` section of `Cargo.toml` that `cargo` formally
9+
/// A cache of metadata from various `Cargo.toml` files.
10+
///
11+
/// "Metadata" refers to the `[metadata.*]` section of `Cargo.toml` that `cargo` formally
712
/// ignores so that packages can implement their own behaviour with it.
8-
#[derive(Debug)]
9-
pub struct Metadata;
13+
#[derive(Debug, Default)]
14+
pub struct MetadataCache {
15+
/// Cached result of `MetadataCommand::new().exec()`.
16+
inner: HashMap<std::path::PathBuf, cargo_metadata::Metadata>,
17+
}
18+
19+
impl MetadataCache {
20+
/// Return the cached cargo metadata for the Cargo.toml at the given path,
21+
/// or find it, populate the cache with it and return it.
22+
fn get_metadata(
23+
&mut self,
24+
maybe_path_to_manifest_dir: Option<&std::path::Path>,
25+
) -> anyhow::Result<&cargo_metadata::Metadata> {
26+
let path = if let Some(path) = maybe_path_to_manifest_dir {
27+
path.to_path_buf()
28+
} else {
29+
std::env::current_dir().context("cannot determine the current working directory")?
30+
};
31+
32+
if !self.inner.contains_key(&path) {
33+
let metadata = MetadataCommand::new().current_dir(&path).exec()?;
34+
self.inner.insert(path.clone(), metadata);
35+
}
36+
37+
self.inner.get(&path).context("unreachable")
38+
}
39+
40+
/// Resolve a package name to a crate directory.
41+
///
42+
/// ## Errors
43+
/// * if fetching cargo metadata fails.
44+
/// * if no packages are listed in the cargo metadata.
45+
/// * if the manifest path has no parent.
46+
pub fn resolve_package_to_shader_crate(
47+
&mut self,
48+
package: &str,
49+
) -> anyhow::Result<std::path::PathBuf> {
50+
log::debug!("resolving package '{package}' to shader crate");
51+
let metadata = self.get_metadata(None)?;
52+
53+
let meta_package = metadata
54+
.packages
55+
.iter()
56+
.find(|pkg| pkg.name.as_str() == package)
57+
.context("Package not found in metadata")?;
58+
let shader_crate_path: std::path::PathBuf = meta_package
59+
.manifest_path
60+
.parent()
61+
.context("manifest is missing a parent directory")?
62+
.to_path_buf()
63+
.into();
64+
log::debug!(
65+
" determined shader crate path to be '{}'",
66+
shader_crate_path.display()
67+
);
68+
Ok(shader_crate_path)
69+
}
1070

11-
impl Metadata {
1271
/// Convert `rust-gpu`-specific sections in `Cargo.toml` to `clap`-compatible arguments.
1372
/// The section in question is: `[package.metadata.rust-gpu.*]`. See the `shader-crate-template`
1473
/// for an example.
1574
///
1675
/// First we generate the CLI arg defaults as JSON. Then on top of those we merge any config
1776
/// from the workspace `Cargo.toml`, then on top of those we merge any config from the shader
1877
/// crate's `Cargo.toml`.
19-
pub fn as_json(path: &std::path::PathBuf) -> anyhow::Result<Value> {
20-
let cargo_json = Self::get_cargo_toml_as_json(path)?;
78+
///
79+
/// ## Errors
80+
/// Errors if cargo metadata cannot be found or if it cannot be operated on.
81+
pub fn as_json(&mut self, path: &std::path::Path) -> anyhow::Result<Value> {
82+
log::debug!("reading package metadata from {}", path.display());
83+
let cargo_json = self.get_cargo_toml_as_json(path)?;
2184
let config = Self::merge_configs(&cargo_json, path)?;
2285
Ok(config)
2386
}
@@ -27,6 +90,7 @@ impl Metadata {
2790
cargo_json: &cargo_metadata::Metadata,
2891
path: &std::path::Path,
2992
) -> anyhow::Result<Value> {
93+
log::debug!("merging cargo metadata from {}", path.display());
3094
let mut metadata = crate::config::Config::defaults_as_json()?;
3195
crate::config::Config::json_merge(
3296
&mut metadata,
@@ -65,9 +129,10 @@ impl Metadata {
65129

66130
/// Convert a `Cargo.toml` to JSON
67131
fn get_cargo_toml_as_json(
68-
path: &std::path::PathBuf,
132+
&mut self,
133+
path: &std::path::Path,
69134
) -> anyhow::Result<cargo_metadata::Metadata> {
70-
Ok(MetadataCommand::new().current_dir(path).exec()?)
135+
self.get_metadata(Some(path)).cloned()
71136
}
72137

73138
/// Get any `rust-gpu` metadata set in the crate's `Cargo.toml`
@@ -136,7 +201,7 @@ mod test {
136201
.exec()
137202
.unwrap();
138203
metadata.packages.first_mut().unwrap().metadata = serde_json::json!({});
139-
let configs = Metadata::merge_configs(&metadata, Path::new("./")).unwrap();
204+
let configs = MetadataCache::merge_configs(&metadata, Path::new("./")).unwrap();
140205
assert_eq!(configs["build"]["release"], Value::Bool(true));
141206
assert_eq!(
142207
configs["install"]["auto_install_rust_toolchain"],
@@ -160,7 +225,7 @@ mod test {
160225
}
161226
}
162227
});
163-
let configs = Metadata::merge_configs(&metadata, Path::new("./")).unwrap();
228+
let configs = MetadataCache::merge_configs(&metadata, Path::new("./")).unwrap();
164229
assert_eq!(configs["build"]["release"], Value::Bool(false));
165230
assert_eq!(
166231
configs["install"]["auto_install_rust_toolchain"],
@@ -189,7 +254,7 @@ mod test {
189254
}
190255
}
191256
});
192-
let configs = Metadata::merge_configs(&metadata, Path::new(".")).unwrap();
257+
let configs = MetadataCache::merge_configs(&metadata, Path::new(".")).unwrap();
193258
assert_eq!(configs["build"]["release"], Value::Bool(false));
194259
assert_eq!(
195260
configs["install"]["auto_install_rust_toolchain"],

0 commit comments

Comments
 (0)