Skip to content

Commit f016552

Browse files
authored
Rollup merge of rust-lang#126230 - onur-ozkan:followup-126225, r=Mark-Simulacrum
tidy: skip submodules if not present for non-CI environments Right now tidy requires rustc-perf to be fetched as it checks its license, but this doesn't make sense for most contributors since rustc-perf is a dist-specific tool and its license will not change frequently, especially during compiler development. This PR makes tidy to skip rustc-perf if it's not fetched and if it's not running in CI. Applies rust-lang#126225 (comment) and reverts rust-lang#126225.
2 parents 33422e7 + e9e3c38 commit f016552

File tree

8 files changed

+61
-30
lines changed

8 files changed

+61
-30
lines changed

Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -5678,6 +5678,7 @@ dependencies = [
56785678
name = "tidy"
56795679
version = "0.1.0"
56805680
dependencies = [
5681+
"build_helper",
56815682
"cargo_metadata 0.15.4",
56825683
"fluent-syntax",
56835684
"ignore",

src/bootstrap/src/core/build_steps/dist.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1018,7 +1018,7 @@ impl Step for PlainSourceTarball {
10181018
// perhaps it should be removed in favor of making `dist` perform the `vendor` step?
10191019

10201020
// Ensure we have all submodules from src and other directories checked out.
1021-
for submodule in builder.get_all_submodules() {
1021+
for submodule in build_helper::util::parse_gitmodules(&builder.src) {
10221022
builder.update_submodule(Path::new(submodule));
10231023
}
10241024

src/bootstrap/src/core/build_steps/test.rs

-2
Original file line numberDiff line numberDiff line change
@@ -1048,8 +1048,6 @@ impl Step for Tidy {
10481048
/// Once tidy passes, this step also runs `fmt --check` if tests are being run
10491049
/// for the `dev` or `nightly` channels.
10501050
fn run(self, builder: &Builder<'_>) {
1051-
builder.build.update_submodule(Path::new("src/tools/rustc-perf"));
1052-
10531051
let mut cmd = builder.tool_cmd(Tool::Tidy);
10541052
cmd.arg(&builder.src);
10551053
cmd.arg(&builder.initial_cargo);

src/bootstrap/src/core/builder.rs

+2-26
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,11 @@ use std::collections::BTreeSet;
44
use std::env;
55
use std::ffi::{OsStr, OsString};
66
use std::fmt::{Debug, Write};
7-
use std::fs::{self, File};
7+
use std::fs;
88
use std::hash::Hash;
9-
use std::io::{BufRead, BufReader};
109
use std::ops::Deref;
1110
use std::path::{Path, PathBuf};
1211
use std::process::Command;
13-
use std::sync::OnceLock;
1412
use std::time::{Duration, Instant};
1513

1614
use crate::core::build_steps::tool::{self, SourceType};
@@ -594,7 +592,7 @@ impl<'a> ShouldRun<'a> {
594592
///
595593
/// [`path`]: ShouldRun::path
596594
pub fn paths(mut self, paths: &[&str]) -> Self {
597-
let submodules_paths = self.builder.get_all_submodules();
595+
let submodules_paths = build_helper::util::parse_gitmodules(&self.builder.src);
598596

599597
self.paths.insert(PathSet::Set(
600598
paths
@@ -2243,28 +2241,6 @@ impl<'a> Builder<'a> {
22432241
out
22442242
}
22452243

2246-
/// Return paths of all submodules.
2247-
pub fn get_all_submodules(&self) -> &[String] {
2248-
static SUBMODULES_PATHS: OnceLock<Vec<String>> = OnceLock::new();
2249-
2250-
let init_submodules_paths = |src: &PathBuf| {
2251-
let file = File::open(src.join(".gitmodules")).unwrap();
2252-
2253-
let mut submodules_paths = vec![];
2254-
for line in BufReader::new(file).lines().map_while(Result::ok) {
2255-
let line = line.trim();
2256-
if line.starts_with("path") {
2257-
let actual_path = line.split(' ').last().expect("Couldn't get value of path");
2258-
submodules_paths.push(actual_path.to_owned());
2259-
}
2260-
}
2261-
2262-
submodules_paths
2263-
};
2264-
2265-
SUBMODULES_PATHS.get_or_init(|| init_submodules_paths(&self.src))
2266-
}
2267-
22682244
/// Ensure that a given step is built *only if it's supposed to be built by default*, returning
22692245
/// its output. This will cache the step, so it's safe (and good!) to call this as often as
22702246
/// needed to ensure that all dependencies are build.

src/tools/build_helper/src/util.rs

+28
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1+
use std::fs::File;
2+
use std::io::{BufRead, BufReader};
3+
use std::path::Path;
14
use std::process::Command;
5+
use std::sync::OnceLock;
26

37
/// Invokes `build_helper::util::detail_exit` with `cfg!(test)`
48
///
@@ -45,3 +49,27 @@ pub fn try_run(cmd: &mut Command, print_cmd_on_fail: bool) -> Result<(), ()> {
4549
Ok(())
4650
}
4751
}
52+
53+
/// Returns the submodule paths from the `.gitmodules` file in the given directory.
54+
pub fn parse_gitmodules(target_dir: &Path) -> &[String] {
55+
static SUBMODULES_PATHS: OnceLock<Vec<String>> = OnceLock::new();
56+
let gitmodules = target_dir.join(".gitmodules");
57+
assert!(gitmodules.exists(), "'{}' file is missing.", gitmodules.display());
58+
59+
let init_submodules_paths = || {
60+
let file = File::open(gitmodules).unwrap();
61+
62+
let mut submodules_paths = vec![];
63+
for line in BufReader::new(file).lines().map_while(Result::ok) {
64+
let line = line.trim();
65+
if line.starts_with("path") {
66+
let actual_path = line.split(' ').last().expect("Couldn't get value of path");
67+
submodules_paths.push(actual_path.to_owned());
68+
}
69+
}
70+
71+
submodules_paths
72+
};
73+
74+
SUBMODULES_PATHS.get_or_init(|| init_submodules_paths())
75+
}

src/tools/tidy/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ edition = "2021"
55
autobins = false
66

77
[dependencies]
8+
build_helper = { path = "../build_helper" }
89
cargo_metadata = "0.15"
910
regex = "1"
1011
miropt-test-tools = { path = "../miropt-test-tools" }

src/tools/tidy/src/deps.rs

+14
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
//! Checks the licenses of third-party dependencies.
22
3+
use build_helper::ci::CiEnv;
34
use cargo_metadata::{Metadata, Package, PackageId};
45
use std::collections::HashSet;
6+
use std::fs::read_dir;
57
use std::path::Path;
68

79
/// These are licenses that are allowed for all crates, including the runtime,
@@ -514,7 +516,19 @@ const PERMITTED_CRANELIFT_DEPENDENCIES: &[&str] = &[
514516
pub fn check(root: &Path, cargo: &Path, bad: &mut bool) {
515517
let mut checked_runtime_licenses = false;
516518

519+
let submodules = build_helper::util::parse_gitmodules(root);
517520
for &(workspace, exceptions, permitted_deps) in WORKSPACES {
521+
// Skip if it's a submodule, not in a CI environment, and not initialized.
522+
//
523+
// This prevents enforcing developers to fetch submodules for tidy.
524+
if submodules.contains(&workspace.into())
525+
&& !CiEnv::is_ci()
526+
// If the directory is empty, we can consider it as an uninitialized submodule.
527+
&& read_dir(root.join(workspace)).unwrap().next().is_none()
528+
{
529+
continue;
530+
}
531+
518532
if !root.join(workspace).join("Cargo.lock").exists() {
519533
tidy_error!(bad, "the `{workspace}` workspace doesn't have a Cargo.lock");
520534
continue;

src/tools/tidy/src/extdeps.rs

+14-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
//! Check for external package sources. Allow only vendorable packages.
22
3-
use std::fs;
3+
use build_helper::ci::CiEnv;
4+
use std::fs::{self, read_dir};
45
use std::path::Path;
56

67
/// List of allowed sources for packages.
@@ -13,7 +14,19 @@ const ALLOWED_SOURCES: &[&str] = &[
1314
/// Checks for external package sources. `root` is the path to the directory that contains the
1415
/// workspace `Cargo.toml`.
1516
pub fn check(root: &Path, bad: &mut bool) {
17+
let submodules = build_helper::util::parse_gitmodules(root);
1618
for &(workspace, _, _) in crate::deps::WORKSPACES {
19+
// Skip if it's a submodule, not in a CI environment, and not initialized.
20+
//
21+
// This prevents enforcing developers to fetch submodules for tidy.
22+
if submodules.contains(&workspace.into())
23+
&& !CiEnv::is_ci()
24+
// If the directory is empty, we can consider it as an uninitialized submodule.
25+
&& read_dir(root.join(workspace)).unwrap().next().is_none()
26+
{
27+
continue;
28+
}
29+
1730
// FIXME check other workspaces too
1831
// `Cargo.lock` of rust.
1932
let path = root.join(workspace).join("Cargo.lock");

0 commit comments

Comments
 (0)