Skip to content

Commit

Permalink
Various improvements to cargo-component. (#240)
Browse files Browse the repository at this point in the history
* Various improvements to `cargo-component`.

This commit includes the following improvements to `cargo-component`:

* `cargo-component` now componentizes based off the presence of component type
  information custom sections rather than always requiring a
  `[package.metadata.component]` section be present in `Cargo.toml`.
* Bindings generation will now occur if a `wit` directory is present and
  `[package.metadata.component]` is not present in `Cargo.toml`.
* Fix a panic in `cargo component add` if it encounters expected items that are
  not tables in `Cargo.toml`.
* Fix not setting implicit tables for intermediate tables for the `cargo
  component add` command.
* Fix not displaying anything when `--help` is present for commands passed
  directly to `cargo`.
* `cargo-component` now uses the command adapter for binary examples and
  test/bench artifacts.
* `cargo-component` now displays the same "Running" message that `cargo` does
  when using the `run`, `test`, and `bench` commands.
* Some paths displayed in status messages have been prefix-stripped based on
  the CWD.

This commit also contains a bit of refactoring to `run_cargo_command`, as it
was getting long and unwieldy.

Fixes #236.

* Remove `Option` from component section in metadata.

This makes using the section more ergonomic.
  • Loading branch information
peterhuene authored Feb 28, 2024
1 parent c627163 commit f98de99
Show file tree
Hide file tree
Showing 9 changed files with 572 additions and 275 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ wit-bindgen-core = { workspace = true }
wit-parser = { workspace = true }
wit-component = { workspace = true }
wasm-metadata = { workspace = true }
wasmparser = { workspace = true }
parse_arg = { workspace = true }
cargo_metadata = { workspace = true }
cargo-config2 = { workspace = true }
Expand All @@ -50,11 +51,11 @@ rpassword = { workspace = true }
futures = { workspace = true }
bytes = { workspace = true }
which = { workspace = true }
shell-escape = "0.1.5"

[dev-dependencies]
assert_cmd = { workspace = true }
predicates = { workspace = true }
wasmparser = { workspace = true }
wat = { workspace = true }
warg-server = { workspace = true }
tempfile = { workspace = true }
Expand Down
12 changes: 4 additions & 8 deletions src/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
use crate::{
last_modified_time,
metadata::{ComponentMetadata, Ownership, Target},
metadata::{ComponentMetadata, Ownership},
registry::PackageDependencyResolution,
};
use anyhow::{bail, Context, Result};
Expand Down Expand Up @@ -277,14 +277,10 @@ impl<'a> BindingsGenerator<'a> {
import_name_map: &mut HashMap<String, String>,
) -> Result<(Resolve, WorldId, Vec<PathBuf>)> {
let (mut merged, world_id, source_files) =
if let Target::Package { name, world, .. } = &resolution.metadata.section.target {
Self::target_package(resolution, name, world.as_deref())?
if let Some(name) = resolution.metadata.target_package() {
Self::target_package(resolution, name, resolution.metadata.target_world())?
} else if let Some(path) = resolution.metadata.target_path() {
Self::target_local_path(
resolution,
&path,
resolution.metadata.section.target.world(),
)?
Self::target_local_path(resolution, &path, resolution.metadata.target_world())?
} else {
let (merged, world) = Self::target_empty_world(resolution);
(merged, world, Vec::new())
Expand Down
41 changes: 24 additions & 17 deletions src/commands/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,6 @@ impl AddCommand {
)?,
};

let metadata = metadata.with_context(|| {
format!(
"manifest `{path}` is not a WebAssembly component package",
path = package.manifest_path
)
})?;

let name = match &self.name {
Some(name) => name,
None => &self.package.name,
Expand Down Expand Up @@ -183,25 +176,39 @@ impl AddCommand {
)
})?;

let metadata = document["package"]["metadata"]
.or_insert(Item::Table(Table::new()))
.as_table_mut()
.context("section `package.metadata` is not a table")?;

metadata.set_implicit(true);

let component = metadata["component"]
.or_insert(Item::Table(Table::new()))
.as_table_mut()
.context("section `package.metadata.component` is not a table")?;

component.set_implicit(true);

let dependencies = if self.target {
let target = document["package"]["metadata"]["component"]["target"]
let target = component["target"]
.or_insert(Item::Table(Table::new()))
.as_table_mut()
.unwrap();
.context("section `package.metadata.component.target` is not a table")?;

target.set_implicit(true);

target["dependencies"]
.or_insert(Item::Table(Table::new()))
.as_table_mut()
.unwrap()
.context(
"section `package.metadata.component.target.dependencies` is not a table",
)?
} else {
document["package"]["metadata"]["component"]["dependencies"]
component["dependencies"]
.or_insert(Item::Table(Table::new()))
.as_table_mut()
.with_context(|| {
format!(
"failed to find component metadata in manifest file `{path}`",
path = pkg.manifest_path
)
})?
.context("section `package.metadata.component.dependencies` is not a table")?
};

body(dependencies)?;
Expand Down
9 changes: 2 additions & 7 deletions src/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,7 @@ impl PublishCommand {
})?];

let package = packages[0].package;
let component_metadata = packages[0].metadata.as_ref().with_context(|| {
format!(
"package `{name}` is missing component metadata in manifest `{path}`",
name = package.name,
path = package.manifest_path
)
})?;
let component_metadata = &packages[0].metadata;

let name = component_metadata.section.package.as_ref().with_context(|| {
format!(
Expand Down Expand Up @@ -153,6 +147,7 @@ impl PublishCommand {
let cargo_build_args = CargoArguments {
color: self.common.color,
verbose: self.common.verbose as usize,
help: false,
quiet: self.common.quiet,
targets: self.target.clone().into_iter().collect(),
manifest_path: self.manifest_path.clone(),
Expand Down
9 changes: 8 additions & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,8 @@ pub struct CargoArguments {
pub color: Option<Color>,
/// The (count of) --verbose argument.
pub verbose: usize,
/// The --help argument.
pub help: bool,
/// The --quiet argument.
pub quiet: bool,
/// The --target argument.
Expand Down Expand Up @@ -422,7 +424,8 @@ impl CargoArguments {
.flag("--all", None)
.flag("--workspace", None)
.counting("--verbose", Some('v'))
.flag("--quiet", Some('q'));
.flag("--quiet", Some('q'))
.flag("--help", Some('h'));

let mut iter = iter.map(Into::into).peekable();

Expand Down Expand Up @@ -453,6 +456,7 @@ impl CargoArguments {
.map(|v| v.parse())
.transpose()?,
verbose: args.get("--verbose").unwrap().count(),
help: args.get("--help").unwrap().count() > 0,
quiet: args.get("--quiet").unwrap().count() > 0,
manifest_path: args
.get_mut("--manifest-path")
Expand Down Expand Up @@ -775,6 +779,7 @@ mod test {
CargoArguments {
color: None,
verbose: 0,
help: false,
quiet: false,
targets: Vec::new(),
manifest_path: None,
Expand All @@ -792,6 +797,7 @@ mod test {
[
"component",
"publish",
"--help",
"-vvv",
"--color=auto",
"--manifest-path",
Expand Down Expand Up @@ -820,6 +826,7 @@ mod test {
CargoArguments {
color: Some(Color::Auto),
verbose: 3,
help: true,
quiet: true,
targets: vec!["foo".to_string(), "bar".to_string()],
manifest_path: Some("Cargo.toml".into()),
Expand Down
Loading

0 comments on commit f98de99

Please sign in to comment.