Skip to content

Commit 6087566

Browse files
committed
Auto merge of #13792 - weihanglo:fix-in-rust-src, r=ehuss
fix(cargo-fix): dont fix into standard library
2 parents a1f8e45 + 0810627 commit 6087566

File tree

2 files changed

+370
-8
lines changed

2 files changed

+370
-8
lines changed

src/cargo/ops/fix.rs

+43-8
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ use rustfix::CodeFix;
5050
use semver::Version;
5151
use tracing::{debug, trace, warn};
5252

53+
use crate::core::compiler::CompileKind;
5354
use crate::core::compiler::RustcTargetData;
5455
use crate::core::resolver::features::{DiffMap, FeatureOpts, FeatureResolver, FeaturesFor};
5556
use crate::core::resolver::{HasDevUnits, Resolve, ResolveBehavior};
@@ -78,6 +79,14 @@ const EDITION_ENV_INTERNAL: &str = "__CARGO_FIX_EDITION";
7879
/// **Internal only.**
7980
/// For passing [`FixOptions::idioms`] through to cargo running in proxy mode.
8081
const IDIOMS_ENV_INTERNAL: &str = "__CARGO_FIX_IDIOMS";
82+
/// **Internal only.**
83+
/// The sysroot path.
84+
///
85+
/// This is for preventing `cargo fix` from fixing rust std/core libs. See
86+
///
87+
/// * <https://github.com/rust-lang/cargo/issues/9857>
88+
/// * <https://github.com/rust-lang/rust/issues/88514#issuecomment-2043469384>
89+
const SYSROOT_INTERNAL: &str = "__CARGO_FIX_RUST_SRC";
8190

8291
pub struct FixOptions {
8392
pub edition: bool,
@@ -97,6 +106,8 @@ pub fn fix(
97106
) -> CargoResult<()> {
98107
check_version_control(gctx, opts)?;
99108

109+
let mut target_data =
110+
RustcTargetData::new(original_ws, &opts.compile_opts.build_config.requested_kinds)?;
100111
if opts.edition {
101112
let specs = opts.compile_opts.spec.to_package_id_specs(&original_ws)?;
102113
let members: Vec<&Package> = original_ws
@@ -105,7 +116,7 @@ pub fn fix(
105116
.collect();
106117
migrate_manifests(original_ws, &members)?;
107118

108-
check_resolver_change(&original_ws, opts)?;
119+
check_resolver_change(&original_ws, &mut target_data, opts)?;
109120
}
110121
let mut ws = Workspace::new(&root_manifest, gctx)?;
111122
ws.set_resolve_honors_rust_version(Some(original_ws.resolve_honors_rust_version()));
@@ -129,6 +140,11 @@ pub fn fix(
129140
wrapper.env(IDIOMS_ENV_INTERNAL, "1");
130141
}
131142

143+
let sysroot = &target_data.info(CompileKind::Host).sysroot;
144+
if sysroot.is_dir() {
145+
wrapper.env(SYSROOT_INTERNAL, sysroot);
146+
}
147+
132148
*opts
133149
.compile_opts
134150
.build_config
@@ -395,7 +411,11 @@ fn add_feature_for_unused_deps(pkg: &Package, parent: &mut dyn toml_edit::TableL
395411
fixes
396412
}
397413

398-
fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<()> {
414+
fn check_resolver_change<'gctx>(
415+
ws: &Workspace<'gctx>,
416+
target_data: &mut RustcTargetData<'gctx>,
417+
opts: &FixOptions,
418+
) -> CargoResult<()> {
399419
let root = ws.root_maybe();
400420
match root {
401421
MaybePackage::Package(root_pkg) => {
@@ -422,12 +442,10 @@ fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<(
422442
// 2018 without `resolver` set must be V1
423443
assert_eq!(ws.resolve_behavior(), ResolveBehavior::V1);
424444
let specs = opts.compile_opts.spec.to_package_id_specs(ws)?;
425-
let mut target_data =
426-
RustcTargetData::new(ws, &opts.compile_opts.build_config.requested_kinds)?;
427445
let mut resolve_differences = |has_dev_units| -> CargoResult<(WorkspaceResolve<'_>, DiffMap)> {
428446
let ws_resolve = ops::resolve_ws_with_opts(
429447
ws,
430-
&mut target_data,
448+
target_data,
431449
&opts.compile_opts.build_config.requested_kinds,
432450
&opts.compile_opts.cli_features,
433451
&specs,
@@ -438,7 +456,7 @@ fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<(
438456
let feature_opts = FeatureOpts::new_behavior(ResolveBehavior::V2, has_dev_units);
439457
let v2_features = FeatureResolver::resolve(
440458
ws,
441-
&mut target_data,
459+
target_data,
442460
&ws_resolve.targeted_resolve,
443461
&ws_resolve.pkg_set,
444462
&opts.compile_opts.cli_features,
@@ -744,7 +762,8 @@ fn rustfix_crate(
744762
// We'll generate new errors below.
745763
file.errors_applying_fixes.clear();
746764
}
747-
(last_output, last_made_changes) = rustfix_and_fix(&mut files, rustc, filename, gctx)?;
765+
(last_output, last_made_changes) =
766+
rustfix_and_fix(&mut files, rustc, filename, args, gctx)?;
748767
if current_iteration == 0 {
749768
first_output = Some(last_output.clone());
750769
}
@@ -801,6 +820,7 @@ fn rustfix_and_fix(
801820
files: &mut HashMap<String, FixedFile>,
802821
rustc: &ProcessBuilder,
803822
filename: &Path,
823+
args: &FixArgs,
804824
gctx: &GlobalContext,
805825
) -> CargoResult<(Output, bool)> {
806826
// If not empty, filter by these lints.
@@ -865,10 +885,17 @@ fn rustfix_and_fix(
865885
continue;
866886
};
867887

888+
let file_path = Path::new(&file_name);
868889
// Do not write into registry cache. See rust-lang/cargo#9857.
869-
if Path::new(&file_name).starts_with(home_path) {
890+
if file_path.starts_with(home_path) {
870891
continue;
871892
}
893+
// Do not write into standard library source. See rust-lang/cargo#9857.
894+
if let Some(sysroot) = args.sysroot.as_deref() {
895+
if file_path.starts_with(sysroot) {
896+
continue;
897+
}
898+
}
872899

873900
if !file_names.clone().all(|f| f == &file_name) {
874901
trace!("rejecting as it changes multiple files: {:?}", suggestion);
@@ -1025,6 +1052,8 @@ struct FixArgs {
10251052
other: Vec<OsString>,
10261053
/// Path to the `rustc` executable.
10271054
rustc: PathBuf,
1055+
/// Path to host sysroot.
1056+
sysroot: Option<PathBuf>,
10281057
}
10291058

10301059
impl FixArgs {
@@ -1096,13 +1125,19 @@ impl FixArgs {
10961125
.saturating_next()
10971126
});
10981127

1128+
// ALLOWED: For the internal mechanism of `cargo fix` only.
1129+
// Shouldn't be set directly by anyone.
1130+
#[allow(clippy::disallowed_methods)]
1131+
let sysroot = env::var_os(SYSROOT_INTERNAL).map(PathBuf::from);
1132+
10991133
Ok(FixArgs {
11001134
file,
11011135
prepare_for_edition,
11021136
idioms,
11031137
enabled_edition,
11041138
other,
11051139
rustc,
1140+
sysroot,
11061141
})
11071142
}
11081143

0 commit comments

Comments
 (0)