-
-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(build-std): include artifact dependency targets #17074
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| use std::path::{Path, PathBuf}; | ||
|
|
||
| use crate::prelude::*; | ||
| use crate::utils::cross_compile::disabled as cross_compile_disabled; | ||
| use cargo_test_support::ProjectBuilder; | ||
| use cargo_test_support::cross_compile; | ||
| use cargo_test_support::registry::{Dependency, Package}; | ||
|
|
@@ -518,6 +519,160 @@ fn depend_same_as_std() { | |
| p.cargo("build -v").build_std(&setup).target_host().run(); | ||
| } | ||
|
|
||
| #[cargo_test(build_std_mock)] | ||
| fn artifact_dep_target_builds_std_for_unrequested_target() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please see https://epage.github.io/dev/pr-style/#c-test (note our contrib guide says this as policy but that goes into more detail) |
||
| if cross_compile_disabled() { | ||
| return; | ||
| } | ||
| let target = cross_compile::alternate(); | ||
| let setup = setup(); | ||
|
|
||
| let p = project() | ||
| .file( | ||
| "Cargo.toml", | ||
| &format!( | ||
| r#" | ||
| [package] | ||
| name = "foo" | ||
| version = "0.0.1" | ||
| edition = "2021" | ||
| resolver = "2" | ||
|
|
||
| [dependencies] | ||
| bindep = {{ path = "bindep", artifact = "bin", target = "{target}" }} | ||
| "#, | ||
| ), | ||
| ) | ||
| .file( | ||
| "src/lib.rs", | ||
| r#" | ||
| pub fn foo() { | ||
| std::custom_api(); | ||
| let _bin = include_bytes!(env!("CARGO_BIN_FILE_BINDEP")); | ||
| } | ||
| "#, | ||
| ) | ||
| .file( | ||
| "bindep/Cargo.toml", | ||
| r#" | ||
| [package] | ||
| name = "bindep" | ||
| version = "0.0.1" | ||
| edition = "2021" | ||
| "#, | ||
| ) | ||
| .file( | ||
| "bindep/src/main.rs", | ||
| r#" | ||
| fn main() { | ||
| std::custom_api(); | ||
| } | ||
| "#, | ||
| ) | ||
| .build(); | ||
|
|
||
| p.cargo("check -v -Z bindeps") | ||
| .masquerade_as_nightly_cargo(&["bindeps"]) | ||
| .build_std(&setup) | ||
| .target_host() | ||
| .with_stderr_data(str![[r#" | ||
| ... | ||
| [RUNNING] `[..]rustc --crate-name std [..]--target [ALT_TARGET][..]` | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add an extra check that the host std is also built? |
||
| ... | ||
| [RUNNING] `[..]rustc --crate-name bindep [..]--target [ALT_TARGET][..]` | ||
| ... | ||
| "#]]) | ||
| .run(); | ||
| } | ||
|
|
||
| #[cargo_test(build_std_mock)] | ||
| fn inactive_artifact_dep_target_does_not_build_std() { | ||
| if cross_compile_disabled() { | ||
| return; | ||
| } | ||
| let target = cross_compile::alternate(); | ||
| let setup = setup(); | ||
|
|
||
| let p = project() | ||
| .file( | ||
| "Cargo.toml", | ||
| &format!( | ||
| r#" | ||
| [package] | ||
| name = "foo" | ||
| version = "0.0.1" | ||
| edition = "2021" | ||
| resolver = "2" | ||
|
|
||
| [dependencies] | ||
| bindep = {{ path = "bindep", artifact = "bin", target = "{target}", optional = true }} | ||
| "#, | ||
| ), | ||
| ) | ||
| .file("src/lib.rs", "pub fn foo() { std::custom_api(); }") | ||
| .file( | ||
| "bindep/Cargo.toml", | ||
| r#" | ||
| [package] | ||
| name = "bindep" | ||
| version = "0.0.1" | ||
| edition = "2021" | ||
| "#, | ||
| ) | ||
| .file("bindep/src/main.rs", "fn main() { std::custom_api(); }") | ||
| .build(); | ||
|
|
||
| p.cargo("check -v -Z bindeps") | ||
| .masquerade_as_nightly_cargo(&["bindeps"]) | ||
| .build_std(&setup) | ||
| .target_host() | ||
| .with_stderr_does_not_contain(str![[r#" | ||
| [RUNNING] `[..]rustc --crate-name std [..]--target [ALT_TARGET][..]` | ||
| "#]]) | ||
| .run(); | ||
| } | ||
|
|
||
| #[cargo_test(build_std_mock)] | ||
| fn per_package_target_builds_std_for_manifest_target() { | ||
| if cross_compile_disabled() { | ||
| return; | ||
| } | ||
| let target = cross_compile::alternate(); | ||
| let setup = setup(); | ||
|
|
||
| let p = project() | ||
| .file( | ||
| "Cargo.toml", | ||
| &format!( | ||
| r#" | ||
| cargo-features = ["per-package-target"] | ||
|
|
||
| [package] | ||
| name = "foo" | ||
| version = "0.0.1" | ||
| edition = "2021" | ||
| default-target = "{target}" | ||
| "#, | ||
| ), | ||
| ) | ||
| .file("src/lib.rs", "pub fn foo() { std::custom_api(); }") | ||
| .build(); | ||
|
|
||
| let mut cargo = p.cargo("check -v"); | ||
| enable_build_std(&mut cargo, &setup); | ||
| cargo | ||
| .arg("-Zbuild-std") | ||
| .masquerade_as_nightly_cargo(&["build-std", "per-package-target"]) | ||
| .with_stderr_data(str![[r#" | ||
| ... | ||
| [RUNNING] `[..]rustc --crate-name std [..]--target [ALT_TARGET][..]` | ||
| ... | ||
| [RUNNING] `[..]rustc --crate-name foo [..]--target [ALT_TARGET][..]` | ||
| ... | ||
| "#]]) | ||
| .run(); | ||
| } | ||
|
|
||
| #[cargo_test(build_std_mock)] | ||
| fn test() { | ||
| let setup = setup(); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit unfortunate that the real target of a package is only figured out during unit generation. As is, this leads to std being resolved with more targets than required if a package has both default and forced targets set.
Right now it would be feasible to do the main resolve, generate its units, then resolve std for the right targets and attach those units. But with #16675, unit generation happens in one step and so will require both resolves ahead of time.
I think that's fine for now as std doesn't actually get built more times than required. per-pkg-target is unstable (as is build-std), and I'd prefer if #16675 is merged before looking for improvements here