Skip to content

Commit

Permalink
Validate libs also don't see artifact dependencies as libraries with …
Browse files Browse the repository at this point in the history
…lib=false

Also
----

 - Add prelimiary test for validating build-time artifacts
 - Try to fix CI on gnu windows

   Which apparently generates paths similar to linux, but with .exe suffix.
   The current linux patterns should match that.

 - refactor

   Help sharing code across modules
  • Loading branch information
Byron committed Feb 18, 2022
1 parent a0f6387 commit d288fa9
Show file tree
Hide file tree
Showing 4 changed files with 172 additions and 51 deletions.
58 changes: 58 additions & 0 deletions src/cargo/core/compiler/artifact.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
use crate::core::compiler::unit_graph::UnitDep;
use crate::core::compiler::{Context, CrateType, FileFlavor, Unit};
use crate::core::TargetKind;
use crate::CargoResult;
use cargo_util::ProcessBuilder;

pub fn set_env(
cx: &Context<'_, '_>,
dependencies: &[UnitDep],
cmd: &mut ProcessBuilder,
) -> CargoResult<()> {
for unit_dep in dependencies.iter().filter(|d| d.unit.artifact) {
for artifact_path in cx
.outputs(&unit_dep.unit)?
.iter()
.filter_map(|f| (f.flavor == FileFlavor::Normal).then(|| &f.path))
{
let artifact_type_upper = unit_artifact_type_name_upper(&unit_dep.unit);
let dep_name_upper = unit_dep
.dep_name
.unwrap_or(unit_dep.unit.pkg.name())
.to_uppercase()
.replace("-", "_");
cmd.env(
&format!("CARGO_{}_DIR_{}", artifact_type_upper, dep_name_upper),
artifact_path.parent().expect("parent dir for artifacts"),
);
cmd.env(
&format!(
"CARGO_{}_FILE_{}_{}",
artifact_type_upper,
dep_name_upper,
unit_dep.unit.target.name()
),
artifact_path,
);
if unit_dep.unit.target.name().to_uppercase() == dep_name_upper {
cmd.env(
&format!("CARGO_{}_FILE_{}", artifact_type_upper, dep_name_upper,),
artifact_path,
);
}
}
}
Ok(())
}

fn unit_artifact_type_name_upper(unit: &Unit) -> &'static str {
match unit.target.kind() {
TargetKind::Lib(kinds) => match kinds.as_slice() {
&[CrateType::Cdylib] => "CDYLIB",
&[CrateType::Staticlib] => "STATICLIB",
invalid => unreachable!("BUG: artifacts cannot be of type {:?}", invalid),
},
TargetKind::Bin => "BIN",
invalid => unreachable!("BUG: artifacts cannot be of type {:?}", invalid),
}
}
50 changes: 3 additions & 47 deletions src/cargo/core/compiler/custom_build.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use super::job::{Freshness, Job, Work};
use super::{fingerprint, Context, LinkType, Unit};
use crate::core::compiler::artifact;
use crate::core::compiler::context::Metadata;
use crate::core::compiler::job_queue::JobState;
use crate::core::compiler::{CrateType, FileFlavor};
use crate::core::{profiles::ProfileRoot, PackageId, Target, TargetKind};
use crate::core::{profiles::ProfileRoot, PackageId, Target};
use crate::util::errors::CargoResult;
use crate::util::machine_message::{self, Message};
use crate::util::{internal, profile};
Expand Down Expand Up @@ -205,39 +205,7 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
.inherit_jobserver(&cx.jobserver);

// Find all artifact dependencies and make their file and containing directory discoverable using environment variables.
for unit_dep in dependencies.iter().filter(|d| d.unit.artifact) {
for artifact_path in cx
.outputs(&unit_dep.unit)?
.iter()
.filter_map(|f| (f.flavor == FileFlavor::Normal).then(|| &f.path))
{
let artifact_type_upper = unit_artifact_type_name_upper(&unit_dep.unit);
let dep_name_upper = unit_dep
.dep_name
.unwrap_or(unit_dep.unit.pkg.name())
.to_uppercase()
.replace("-", "_");
cmd.env(
&format!("CARGO_{}_DIR_{}", artifact_type_upper, dep_name_upper),
artifact_path.parent().expect("parent dir for artifacts"),
);
cmd.env(
&format!(
"CARGO_{}_FILE_{}_{}",
artifact_type_upper,
dep_name_upper,
unit_dep.unit.target.name()
),
artifact_path,
);
if unit_dep.unit.target.name().to_uppercase() == dep_name_upper {
cmd.env(
&format!("CARGO_{}_FILE_{}", artifact_type_upper, dep_name_upper,),
artifact_path,
);
}
}
}
artifact::set_env(cx, dependencies, &mut cmd)?;

