Skip to content

Commit

Permalink
Various improvements to cargo-component.
Browse files Browse the repository at this point in the history
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.
* 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.
  • Loading branch information
peterhuene committed Feb 27, 2024
1 parent c627163 commit ed45674
Show file tree
Hide file tree
Showing 9 changed files with 661 additions and 333 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
20 changes: 11 additions & 9 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 @@ -140,7 +140,13 @@ impl<'a> BindingsGenerator<'a> {

/// Generates the bindings source for a package.
pub fn generate(self) -> Result<String> {
let settings = &self.resolution.metadata.section.bindings;
let settings = self
.resolution
.metadata
.section
.as_ref()
.map(|s| Cow::Borrowed(&s.bindings))
.unwrap_or_default();

fn implementor_path_str(path: &str) -> String {
format!("super::{path}")
Expand Down Expand Up @@ -277,14 +283,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
72 changes: 44 additions & 28 deletions src/commands/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use cargo_metadata::Package;
use clap::Args;
use semver::VersionReq;
use std::{
borrow::Cow,
fs,
path::{Path, PathBuf},
};
Expand Down Expand Up @@ -86,13 +87,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 @@ -131,9 +125,15 @@ impl AddCommand {
name: &PackageName,
network_allowed: bool,
) -> Result<String> {
let registries = metadata
.section
.as_ref()
.map(|s| Cow::Borrowed(&s.registries))
.unwrap_or_default();

let mut resolver = DependencyResolver::new(
config.warg(),
&metadata.section.registries,
&registries,
None,
config.terminal(),
network_allowed,
Expand Down Expand Up @@ -183,25 +183,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 Expand Up @@ -254,19 +268,21 @@ impl AddCommand {
}

fn validate(&self, metadata: &ComponentMetadata, name: &PackageName) -> Result<()> {
if self.target {
match &metadata.section.target {
Target::Package { .. } => {
bail!("cannot add dependency `{name}` to a registry package target")
}
Target::Local { dependencies, .. } => {
if dependencies.contains_key(name) {
bail!("cannot add dependency `{name}` as it conflicts with an existing dependency");
if let Some(section) = &metadata.section {
if self.target {
match &section.target {
Target::Package { .. } => {
bail!("cannot add dependency `{name}` to a registry package target")
}
Target::Local { dependencies, .. } => {
if dependencies.contains_key(name) {
bail!("cannot add dependency `{name}` as it conflicts with an existing dependency");
}
}
}
} else if section.dependencies.contains_key(name) {
bail!("cannot add dependency `{name}` as it conflicts with an existing dependency");
}
} else if metadata.section.dependencies.contains_key(name) {
bail!("cannot add dependency `{name}` as it conflicts with an existing dependency");
}

Ok(())
Expand Down
21 changes: 11 additions & 10 deletions src/commands/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
use anyhow::{bail, Context, Result};
use cargo_component_core::{command::CommonOptions, keyring::get_signing_key, registry::find_url};
use clap::Args;
use std::path::PathBuf;
use std::{borrow::Cow, path::PathBuf};
use warg_client::RegistryUrl;
use warg_crypto::signing::PrivateKey;

Expand Down Expand Up @@ -118,25 +118,25 @@ 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(|| {
let name = component_metadata.section.as_ref().map(|s| s.package.as_ref()).unwrap_or_default().with_context(|| {
format!(
"package `{name}` is missing a `package.metadata.component.package` setting in manifest `{path}`",
name = package.name,
path = package.manifest_path
)
})?;

let registries = component_metadata
.section
.as_ref()
.map(|s| Cow::Borrowed(&s.registries))
.unwrap_or_default();

let registry_url = find_url(
self.registry.as_deref(),
&component_metadata.section.registries,
&registries,
config.warg().default_url.as_deref(),
)?;

Expand All @@ -153,6 +153,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 ed45674

Please sign in to comment.