Skip to content

Commit bffece8

Browse files
authored
refactor: separate "global" mode from CompileMode (#15601)
### What does this PR try to resolve? This separates the concern of two different "mode". - UserIntent: focus on the overall goal of the build - CompileMode: the actual compile operation for each unit (I'd like to rename it to something else in the future, such as CompileAction) This is a preparation of adding `-Zno-link`/`-Zlink-only` support, which we'll have `CompileMode::Link` but that doesn't make sense to show up in `UserIntent`. ### How should we test and review this PR? It should have no functional change. ### Additional information
2 parents 36daa73 + 2fc7e6e commit bffece8

File tree

27 files changed

+204
-169
lines changed

27 files changed

+204
-169
lines changed

src/bin/cargo/commands/bench.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
6161
let ws = args.workspace(gctx)?;
6262

6363
let mut compile_opts =
64-
args.compile_options(gctx, CompileMode::Bench, Some(&ws), ProfileChecking::Custom)?;
64+
args.compile_options(gctx, UserIntent::Bench, Some(&ws), ProfileChecking::Custom)?;
6565

6666
compile_opts.build_config.requested_profile =
6767
args.get_profile_name("bench", ProfileChecking::Custom)?;

src/bin/cargo/commands/build.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ pub fn cli() -> Command {
4949
pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
5050
let ws = args.workspace(gctx)?;
5151
let mut compile_opts =
52-
args.compile_options(gctx, CompileMode::Build, Some(&ws), ProfileChecking::Custom)?;
52+
args.compile_options(gctx, UserIntent::Build, Some(&ws), ProfileChecking::Custom)?;
5353

5454
if let Some(artifact_dir) = args.value_of_path("artifact-dir", gctx) {
5555
// If the user specifies `--artifact-dir`, use that

src/bin/cargo/commands/check.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,9 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
5050
args.get_one::<String>("profile").map(String::as_str),
5151
Some("test")
5252
);
53-
let mode = CompileMode::Check { test };
53+
let intent = UserIntent::Check { test };
5454
let compile_opts =
55-
args.compile_options(gctx, mode, Some(&ws), ProfileChecking::LegacyTestOnly)?;
55+
args.compile_options(gctx, intent, Some(&ws), ProfileChecking::LegacyTestOnly)?;
5656

5757
ops::compile(&ws, &compile_opts)?;
5858
Ok(())

src/bin/cargo/commands/doc.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,12 @@ pub fn cli() -> Command {
4848

4949
pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
5050
let ws = args.workspace(gctx)?;
51-
let mode = CompileMode::Doc {
51+
let intent = UserIntent::Doc {
5252
deps: !args.flag("no-deps"),
5353
json: false,
5454
};
55-
let mut compile_opts = args.compile_options(gctx, mode, Some(&ws), ProfileChecking::Custom)?;
55+
let mut compile_opts =
56+
args.compile_options(gctx, intent, Some(&ws), ProfileChecking::Custom)?;
5657
compile_opts.rustdoc_document_private_items = args.flag("document-private-items");
5758

5859
let doc_opts = DocOptions {

src/bin/cargo/commands/fix.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
6767
args.get_one::<String>("profile").map(String::as_str),
6868
Some("test")
6969
);
70-
let mode = CompileMode::Check { test };
70+
let intent = UserIntent::Check { test };
7171

7272
// Unlike other commands default `cargo fix` to all targets to fix as much
7373
// code as we can.
@@ -79,7 +79,8 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
7979
let lockfile_path = args.lockfile_path(gctx)?;
8080
ws.set_requested_lockfile_path(lockfile_path.clone());
8181

82-
let mut opts = args.compile_options(gctx, mode, Some(&ws), ProfileChecking::LegacyTestOnly)?;
82+
let mut opts =
83+
args.compile_options(gctx, intent, Some(&ws), ProfileChecking::LegacyTestOnly)?;
8384

8485
let edition = args.flag("edition") || args.flag("edition-idioms");
8586
if !opts.filter.is_specific() && edition {

src/bin/cargo/commands/install.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
216216

217217
let mut compile_opts = args.compile_options(
218218
gctx,
219-
CompileMode::Build,
219+
UserIntent::Build,
220220
workspace.as_ref(),
221221
ProfileChecking::Custom,
222222
)?;

src/bin/cargo/commands/run.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
5151
let ws = args.workspace(gctx)?;
5252

5353
let mut compile_opts =
54-
args.compile_options(gctx, CompileMode::Build, Some(&ws), ProfileChecking::Custom)?;
54+
args.compile_options(gctx, UserIntent::Build, Some(&ws), ProfileChecking::Custom)?;
5555

5656
// Disallow `spec` to be an glob pattern
5757
if let Packages::Packages(opt_in) = &compile_opts.spec {
@@ -180,7 +180,7 @@ pub fn exec_manifest_command(gctx: &mut GlobalContext, cmd: &str, args: &[OsStri
180180
}
181181

182182
let mut compile_opts =
183-
cargo::ops::CompileOptions::new(gctx, cargo::core::compiler::CompileMode::Build)?;
183+
cargo::ops::CompileOptions::new(gctx, cargo::core::compiler::UserIntent::Build)?;
184184
compile_opts.spec = cargo::ops::Packages::Default;
185185

186186
cargo::ops::run(&ws, &compile_opts, args).map_err(|err| to_run_error(gctx, err))

src/bin/cargo/commands/rustc.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,15 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
6464
// This is a legacy behavior that changes the behavior based on the profile.
6565
// If we want to support this more formally, I think adding a --mode flag
6666
// would be warranted.
67-
let mode = match args.get_one::<String>("profile").map(String::as_str) {
68-
Some("test") => CompileMode::Test,
69-
Some("bench") => CompileMode::Bench,
70-
Some("check") => CompileMode::Check { test: false },
71-
_ => CompileMode::Build,
67+
let intent = match args.get_one::<String>("profile").map(String::as_str) {
68+
Some("test") => UserIntent::Test,
69+
Some("bench") => UserIntent::Bench,
70+
Some("check") => UserIntent::Check { test: false },
71+
_ => UserIntent::Build,
7272
};
7373
let mut compile_opts = args.compile_options_for_single_package(
7474
gctx,
75-
mode,
75+
intent,
7676
Some(&ws),
7777
ProfileChecking::LegacyRustc,
7878
)?;

src/bin/cargo/commands/rustdoc.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
6464

6565
let mut compile_opts = args.compile_options_for_single_package(
6666
gctx,
67-
CompileMode::Doc {
67+
UserIntent::Doc {
6868
deps: false,
6969
json: matches!(output_format, OutputFormat::Json),
7070
},

src/bin/cargo/commands/test.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
7272
let ws = args.workspace(gctx)?;
7373

7474
let mut compile_opts =
75-
args.compile_options(gctx, CompileMode::Test, Some(&ws), ProfileChecking::Custom)?;
75+
args.compile_options(gctx, UserIntent::Test, Some(&ws), ProfileChecking::Custom)?;
7676

7777
compile_opts.build_config.requested_profile =
7878
args.get_profile_name("test", ProfileChecking::Custom)?;
@@ -95,7 +95,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
9595
if no_run {
9696
return Err(anyhow::format_err!("Can't skip running doc tests with --no-run").into());
9797
}
98-
compile_opts.build_config.mode = CompileMode::Doctest;
98+
compile_opts.build_config.intent = UserIntent::Doctest;
9999
compile_opts.filter = ops::CompileFilter::lib_only();
100100
} else if test_name.is_some() && !compile_opts.filter.is_specific() {
101101
// If arg `TESTNAME` is provided, assumed that the user knows what

src/cargo/core/compiler/build_config.rs

Lines changed: 73 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ pub struct BuildConfig {
2121
pub keep_going: bool,
2222
/// Build profile
2323
pub requested_profile: InternedString,
24-
/// The mode we are compiling in.
25-
pub mode: CompileMode,
24+
/// The intent we are compiling in.
25+
pub intent: UserIntent,
2626
/// `true` to print stdout in JSON format (for machine reading).
2727
pub message_format: MessageFormat,
2828
/// Force Cargo to do a full rebuild and treat each target as changed.
@@ -72,7 +72,7 @@ impl BuildConfig {
7272
jobs: Option<JobsConfig>,
7373
keep_going: bool,
7474
requested_targets: &[String],
75-
mode: CompileMode,
75+
intent: UserIntent,
7676
) -> CargoResult<BuildConfig> {
7777
let cfg = gctx.build_config()?;
7878
let requested_kinds = CompileKind::from_requested_targets(gctx, requested_targets)?;
@@ -117,7 +117,7 @@ impl BuildConfig {
117117
jobs,
118118
keep_going,
119119
requested_profile: InternedString::new("dev"),
120-
mode,
120+
intent,
121121
message_format: MessageFormat::Human,
122122
force_rebuild: false,
123123
build_plan: false,
@@ -138,10 +138,6 @@ impl BuildConfig {
138138
matches!(self.message_format, MessageFormat::Json { .. })
139139
}
140140

141-
pub fn test(&self) -> bool {
142-
self.mode == CompileMode::Test || self.mode == CompileMode::Bench
143-
}
144-
145141
pub fn single_requested_kind(&self) -> CargoResult<CompileKind> {
146142
match self.requested_kinds.len() {
147143
1 => Ok(self.requested_kinds[0]),
@@ -167,37 +163,28 @@ pub enum MessageFormat {
167163
Short,
168164
}
169165

170-
/// The general "mode" for what to do.
171-
///
172-
/// This is used for two purposes. The commands themselves pass this in to
173-
/// `compile_ws` to tell it the general execution strategy. This influences
174-
/// the default targets selected. The other use is in the `Unit` struct
175-
/// to indicate what is being done with a specific target.
166+
/// The specific action to be performed on each `Unit` of work.
176167
#[derive(Clone, Copy, PartialEq, Debug, Eq, Hash, PartialOrd, Ord)]
177168
pub enum CompileMode {
178-
/// A target being built for a test.
169+
/// Test with `rustc`.
179170
Test,
180-
/// Building a target with `rustc` (lib or bin).
171+
/// Compile with `rustc`.
181172
Build,
182-
/// Building a target with `rustc` to emit `rmeta` metadata only. If
183-
/// `test` is true, then it is also compiled with `--test` to check it like
173+
/// Type-check with `rustc` by emitting `rmeta` metadata only.
174+
///
175+
/// If `test` is true, then it is also compiled with `--test` to check it like
184176
/// a test.
185177
Check { test: bool },
186-
/// Used to indicate benchmarks should be built. This is not used in
187-
/// `Unit`, because it is essentially the same as `Test` (indicating
188-
/// `--test` should be passed to rustc) and by using `Test` instead it
189-
/// allows some de-duping of Units to occur.
190-
Bench,
191-
/// A target that will be documented with `rustdoc`.
192-
178+
/// Document with `rustdoc`.
179+
///
193180
/// If `deps` is true, then it will also document all dependencies.
194181
/// if `json` is true, the documentation output is in json format.
195182
Doc { deps: bool, json: bool },
196-
/// A target that will be tested with `rustdoc`.
183+
/// Test with `rustdoc`.
197184
Doctest,
198-
/// An example or library that will be scraped for function calls by `rustdoc`.
185+
/// Scrape for function calls by `rustdoc`.
199186
Docscrape,
200-
/// A marker for Units that represent the execution of a `build.rs` script.
187+
/// Execute the binary built from the `build.rs` script.
201188
RunCustomBuild,
202189
}
203190

@@ -211,7 +198,6 @@ impl ser::Serialize for CompileMode {
211198
Test => "test".serialize(s),
212199
Build => "build".serialize(s),
213200
Check { .. } => "check".serialize(s),
214-
Bench => "bench".serialize(s),
215201
Doc { .. } => "doc".serialize(s),
216202
Doctest => "doctest".serialize(s),
217203
Docscrape => "docscrape".serialize(s),
@@ -246,19 +232,13 @@ impl CompileMode {
246232
pub fn is_any_test(self) -> bool {
247233
matches!(
248234
self,
249-
CompileMode::Test
250-
| CompileMode::Bench
251-
| CompileMode::Check { test: true }
252-
| CompileMode::Doctest
235+
CompileMode::Test | CompileMode::Check { test: true } | CompileMode::Doctest
253236
)
254237
}
255238

256239
/// Returns `true` if this is something that passes `--test` to rustc.
257240
pub fn is_rustc_test(self) -> bool {
258-
matches!(
259-
self,
260-
CompileMode::Test | CompileMode::Bench | CompileMode::Check { test: true }
261-
)
241+
matches!(self, CompileMode::Test | CompileMode::Check { test: true })
262242
}
263243

264244
/// Returns `true` if this is the *execution* of a `build.rs` script.
@@ -271,9 +251,64 @@ impl CompileMode {
271251
/// Note that this also returns `true` for building libraries, so you also
272252
/// have to check the target.
273253
pub fn generates_executable(self) -> bool {
254+
matches!(self, CompileMode::Test | CompileMode::Build)
255+
}
256+
}
257+
258+
/// Represents the high-level operation requested by the user.
259+
///
260+
/// It determines which "Cargo targets" are selected by default and influences
261+
/// how they will be processed. This is derived from the Cargo command the user
262+
/// invoked (like `cargo build` or `cargo test`).
263+
///
264+
/// Unlike [`CompileMode`], which describes the specific compilation steps for
265+
/// individual units, [`UserIntent`] represents the overall goal of the build
266+
/// process as specified by the user.
267+
///
268+
/// For example, when a user runs `cargo test`, the intent is [`UserIntent::Test`],
269+
/// but this might result in multiple [`CompileMode`]s for different units.
270+
#[derive(Clone, Copy, Debug)]
271+
pub enum UserIntent {
272+
/// Build benchmark binaries, e.g., `cargo bench`
273+
Bench,
274+
/// Build binaries and libraray, e.g., `cargo run`, `cargo install`, `cargo build`.
275+
Build,
276+
/// Perform type-check, e.g., `cargo check`.
277+
Check { test: bool },
278+
/// Document packages.
279+
///
280+
/// If `deps` is true, then it will also document all dependencies.
281+
/// if `json` is true, the documentation output is in json format.
282+
Doc { deps: bool, json: bool },
283+
/// Build doctest binaries, e.g., `cargo test --doc`
284+
Doctest,
285+
/// Build test binaries, e.g., `cargo test`
286+
Test,
287+
}
288+
289+
impl UserIntent {
290+
/// Returns `true` if this is generating documentation.
291+
pub fn is_doc(self) -> bool {
292+
matches!(self, UserIntent::Doc { .. })
293+
}
294+
295+
/// Returns `true` if this is any type of test (test, benchmark, doc test, or
296+
/// check test).
297+
pub fn is_any_test(self) -> bool {
298+
matches!(
299+
self,
300+
UserIntent::Test
301+
| UserIntent::Bench
302+
| UserIntent::Check { test: true }
303+
| UserIntent::Doctest
304+
)
305+
}
306+
307+
/// Returns `true` if this is something that passes `--test` to rustc.
308+
pub fn is_rustc_test(self) -> bool {
274309
matches!(
275310
self,
276-
CompileMode::Test | CompileMode::Bench | CompileMode::Build
311+
UserIntent::Test | UserIntent::Bench | UserIntent::Check { test: true }
277312
)
278313
}
279314
}

src/cargo/core/compiler/build_context/target_info.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -574,7 +574,7 @@ impl TargetInfo {
574574
) -> CargoResult<(Vec<FileType>, Vec<CrateType>)> {
575575
match mode {
576576
CompileMode::Build => self.calc_rustc_outputs(target_kind, target_triple, gctx),
577-
CompileMode::Test | CompileMode::Bench => {
577+
CompileMode::Test => {
578578
match self.file_types(&CrateType::Bin, FileFlavor::Normal, target_triple)? {
579579
Some(fts) => Ok((fts, Vec::new())),
580580
None => Ok((Vec::new(), vec![CrateType::Bin])),

src/cargo/core/compiler/build_runner/compilation_files.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -493,10 +493,7 @@ impl<'a, 'gctx: 'a> CompilationFiles<'a, 'gctx> {
493493
flavor: FileFlavor::Normal,
494494
}]
495495
}
496-
CompileMode::Test
497-
| CompileMode::Build
498-
| CompileMode::Bench
499-
| CompileMode::Check { .. } => {
496+
CompileMode::Test | CompileMode::Build | CompileMode::Check { .. } => {
500497
let mut outputs = self.calc_outputs_rustc(unit, bcx)?;
501498
if bcx.build_config.sbom && bcx.gctx.cli_unstable().sbom {
502499
let sbom_files: Vec<_> = outputs

src/cargo/core/compiler/build_runner/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
183183
// any versioning (See https://github.com/rust-lang/cargo/issues/8461).
184184
// Therefore, we can end up with weird bugs and behaviours if we mix different
185185
// versions of these files.
186-
if self.bcx.build_config.mode.is_doc() {
186+
if self.bcx.build_config.intent.is_doc() {
187187
RustDocFingerprint::check_rustdoc_fingerprint(&self)?
188188
}
189189

src/cargo/core/compiler/custom_build.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,16 +28,16 @@
2828
//! [build script]: https://doc.rust-lang.org/nightly/cargo/reference/build-scripts.html
2929
//! [`TargetKind::CustomBuild`]: crate::core::manifest::TargetKind::CustomBuild
3030
//! [`UnitGraph`]: super::unit_graph::UnitGraph
31-
//! [`CompileMode::RunCustomBuild`]: super::CompileMode
31+
//! [`CompileMode::RunCustomBuild`]: crate::core::compiler::CompileMode::RunCustomBuild
3232
//! [instructions]: https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script
3333
3434
use super::{fingerprint, get_dynamic_search_path, BuildRunner, Job, Unit, Work};
3535
use crate::core::compiler::artifact;
3636
use crate::core::compiler::build_runner::UnitHash;
3737
use crate::core::compiler::fingerprint::DirtyReason;
3838
use crate::core::compiler::job_queue::JobState;
39+
use crate::core::compiler::CompileMode;
3940
use crate::core::{profiles::ProfileRoot, PackageId, Target};
40-
use crate::util::command_prelude::CompileMode;
4141
use crate::util::errors::CargoResult;
4242
use crate::util::internal;
4343
use crate::util::machine_message::{self, Message};

src/cargo/core/compiler/lto.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ fn calculate(
9797
let crate_types = match unit.mode {
9898
// Note: Doctest ignores LTO, but for now we'll compute it as-if it is
9999
// a Bin, in case it is ever supported in the future.
100-
CompileMode::Test | CompileMode::Bench | CompileMode::Doctest => vec![CrateType::Bin],
100+
CompileMode::Test | CompileMode::Doctest => vec![CrateType::Bin],
101101
// Notes on other modes:
102102
// - Check: Treat as the underlying type, it doesn't really matter.
103103
// - Doc: LTO is N/A for the Doc unit itself since rustdoc does not

src/cargo/core/compiler/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ use anyhow::{Context as _, Error};
6969
use lazycell::LazyCell;
7070
use tracing::{debug, instrument, trace};
7171

72+
pub use self::build_config::UserIntent;
7273
pub use self::build_config::{BuildConfig, CompileMode, MessageFormat, TimingOutput};
7374
pub use self::build_context::{
7475
BuildContext, FileFlavor, FileType, RustDocFingerprint, RustcTargetData, TargetInfo,

src/cargo/core/compiler/timings.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,6 @@ impl<'gctx> Timings<'gctx> {
168168
CompileMode::Build => {}
169169
CompileMode::Check { test: true } => target.push_str(" (check-test)"),
170170
CompileMode::Check { test: false } => target.push_str(" (check)"),
171-
CompileMode::Bench => target.push_str(" (bench)"),
172171
CompileMode::Doc { .. } => target.push_str(" (doc)"),
173172
CompileMode::Doctest => target.push_str(" (doc test)"),
174173
CompileMode::Docscrape => target.push_str(" (doc scrape)"),

0 commit comments

Comments
 (0)