Skip to content

Commit

Permalink
Auto merge of #13842 - skogseth:build-only-specified-artifact-library…
Browse files Browse the repository at this point in the history
…, r=weihanglo

Fix: Build only the specified artifact library when multiple types are available

### What does this PR try to resolve?
Fixes #12109.

#### TL;DR:
A crate `bar` exposes it's library as both a `staticlib` and a `cdylib`. A second crate `foo` specifies an artifact dependency of type `staticlib` on `bar`, meaning cargo should build `bar` as a `staticlib` and expose certain environment variables (e.g. `CARGO_STATICLIB_FILE_BAR`). However, due to a bug, cargo ends up building (and exposing the associated environment variables) for both the `staticlib` and `cdylib`.

The same happens if `foo` specifies an artifact dependency of type `cdylib`; both artifact types are built.

### How should we test and review this PR?
The first commit introduces a test which reproduces the issue, the second commit introduces the fix. This setup was recommended by `@ehuss.`

### Additional information
Artifact dependencies: https://rust-lang.github.io/rfcs/3028-cargo-binary-dependencies.html

#### TL;DR
If a crate `foo` requests an artifact dependency of kind <artifact_kind> from a crate `bar` then the following happens:

- `bar` is built with crate-type <artifact_kind> and a directory is created at `target/<profile>/deps/artifact/bar-<build_hash_or_something>/<artifact_kind>`. The binary artifact is placed in this directory.
- Cargo exposes certain environment variables, the most important for this PR is `CARGO_<artifact_kind>_FILE_BAR`, which points to the binary artifact that was specified. This environment variable is available at compile time for normal dependencies and at runtime for build-dependencies.

If multiple artifact-kinds are requested cargo will create a unit for each, and so they will all be built separately.
  • Loading branch information
bors committed May 7, 2024
2 parents 9b4a501 + 9a5cfbc commit 4e59631
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 5 deletions.
13 changes: 8 additions & 5 deletions src/cargo/core/compiler/unit_dependencies.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use crate::core::compiler::unit_graph::{UnitDep, UnitGraph};
use crate::core::compiler::{
CompileKind, CompileMode, CrateType, RustcTargetData, Unit, UnitInterner,
};
use crate::core::dependency::{Artifact, ArtifactTarget, DepKind};
use crate::core::dependency::{Artifact, ArtifactKind, ArtifactTarget, DepKind};
use crate::core::profiles::{Profile, Profiles, UnitFor};
use crate::core::resolver::features::{FeaturesFor, ResolvedFeatures};
use crate::core::resolver::Resolve;
Expand Down Expand Up @@ -555,17 +555,20 @@ fn artifact_targets_to_unit_deps(
let ret =
match_artifacts_kind_with_targets(dep, artifact_pkg.targets(), parent.pkg.name().as_str())?
.into_iter()
.map(|(_artifact_kind, target)| target)
.flat_map(|target| {
.flat_map(|(artifact_kind, target)| {
// We split target libraries into individual units, even though rustc is able
// to produce multiple kinds in an single invocation for the sole reason that
// to produce multiple kinds in a single invocation for the sole reason that
// each artifact kind has its own output directory, something we can't easily
// teach rustc for now.
match target.kind() {
TargetKind::Lib(kinds) => Box::new(
kinds
.iter()
.filter(|tk| matches!(tk, CrateType::Cdylib | CrateType::Staticlib))
.filter(move |tk| match (tk, artifact_kind) {
(CrateType::Cdylib, ArtifactKind::Cdylib) => true,
(CrateType::Staticlib, ArtifactKind::Staticlib) => true,
_ => false,
})
.map(|target_kind| {
new_unit_dep(
state,
Expand Down
74 changes: 74 additions & 0 deletions tests/testsuite/artifact_dep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3189,3 +3189,77 @@ fn check_transitive_artifact_dependency_with_different_target() {
.with_status(101)
.run();
}

#[cargo_test]
fn build_only_specified_artifact_library() {
// Create a project with:
// - A crate `bar` with both `staticlib` and `cdylib` as crate-types.
// - A crate `foo` which depends on either the `staticlib` or `cdylib` artifact of bar,
// whose build-script simply checks which library artifacts are present.
let create_project = |artifact_lib| {
project()
.file(
"bar/Cargo.toml",
r#"
[package]
name = "bar"
version = "1.0.0"
[lib]
crate-type = ["staticlib", "cdylib"]
"#,
)
.file("bar/src/lib.rs", "")
.file(
"Cargo.toml",
&format!(
r#"
[package]
name = "foo"
version = "1.0.0"
[build-dependencies]
bar = {{ path = "bar", artifact = "{artifact_lib}" }}
"#),
)
.file("src/lib.rs", "")
.file(
"build.rs",
r#"
fn main() {
println!("cdylib present: {}", std::env::var_os("CARGO_CDYLIB_FILE_BAR").is_some());
println!("staticlib present: {}", std::env::var_os("CARGO_STATICLIB_FILE_BAR").is_some());
}
"#,
)
.build()
};

let cdylib = create_project("cdylib");
cdylib
.cargo("build -Z bindeps")
.masquerade_as_nightly_cargo(&["bindeps"])
.run();
match_exact(
"cdylib present: true\nstaticlib present: false",
&build_script_output_string(&cdylib, "foo"),
"build script output",
"",
None,
)
.unwrap();

let staticlib = create_project("staticlib");
staticlib
.cargo("build -Z bindeps")
.masquerade_as_nightly_cargo(&["bindeps"])
.run();
match_exact(
"cdylib present: false\nstaticlib present: true",
&build_script_output_string(&staticlib, "foo"),
"build script output",
"",
None,
)
.unwrap();
}

0 comments on commit 4e59631

Please sign in to comment.