Skip to content

Commit

Permalink
fix(toml): Warn, rather than fail publish, if targets are excluded
Browse files Browse the repository at this point in the history 
This could offer performance gains when parsing a published
manifest since the targets don't need to be discovered.
To see this, we'd first need to stop discovering potential targets even when it isn't
needed.
  • Loading branch information
@epage
epage committed Apr 10, 2024
1 parent f38fce5 commit e2e0fb4
Show file tree
Hide file tree
Showing 9 changed files with 488 additions and 91 deletions.
121 changes: 95 additions & 26 deletions src/cargo/util/toml/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,14 +263,14 @@ fn resolve_toml(
manifest_file: &Path,
gctx: &GlobalContext,
warnings: &mut Vec<String>,
_errors: &mut Vec<String>,
errors: &mut Vec<String>,
) -> CargoResult<manifest::TomlManifest> {
let mut resolved_toml = manifest::TomlManifest {
cargo_features: original_toml.cargo_features.clone(),
package: None,
project: None,
profile: original_toml.profile.clone(),
lib: original_toml.lib.clone(),
lib: None,
bin: original_toml.bin.clone(),
example: original_toml.example.clone(),
test: original_toml.test.clone(),
Expand Down Expand Up @@ -301,6 +301,62 @@ fn resolve_toml(
if let Some(original_package) = original_toml.package() {
let resolved_package = resolve_package_toml(original_package, package_root, &inherit)?;
resolved_toml.package = Some(resolved_package);
let edition = if let Some(edition) = resolved_toml
.package
.as_ref()
.expect("in `package` branch")
.resolved_edition()
.expect("previously resolved")
{
let edition: Edition = edition
.parse()
.with_context(|| "failed to parse the `edition` key")?;
edition
} else {
let default_edition = Edition::default();
default_edition
};

resolved_toml.lib = targets::resolve_lib(
original_toml.lib.as_ref(),
package_root,
&original_package.name,
edition,
warnings,
)?;
resolved_toml.bin = Some(targets::resolve_bins(
original_toml.bin.as_ref(),
package_root,
&original_package.name,
edition,
original_package.autobins,
warnings,
resolved_toml.lib.is_some(),
)?);
resolved_toml.example = Some(targets::resolve_examples(
original_toml.example.as_ref(),
package_root,
edition,
original_package.autoexamples,
warnings,
errors,
)?);
resolved_toml.test = Some(targets::resolve_tests(
original_toml.test.as_ref(),
package_root,
edition,
original_package.autotests,
warnings,
errors,
)?);
resolved_toml.bench = Some(targets::resolve_benches(
original_toml.bench.as_ref(),
package_root,
edition,
original_package.autobenches,
warnings,
errors,
)?);

resolved_toml.dependencies = resolve_dependencies(
gctx,
Expand Down Expand Up @@ -453,10 +509,10 @@ fn resolve_package_toml<'a>(
.map(manifest::InheritableField::Value),
workspace: original_package.workspace.clone(),
im_a_teapot: original_package.im_a_teapot.clone(),
autobins: original_package.autobins.clone(),
autoexamples: original_package.autoexamples.clone(),
autotests: original_package.autotests.clone(),
autobenches: original_package.autobenches.clone(),
autobins: Some(false),
autoexamples: Some(false),
autotests: Some(false),
autobenches: Some(false),
default_run: original_package.default_run.clone(),
description: original_package
.description
Expand Down Expand Up @@ -1093,8 +1149,8 @@ fn to_real_manifest(
// If we have a lib with no path, use the inferred lib or else the package name.
let targets = to_targets(
&features,
&original_toml,
&resolved_toml,
package_name,
package_root,
edition,
&resolved_package.metabuild,
Expand Down Expand Up @@ -2471,14 +2527,14 @@ fn prepare_toml_for_publish(
}

let lib = if let Some(target) = &me.lib {
Some(prepare_target_for_publish(target, "library")?)
prepare_target_for_publish(target, included, "library", ws.gctx())?
} else {
None
};
let bin = prepare_targets_for_publish(me.bin.as_ref(), "binary")?;
let example = prepare_targets_for_publish(me.example.as_ref(), "example")?;
let test = prepare_targets_for_publish(me.test.as_ref(), "test")?;
let bench = prepare_targets_for_publish(me.bench.as_ref(), "benchmark")?;
let bin = prepare_targets_for_publish(me.bin.as_ref(), included, "binary", ws.gctx())?;
let example = prepare_targets_for_publish(me.example.as_ref(), included, "example", ws.gctx())?;
let test = prepare_targets_for_publish(me.test.as_ref(), included, "test", ws.gctx())?;
let bench = prepare_targets_for_publish(me.bench.as_ref(), included, "benchmark", ws.gctx())?;

let all = |_d: &manifest::TomlDependency| true;
let mut manifest = manifest::TomlManifest {
Expand Down Expand Up @@ -2636,40 +2692,53 @@ fn prepare_toml_for_publish(

fn prepare_targets_for_publish(
targets: Option<&Vec<manifest::TomlTarget>>,
included: &[String],
context: &str,
gctx: &GlobalContext,
) -> CargoResult<Option<Vec<manifest::TomlTarget>>> {
let Some(targets) = targets else {
return Ok(None);
};

let mut prepared = Vec::with_capacity(targets.len());
for target in targets {
let target = prepare_target_for_publish(target, context)?;
let Some(target) = prepare_target_for_publish(target, included, context, gctx)? else {
continue;
};
prepared.push(target);
}

Ok(Some(prepared))
if prepared.is_empty() {
Ok(None)
} else {
Ok(Some(prepared))
}
}

fn prepare_target_for_publish(
target: &manifest::TomlTarget,
included: &[String],
context: &str,
) -> CargoResult<manifest::TomlTarget> {
let mut target = target.clone();
if let Some(path) = target.path {
let path = normalize_path(&path.0);
target.path = Some(manifest::PathValue(normalize_path_sep(path, context)?));
}
Ok(target)
}

fn normalize_path_sep(path: PathBuf, context: &str) -> CargoResult<PathBuf> {
gctx: &GlobalContext,
) -> CargoResult<Option<manifest::TomlTarget>> {
let path = target.path.as_ref().expect("previously resolved");
let path = normalize_path(&path.0);
let path = path
.into_os_string()
.into_string()
.map_err(|_err| anyhow::format_err!("non-UTF8 path for {context}"))?;
let path = normalize_path_string_sep(path);
Ok(path.into())
if !included.contains(&path) {
let name = target.name.as_ref().expect("previously resolved");
gctx.shell().warn(format!(
"ignoring {context} `{name}` as `{path}` is not included in the published package",
))?;
return Ok(None);
}

let mut target = target.clone();
target.path = Some(manifest::PathValue(normalize_path_string_sep(path).into()));

Ok(Some(target))
}

fn normalize_path_string_sep(path: String) -> String {
Expand Down
67 changes: 15 additions & 52 deletions src/cargo/util/toml/targets.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ const DEFAULT_EXAMPLE_DIR_NAME: &'static str = "examples";
#[tracing::instrument(skip_all)]
pub(super) fn to_targets(
features: &Features,
original_toml: &TomlManifest,
resolved_toml: &TomlManifest,
package_name: &str,
package_root: &Path,
edition: Edition,
metabuild: &Option<StringOrVec>,
Expand All @@ -44,84 +44,47 @@ pub(super) fn to_targets(
) -> CargoResult<Vec<Target>> {
let mut targets = Vec::new();

let has_lib;

let lib = resolve_lib(
resolved_toml.lib.as_ref(),
package_root,
package_name,
edition,
warnings,
)?;
if let Some(target) = to_lib_target(
original_toml.lib.as_ref(),
resolved_toml.lib.as_ref(),
lib.as_ref(),
package_root,
edition,
warnings,
)? {
targets.push(target);
has_lib = true;
} else {
has_lib = false;
}

let package = resolved_toml
.package
.as_ref()
.ok_or_else(|| anyhow::format_err!("manifest has no `package` (or `project`)"))?;

let bins = resolve_bins(
resolved_toml.bin.as_ref(),
package_root,
package_name,
edition,
package.autobins,
warnings,
has_lib,
)?;
targets.extend(to_bin_targets(
features,
&bins,
resolved_toml.bin.as_deref().unwrap_or_default(),
package_root,
edition,
errors,
)?);

let toml_examples = resolve_examples(
resolved_toml.example.as_ref(),
package_root,
edition,
package.autoexamples,
warnings,
errors,
)?;
targets.extend(to_example_targets(
&toml_examples,
resolved_toml.example.as_deref().unwrap_or_default(),
package_root,
edition,
warnings,
)?);

let toml_tests = resolve_tests(
resolved_toml.test.as_ref(),
targets.extend(to_test_targets(
resolved_toml.test.as_deref().unwrap_or_default(),
package_root,
edition,
package.autotests,
warnings,
errors,
)?;
targets.extend(to_test_targets(&toml_tests, package_root, edition)?);
)?);

let toml_benches = resolve_benches(
resolved_toml.bench.as_ref(),
targets.extend(to_bench_targets(
resolved_toml.bench.as_deref().unwrap_or_default(),
package_root,
edition,
package.autobenches,
warnings,
errors,
)?;
targets.extend(to_bench_targets(&toml_benches, package_root, edition)?);
)?);

// processing the custom build script
if let Some(custom_build) = package.resolved_build().expect("should be resolved") {
Expand Down Expand Up @@ -163,7 +126,7 @@ pub(super) fn to_targets(
Ok(targets)
}

fn resolve_lib(
pub fn resolve_lib(
original_lib: Option<&TomlLibTarget>,
package_root: &Path,
package_name: &str,
Expand Down Expand Up @@ -286,7 +249,7 @@ fn to_lib_target(
Ok(Some(target))
}

fn resolve_bins(
pub fn resolve_bins(
toml_bins: Option<&Vec<TomlBinTarget>>,
package_root: &Path,
package_name: &str,
Expand Down Expand Up @@ -412,7 +375,7 @@ fn legacy_bin_path(package_root: &Path, name: &str, has_lib: bool) -> Option<Pat
None
}

fn resolve_examples(
pub fn resolve_examples(
toml_examples: Option<&Vec<TomlExampleTarget>>,
package_root: &Path,
edition: Edition,
Expand Down Expand Up @@ -469,7 +432,7 @@ fn to_example_targets(
Ok(result)
}

fn resolve_tests(
pub fn resolve_tests(
toml_tests: Option<&Vec<TomlTestTarget>>,
package_root: &Path,
edition: Edition,
Expand Down Expand Up @@ -517,7 +480,7 @@ fn to_test_targets(
Ok(result)
}

fn resolve_benches(
pub fn resolve_benches(
toml_benches: Option<&Vec<TomlBenchTarget>>,
package_root: &Path,
edition: Edition,
Expand Down
8 changes: 8 additions & 0 deletions tests/testsuite/artifact_dep.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2165,6 +2165,10 @@ name = "foo"
version = "0.1.0"
authors = []
build = false
autobins = false
autoexamples = false
autotests = false
autobenches = false
description = "foo"
homepage = "foo"
documentation = "foo"
Expand All @@ -2173,6 +2177,10 @@ license = "MIT"
repository = "foo"
resolver = "2"
[lib]
name = "foo"
path = "src/lib.rs"
[dependencies.bar]
version = "1.0"
artifact = ["bin"]
Expand Down
8 changes: 8 additions & 0 deletions tests/testsuite/features2.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1704,11 +1704,19 @@ name = "a"
version = "0.1.0"
authors = ["Zzz"]
build = false
autobins = false
autoexamples = false
autotests = false
autobenches = false
description = "foo"
homepage = "https://example.com/"
readme = false
license = "MIT"
resolver = "2"
[lib]
name = "a"
path = "src/lib.rs"
"#,
cargo::core::manifest::MANIFEST_PREAMBLE
);
Expand Down

0 comments on commit e2e0fb4

Please sign in to comment.