if let Some(linker) = &bcx.target_data.target_config(unit.kind).linker {
cmd.env(
Expand Down Expand Up @@ -518,18 +486,6 @@ fn build_work(cx: &mut Context<'_, '_>, unit: &Unit) -> CargoResult<Job> {
Ok(job)
}

fn unit_artifact_type_name_upper(unit: &Unit) -> &'static str {
match unit.target.kind() {
TargetKind::Lib(kinds) => match kinds.as_slice() {
&[CrateType::Cdylib] => "CDYLIB",
&[CrateType::Staticlib] => "STATICLIB",
invalid => unreachable!("BUG: artifacts cannot be of type {:?}", invalid),
},
TargetKind::Bin => "BIN",
invalid => unreachable!("BUG: artifacts cannot be of type {:?}", invalid),
}
}

fn insert_warnings_in_build_outputs(
build_script_outputs: Arc<Mutex<BuildScriptOutputs>>,
id: PackageId,
Expand Down
3 changes: 2 additions & 1 deletion src/cargo/core/compiler/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mod artifact;
mod build_config;
mod build_context;
mod build_plan;
Expand Down Expand Up @@ -273,7 +274,7 @@ fn rustc(cx: &mut Context<'_, '_>, unit: &Unit, exec: &Arc<dyn Executor>) -> Car
// previous build scripts, we include them in the rustc invocation.

// Artifacts are in a different location than typical units, hence
// we must assure the crate-dependent directory is present.
// we must assure the crate- and target-dependent directory is present.
if is_artifact {
paths::create_dir_all(&root)?;
}
Expand Down
112 changes: 109 additions & 3 deletions tests/testsuite/artifact_dep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ fn warn_about_artifact_and_no_artifact_dep_to_same_package_within_the_same_dep_c
.run();
}

// TODO(ST): add static and cdylib artifacts, too.
#[cargo_test]
fn build_script_with_bin_artifacts() {
let p = project()
Expand Down Expand Up @@ -227,7 +228,7 @@ fn build_script_with_bin_artifacts() {

let build_script_output = build_script_output_string(&p, "foo");
let msg = "we need the binary directory for this artifact along with all binary paths";
#[cfg(not(windows))]
#[cfg(any(not(windows), target_env = "gnu"))]
{
cargo_test_support::compare::match_exact(
"[..]/artifact/bar-[..]/bin/baz-[..]\n\
Expand Down Expand Up @@ -313,6 +314,49 @@ fn build_script_with_bin_artifact_and_lib_false() {
.run();
}

#[cargo_test]
fn lib_with_bin_artifact_and_lib_false() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.0"
authors = []
[dependencies]
bar = { path = "bar/", artifact = "bin" }
"#,
)
.file(
"src/lib.rs",
r#"
fn main() {
bar::doit()
}"#,
)
.file("bar/Cargo.toml", &basic_bin_manifest("bar"))
.file("bar/src/main.rs", "fn main() { bar::doit(); }")
.file(
"bar/src/lib.rs",
r#"
pub fn doit() {
panic!("cannot be called from library due to lib = false");
}
"#,
)
.build();
p.cargo("build -Z unstable-options -Z bindeps")
.masquerade_as_nightly_cargo()
.with_status(101)
.with_stderr_contains(
"error[E0433]: failed to resolve: use of undeclared crate or module `bar`",
)
.with_stderr_contains(" --> src/lib.rs:3:16")
.run();
}

#[cargo_test]
fn build_script_with_selected_dashed_bin_artifact_and_lib_true() {
let p = project()
Expand Down Expand Up @@ -376,7 +420,7 @@ fn build_script_with_selected_dashed_bin_artifact_and_lib_true() {
let build_script_output = build_script_output_string(&p, "foo");
let msg = "we need the binary directory for this artifact and the binary itself";

#[cfg(not(windows))]
#[cfg(any(not(windows), target_env = "gnu"))]
{
cargo_test_support::compare::match_exact(
"[..]/artifact/bar-baz-[..]/bin\n\
Expand Down Expand Up @@ -411,6 +455,68 @@ fn build_script_with_selected_dashed_bin_artifact_and_lib_true() {
assert_artifact_executable_output(&p, "debug", "bar", "baz_suffix");
}

// TODO(ST): impl this, and add static and cdylib artifacts, too.
#[cargo_test]
fn lib_with_selected_dashed_bin_artifact_and_lib_true() {
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.0"
authors = []
[dependencies]
bar-baz = { path = "bar/", artifact = "bin:baz-suffix", lib = true }
"#,
)
.file(
"src/lib.rs",
r#"
pub fn foo() {
bar_baz::exists();
env!("CARGO_BIN_DIR_BAR_BAZ");
let _b = include_bytes!(env!("CARGO_BIN_FILE_BAR_BAZ_baz-suffix"));
}
"#,
)
.file(
"bar/Cargo.toml",
r#"
[package]
name = "bar-baz"
version = "0.5.0"
authors = []
[[bin]]
name = "bar"
[[bin]]
name = "baz-suffix"
"#,
)
.file("bar/src/main.rs", "fn main() {}")
.file("bar/src/lib.rs", "pub fn exists() {}")
.build();
p.cargo("build -Z unstable-options -Z bindeps")
.masquerade_as_nightly_cargo()
.with_stderr(
"\
[COMPILING] bar-baz v0.5.0 ([CWD]/bar)
[COMPILING] foo [..]
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]",
)
.run();

assert!(
!p.bin("bar").is_file(),
"artifacts are located in their own directory, exclusively, and won't be lifted up"
);
assert_artifact_executable_output(&p, "debug", "bar", "baz_suffix");
}

#[cargo_test]
fn allow_artifact_and_no_artifact_dep_to_same_package_within_different_dep_categories() {
let p = project()
Expand Down Expand Up @@ -999,7 +1105,7 @@ fn assert_artifact_executable_output(
dep_name: &str,
bin_name: &str,
) {
#[cfg(not(windows))]
#[cfg(any(not(windows), target_env = "gnu"))]
{
assert_eq!(
p.glob(format!(
Expand Down

0 comments on commit d288fa9

Please sign in to comment.