Skip to content

Commit 4ac865d

Browse files
authored
Rework cargo-test-support & testsuite to use CARGO_BIN_EXE_* for Cargo (#15692)
### What does this PR try to resolve? This PR reworks `cargo-test-support` and `testsuite` to use Snapbox's [`cargo_bin!()`](https://docs.rs/snapbox/latest/snapbox/cmd/macro.cargo_bin.html) instead of [`cargo_bin()`](https://docs.rs/snapbox/latest/snapbox/cmd/fn.cargo_bin.html) which makes assumptions about the structure of Cargo's build directory. `cargo_bin!()` uses `CARGO_BIN_EXE_*` for locating the `cargo` binary which should be more resilient to directory/layout changes. Linking a relevant Zulip discussion [here](https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/cargo_bin_exe.20and.20tests/with/513638220)[#t-cargo > cargo_bin_exe and tests](https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/cargo_bin_exe.20and.20tests/with/513638220) As shown in that link, we could make these variables available at runtime and not need to do this. However, `cargo-test-support`, as an API, is a bit weird in that it is baking in support for one specific binary. This can be confusing for callers and makes it more annoying for callers provide their own `fn cargo`, e.g. see crate-ci/cargo-fixit#7 ### Implementation Notes `cargo_bin!()` only works when being called from the `testsuite` as it's only set when executing integration tests and `cargo-test-support` is a regular crate. To make this change, I introduced an extension trait `CargoProjectExt` in `testsuite` for running `.cargo()` and implemented it on `Project`. In `cargo-test-support` other functionality relies on `.cargo()` so these also needed to be moved to `testsuite` * [`src/tools.rs`](https://github.com/rust-lang/cargo/blob/master/crates/cargo-test-support/src/tools.rs) * Parts [`src/cross_compile`](https://github.com/rust-lang/cargo/blob/master/crates/cargo-test-support/src/cross_compile.rs) * I had to split this up unfortunately, as `disabled()` requires running Cargo to check if we should disable cross compile tests. * Other fns in `cross_compile` are used in `cargo-test-support` so moving everything to `testsuite` would have ended up requiring moving many things to test suite. ### How to test and review this PR? I'd definitely recommend reviewing commit by commit. There are a lot of diffs due to the nature of reorganizing things. I did my best to split things things into smaller PRs but they still contain a lot of `use` statement diffs. r? @epage
2 parents 7388d86 + 02b56d4 commit 4ac865d

File tree

499 files changed

+992
-957
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

499 files changed

+992
-957
lines changed

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ cargo-credential-macos-keychain = { version = "0.4.15", path = "credential/cargo
3232
cargo-credential-wincred = { version = "0.4.15", path = "credential/cargo-credential-wincred" }
3333
cargo-platform = { path = "crates/cargo-platform", version = "0.3.0" }
3434
cargo-test-macro = { version = "0.4.4", path = "crates/cargo-test-macro" }
35-
cargo-test-support = { version = "0.7.5", path = "crates/cargo-test-support" }
35+
cargo-test-support = { version = "0.8.0", path = "crates/cargo-test-support" }
3636
cargo-util = { version = "0.2.22", path = "crates/cargo-util" }
3737
cargo-util-schemas = { version = "0.9.0", path = "crates/cargo-util-schemas" }
3838
cargo_metadata = "0.19.1"

crates/cargo-test-support/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "cargo-test-support"
3-
version = "0.7.5"
3+
version = "0.8.0"
44
edition.workspace = true
55
rust-version = "1.87" # MSRV:1
66
license.workspace = true

crates/cargo-test-support/src/cross_compile.rs

Lines changed: 1 addition & 205 deletions
Original file line numberDiff line numberDiff line change
@@ -9,189 +9,7 @@
99
//!
1010
//! These tests are all disabled on rust-lang/rust's CI, but run in Cargo's CI.
1111
12-
use crate::{basic_manifest, main_file, project};
13-
use cargo_util::ProcessError;
1412
use std::env;
15-
use std::fmt::Write;
16-
use std::process::{Command, Output};
17-
use std::sync::atomic::{AtomicBool, Ordering};
18-
use std::sync::Once;
19-
20-
/// Whether or not the resulting cross binaries can run on the host.
21-
static CAN_RUN_ON_HOST: AtomicBool = AtomicBool::new(false);
22-
23-
pub fn disabled() -> bool {
24-
// First, disable if requested.
25-
match env::var("CFG_DISABLE_CROSS_TESTS") {
26-
Ok(ref s) if *s == "1" => return true,
27-
_ => {}
28-
}
29-
30-
// It requires setting `target.linker` for cross-compilation to work on aarch64,
31-
// so not going to bother now.
32-
if cfg!(all(target_arch = "aarch64", target_os = "linux")) {
33-
return true;
34-
}
35-
36-
// Cross tests are only tested to work on macos, linux, and MSVC windows.
37-
if !(cfg!(target_os = "macos") || cfg!(target_os = "linux") || cfg!(target_env = "msvc")) {
38-
return true;
39-
}
40-
41-
// It's not particularly common to have a cross-compilation setup, so
42-
// try to detect that before we fail a bunch of tests through no fault
43-
// of the user.
44-
static CAN_BUILD_CROSS_TESTS: AtomicBool = AtomicBool::new(false);
45-
static CHECK: Once = Once::new();
46-
47-
let cross_target = alternate();
48-
49-
let run_cross_test = || -> anyhow::Result<Output> {
50-
let p = project()
51-
.at("cross_test")
52-
.file("Cargo.toml", &basic_manifest("cross_test", "1.0.0"))
53-
.file("src/main.rs", &main_file(r#""testing!""#, &[]))
54-
.build();
55-
56-
let build_result = p
57-
.cargo("build --target")
58-
.arg(&cross_target)
59-
.exec_with_output();
60-
61-
if build_result.is_ok() {
62-
CAN_BUILD_CROSS_TESTS.store(true, Ordering::SeqCst);
63-
}
64-
65-
let result = p
66-
.cargo("run --target")
67-
.arg(&cross_target)
68-
.exec_with_output();
69-
70-
if result.is_ok() {
71-
CAN_RUN_ON_HOST.store(true, Ordering::SeqCst);
72-
}
73-
build_result
74-
};
75-
76-
CHECK.call_once(|| {
77-
drop(run_cross_test());
78-
});
79-
80-
if CAN_BUILD_CROSS_TESTS.load(Ordering::SeqCst) {
81-
// We were able to compile a simple project, so the user has the
82-
// necessary `std::` bits installed. Therefore, tests should not
83-
// be disabled.
84-
return false;
85-
}
86-
87-
// We can't compile a simple cross project. We want to warn the user
88-
// by failing a single test and having the remainder of the cross tests
89-
// pass. We don't use `std::sync::Once` here because panicking inside its
90-
// `call_once` method would poison the `Once` instance, which is not what
91-
// we want.
92-
static HAVE_WARNED: AtomicBool = AtomicBool::new(false);
93-
94-
if HAVE_WARNED.swap(true, Ordering::SeqCst) {
95-
// We are some other test and somebody else is handling the warning.
96-
// Just disable the current test.
97-
return true;
98-
}
99-
100-
// We are responsible for warning the user, which we do by panicking.
101-
let mut message = format!(
102-
"
103-
Cannot cross compile to {}.
104-
105-
This failure can be safely ignored. If you would prefer to not see this
106-
failure, you can set the environment variable CFG_DISABLE_CROSS_TESTS to \"1\".
107-
108-
Alternatively, you can install the necessary libraries to enable cross
109-
compilation tests. Cross compilation tests depend on your host platform.
110-
",
111-
cross_target
112-
);
113-
114-
if cfg!(target_os = "linux") {
115-
message.push_str(
116-
"
117-
Linux cross tests target i686-unknown-linux-gnu, which requires the ability to
118-
build and run 32-bit targets. This requires the 32-bit libraries to be
119-
installed. For example, on Ubuntu, run `sudo apt install gcc-multilib` to
120-
install the necessary libraries.
121-
",
122-
);
123-
} else if cfg!(all(target_os = "macos", target_arch = "aarch64")) {
124-
message.push_str(
125-
"
126-
macOS on aarch64 cross tests to target x86_64-apple-darwin.
127-
This should be natively supported via Xcode, nothing additional besides the
128-
rustup target should be needed.
129-
",
130-
);
131-
} else if cfg!(target_os = "macos") {
132-
message.push_str(
133-
"
134-
macOS on x86_64 cross tests to target x86_64-apple-ios, which requires the iOS
135-
SDK to be installed. This should be included with Xcode automatically. If you
136-
are using the Xcode command line tools, you'll need to install the full Xcode
137-
app (from the Apple App Store), and switch to it with this command:
138-
139-
sudo xcode-select --switch /Applications/Xcode.app/Contents/Developer
140-
141-
Some cross-tests want to *run* the executables on the host. These tests will
142-
be ignored if this is not possible. On macOS, this means you need an iOS
143-
simulator installed to run these tests. To install a simulator, open Xcode, go
144-
to preferences > Components, and download the latest iOS simulator.
145-
",
146-
);
147-
} else if cfg!(target_os = "windows") {
148-
message.push_str(
149-
"
150-
Windows cross tests target i686-pc-windows-msvc, which requires the ability
151-
to build and run 32-bit targets. This should work automatically if you have
152-
properly installed Visual Studio build tools.
153-
",
154-
);
155-
} else {
156-
// The check at the top should prevent this.
157-
panic!("platform should have been skipped");
158-
}
159-
160-
let rustup_available = Command::new("rustup").output().is_ok();
161-
if rustup_available {
162-
write!(
163-
message,
164-
"
165-
Make sure that the appropriate `rustc` target is installed with rustup:
166-
167-
rustup target add {}
168-
",
169-
cross_target
170-
)
171-
.unwrap();
172-
} else {
173-
write!(
174-
message,
175-
"
176-
rustup does not appear to be installed. Make sure that the appropriate
177-
`rustc` target is installed for the target `{}`.
178-
",
179-
cross_target
180-
)
181-
.unwrap();
182-
}
183-
184-
// Show the actual error message.
185-
match run_cross_test() {
186-
Ok(_) => message.push_str("\nUh oh, second run succeeded?\n"),
187-
Err(err) => match err.downcast_ref::<ProcessError>() {
188-
Some(proc_err) => write!(message, "\nTest error: {}\n", proc_err).unwrap(),
189-
None => write!(message, "\nUnexpected non-process error: {}\n", err).unwrap(),
190-
},
191-
}
192-
193-
panic!("{}", message);
194-
}
19513

19614
/// The arch triple of the test-running host.
19715
pub fn native() -> &'static str {
@@ -252,31 +70,9 @@ pub fn unused() -> &'static str {
25270
"wasm32-unknown-unknown"
25371
}
25472

255-
/// Whether or not the host can run cross-compiled executables.
256-
pub fn can_run_on_host() -> bool {
257-
if disabled() {
258-
return false;
259-
}
260-
// macos is currently configured to cross compile to x86_64-apple-ios
261-
// which requires a simulator to run. Azure's CI image appears to have the
262-
// SDK installed, but are not configured to launch iOS images with a
263-
// simulator.
264-
if cfg!(target_os = "macos") {
265-
if CAN_RUN_ON_HOST.load(Ordering::SeqCst) {
266-
return true;
267-
} else {
268-
println!("Note: Cannot run on host, skipping.");
269-
return false;
270-
}
271-
} else {
272-
assert!(CAN_RUN_ON_HOST.load(Ordering::SeqCst));
273-
return true;
274-
}
275-
}
276-
27773
/// Check if the given target has been installed.
27874
///
279-
/// Generally [`disabled`] should be used to check if cross-compilation is allowed.
75+
/// Generally `testsuite::utils::cross_compile::disabled` should be used to check if cross-compilation is allowed.
28076
/// And [`alternate`] to get the cross target.
28177
///
28278
/// You should only use this as a last resort to skip tests,

crates/cargo-test-support/src/lib.rs

Lines changed: 1 addition & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,11 @@ pub mod install;
110110
pub mod paths;
111111
pub mod publish;
112112
pub mod registry;
113-
pub mod tools;
114113

115114
pub mod prelude {
116115
pub use crate::cargo_test;
117116
pub use crate::paths::CargoPathExt;
118117
pub use crate::ArgLineCommandExt;
119-
pub use crate::CargoCommandExt;
120118
pub use crate::ChannelChangerCommandExt;
121119
pub use crate::TestEnvCommandExt;
122120
pub use snapbox::IntoData;
@@ -491,28 +489,6 @@ impl Project {
491489
execs().with_process_builder(p)
492490
}
493491

494-
/// Creates a `ProcessBuilder` to run cargo.
495-
///
496-
/// Arguments can be separated by spaces.
497-
///
498-
/// For `cargo run`, see [`Project::rename_run`].
499-
///
500-
/// # Example:
501-
///
502-
/// ```no_run
503-
/// # let p = cargo_test_support::project().build();
504-
/// p.cargo("build --bin foo").run();
505-
/// ```
506-
pub fn cargo(&self, cmd: &str) -> Execs {
507-
let cargo = cargo_exe();
508-
let mut execs = self.process(&cargo);
509-
if let Some(ref mut p) = execs.process_builder {
510-
p.env("CARGO", cargo);
511-
p.arg_line(cmd);
512-
}
513-
execs
514-
}
515-
516492
/// Safely run a process after `cargo build`.
517493
///
518494
/// Windows has a problem where a process cannot be reliably
@@ -621,11 +597,6 @@ pub fn main_file(println: &str, externed_deps: &[&str]) -> String {
621597
buf
622598
}
623599

624-
/// Path to the cargo binary
625-
pub fn cargo_exe() -> PathBuf {
626-
snapbox::cmd::cargo_bin("cargo")
627-
}
628-
629600
/// This is the raw output from the process.
630601
///
631602
/// This is similar to `std::process::Output`, however the `status` is
@@ -643,8 +614,8 @@ pub struct RawOutput {
643614
///
644615
/// Construct with
645616
/// - [`execs`]
646-
/// - [`cargo_process`]
647617
/// - [`Project`] methods
618+
/// - `cargo_process` in testsuite
648619
#[must_use]
649620
#[derive(Clone)]
650621
pub struct Execs {
@@ -1495,21 +1466,6 @@ impl TestEnvCommandExt for snapbox::cmd::Command {
14951466
}
14961467
}
14971468

1498-
/// Test the cargo command
1499-
pub trait CargoCommandExt {
1500-
fn cargo_ui() -> Self;
1501-
}
1502-
1503-
impl CargoCommandExt for snapbox::cmd::Command {
1504-
fn cargo_ui() -> Self {
1505-
Self::new(cargo_exe())
1506-
.with_assert(compare::assert_ui())
1507-
.env("CARGO_TERM_COLOR", "always")
1508-
.env("CARGO_TERM_HYPERLINKS", "true")
1509-
.test_env()
1510-
}
1511-
}
1512-
15131469
/// Add a list of arguments as a line
15141470
pub trait ArgLineCommandExt: Sized {
15151471
fn arg_line(mut self, s: &str) -> Self {
@@ -1547,15 +1503,6 @@ impl ArgLineCommandExt for snapbox::cmd::Command {
15471503
}
15481504
}
15491505

1550-
/// Run `cargo $arg_line`, see [`Execs`]
1551-
pub fn cargo_process(arg_line: &str) -> Execs {
1552-
let cargo = cargo_exe();
1553-
let mut p = process(&cargo);
1554-
p.env("CARGO", cargo);
1555-
p.arg_line(arg_line);
1556-
execs().with_process_builder(p)
1557-
}
1558-
15591506
/// Run `git $arg_line`, see [`ProcessBuilder`]
15601507
pub fn git_process(arg_line: &str) -> ProcessBuilder {
15611508
let mut p = process("git");

crates/cargo-test-support/src/publish.rs

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,29 +6,6 @@
66
//! # use cargo_test_support::registry::RegistryBuilder;
77
//! # use cargo_test_support::publish::validate_upload;
88
//! # use cargo_test_support::project;
9-
//! // This replaces `registry::init()` and must be called before `Package::new().publish()`
10-
//! let registry = RegistryBuilder::new().http_api().http_index().build();
11-
//!
12-
//! let p = project()
13-
//! .file(
14-
//! "Cargo.toml",
15-
//! r#"
16-
//! [package]
17-
//! name = "foo"
18-
//! version = "0.0.1"
19-
//! edition = "2015"
20-
//! authors = []
21-
//! license = "MIT"
22-
//! description = "foo"
23-
//! "#,
24-
//! )
25-
//! .file("src/main.rs", "fn main() {}")
26-
//! .build();
27-
//!
28-
//! p.cargo("publish --no-verify")
29-
//! .replace_crates_io(registry.index_url())
30-
//! .run();
31-
//!
329
//! validate_upload(
3310
//! r#"
3411
//! {

crates/cargo-test-support/src/registry.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
//! "#)
4040
//! .build();
4141
//!
42-
//! p.cargo("run").with_stdout_data(str!["24"]).run();
42+
//! // p.cargo("run").with_stdout_data(str!["24"]).run();
4343
//! ```
4444
4545
use crate::git::repo;

0 commit comments

Comments
 (0)