about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMatthias Krüger <matthias.krueger@famsik.de>2024-06-23 22:38:58 +0200
committerGitHub <noreply@github.com>2024-06-23 22:38:58 +0200
commitf016552b1cbdf4c6acefa3dd2d69f658c8814cf8 (patch)
tree33f0bdc449b39c13152f8e5b859259594ae685ef
parent33422e72c8a66bdb5ee21246a948a1a02ca91674 (diff)
parente9e3c38d01669d87d2d14b17d5c7e0b2188991df (diff)
downloadrust-f016552b1cbdf4c6acefa3dd2d69f658c8814cf8.tar.gz
rust-f016552b1cbdf4c6acefa3dd2d69f658c8814cf8.zip
Rollup merge of #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 https://github.com/rust-lang/rust/pull/126225#issuecomment-2158143674 and reverts #126225.
-rw-r--r--Cargo.lock1
-rw-r--r--src/bootstrap/src/core/build_steps/dist.rs2
-rw-r--r--src/bootstrap/src/core/build_steps/test.rs2
-rw-r--r--src/bootstrap/src/core/builder.rs28
-rw-r--r--src/tools/build_helper/src/util.rs28
-rw-r--r--src/tools/tidy/Cargo.toml1
-rw-r--r--src/tools/tidy/src/deps.rs14
-rw-r--r--src/tools/tidy/src/extdeps.rs15
8 files changed, 61 insertions, 30 deletions
diff --git a/Cargo.lock b/Cargo.lock
index 9d1481ec768..b4f623e626d 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -5678,6 +5678,7 @@ dependencies = [
 name = "tidy"
 version = "0.1.0"
 dependencies = [
+ "build_helper",
  "cargo_metadata 0.15.4",
  "fluent-syntax",
  "ignore",
diff --git a/src/bootstrap/src/core/build_steps/dist.rs b/src/bootstrap/src/core/build_steps/dist.rs
index 4fb64595504..2dc7cd7de6a 100644
--- a/src/bootstrap/src/core/build_steps/dist.rs
+++ b/src/bootstrap/src/core/build_steps/dist.rs
@@ -1018,7 +1018,7 @@ impl Step for PlainSourceTarball {
             // perhaps it should be removed in favor of making `dist` perform the `vendor` step?
 
             // Ensure we have all submodules from src and other directories checked out.
-            for submodule in builder.get_all_submodules() {
+            for submodule in build_helper::util::parse_gitmodules(&builder.src) {
                 builder.update_submodule(Path::new(submodule));
             }
 
diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs
index 8a2bc3b9d48..efc09c41bf4 100644
--- a/src/bootstrap/src/core/build_steps/test.rs
+++ b/src/bootstrap/src/core/build_steps/test.rs
@@ -1048,8 +1048,6 @@ impl Step for Tidy {
     /// Once tidy passes, this step also runs `fmt --check` if tests are being run
     /// for the `dev` or `nightly` channels.
     fn run(self, builder: &Builder<'_>) {
-        builder.build.update_submodule(Path::new("src/tools/rustc-perf"));
-
         let mut cmd = builder.tool_cmd(Tool::Tidy);
         cmd.arg(&builder.src);
         cmd.arg(&builder.initial_cargo);
diff --git a/src/bootstrap/src/core/builder.rs b/src/bootstrap/src/core/builder.rs
index 2e9c04624c7..14ccbfe0267 100644
--- a/src/bootstrap/src/core/builder.rs
+++ b/src/bootstrap/src/core/builder.rs
@@ -4,13 +4,11 @@ use std::collections::BTreeSet;
 use std::env;
 use std::ffi::{OsStr, OsString};
 use std::fmt::{Debug, Write};
-use std::fs::{self, File};
+use std::fs;
 use std::hash::Hash;
-use std::io::{BufRead, BufReader};
 use std::ops::Deref;
 use std::path::{Path, PathBuf};
 use std::process::Command;
-use std::sync::OnceLock;
 use std::time::{Duration, Instant};
 
 use crate::core::build_steps::tool::{self, SourceType};
@@ -594,7 +592,7 @@ impl<'a> ShouldRun<'a> {
     ///
     /// [`path`]: ShouldRun::path
     pub fn paths(mut self, paths: &[&str]) -> Self {
-        let submodules_paths = self.builder.get_all_submodules();
+        let submodules_paths = build_helper::util::parse_gitmodules(&self.builder.src);
 
         self.paths.insert(PathSet::Set(
             paths
@@ -2243,28 +2241,6 @@ impl<'a> Builder<'a> {
         out
     }
 
-    /// Return paths of all submodules.
-    pub fn get_all_submodules(&self) -> &[String] {
-        static SUBMODULES_PATHS: OnceLock<Vec<String>> = OnceLock::new();
-
-        let init_submodules_paths = |src: &PathBuf| {
-            let file = File::open(src.join(".gitmodules")).unwrap();
-
-            let mut submodules_paths = vec![];
-            for line in BufReader::new(file).lines().map_while(Result::ok) {
-                let line = line.trim();
-                if line.starts_with("path") {
-                    let actual_path = line.split(' ').last().expect("Couldn't get value of path");
-                    submodules_paths.push(actual_path.to_owned());
-                }
-            }
-
-            submodules_paths
-        };
-
-        SUBMODULES_PATHS.get_or_init(|| init_submodules_paths(&self.src))
-    }
-
     /// Ensure that a given step is built *only if it's supposed to be built by default*, returning
     /// its output. This will cache the step, so it's safe (and good!) to call this as often as
     /// needed to ensure that all dependencies are build.
diff --git a/src/tools/build_helper/src/util.rs b/src/tools/build_helper/src/util.rs
index 5801a8648f2..72c05c4c48a 100644
--- a/src/tools/build_helper/src/util.rs
+++ b/src/tools/build_helper/src/util.rs
@@ -1,4 +1,8 @@
+use std::fs::File;
+use std::io::{BufRead, BufReader};
+use std::path::Path;
 use std::process::Command;
+use std::sync::OnceLock;
 
 /// Invokes `build_helper::util::detail_exit` with `cfg!(test)`
 ///
@@ -45,3 +49,27 @@ pub fn try_run(cmd: &mut Command, print_cmd_on_fail: bool) -> Result<(), ()> {
         Ok(())
     }
 }
+
+/// Returns the submodule paths from the `.gitmodules` file in the given directory.
+pub fn parse_gitmodules(target_dir: &Path) -> &[String] {
+    static SUBMODULES_PATHS: OnceLock<Vec<String>> = OnceLock::new();
+    let gitmodules = target_dir.join(".gitmodules");
+    assert!(gitmodules.exists(), "'{}' file is missing.", gitmodules.display());
+
+    let init_submodules_paths = || {
+        let file = File::open(gitmodules).unwrap();
+
+        let mut submodules_paths = vec![];
+        for line in BufReader::new(file).lines().map_while(Result::ok) {
+            let line = line.trim();
+            if line.starts_with("path") {
+                let actual_path = line.split(' ').last().expect("Couldn't get value of path");
+                submodules_paths.push(actual_path.to_owned());
+            }
+        }
+
+        submodules_paths
+    };
+
+    SUBMODULES_PATHS.get_or_init(|| init_submodules_paths())
+}
diff --git a/src/tools/tidy/Cargo.toml b/src/tools/tidy/Cargo.toml
index f39438bd9ac..0b05d5add52 100644
--- a/src/tools/tidy/Cargo.toml
+++ b/src/tools/tidy/Cargo.toml
@@ -5,6 +5,7 @@ edition = "2021"
 autobins = false
 
 [dependencies]
+build_helper = { path = "../build_helper" }
 cargo_metadata = "0.15"
 regex = "1"
 miropt-test-tools = { path = "../miropt-test-tools" }
diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs
index 7337c9843c7..aa119819aaa 100644
--- a/src/tools/tidy/src/deps.rs
+++ b/src/tools/tidy/src/deps.rs
@@ -1,7 +1,9 @@
 //! Checks the licenses of third-party dependencies.
 
+use build_helper::ci::CiEnv;
 use cargo_metadata::{Metadata, Package, PackageId};
 use std::collections::HashSet;
+use std::fs::read_dir;
 use std::path::Path;
 
 /// These are licenses that are allowed for all crates, including the runtime,
@@ -514,7 +516,19 @@ const PERMITTED_CRANELIFT_DEPENDENCIES: &[&str] = &[
 pub fn check(root: &Path, cargo: &Path, bad: &mut bool) {
     let mut checked_runtime_licenses = false;
 
+    let submodules = build_helper::util::parse_gitmodules(root);
     for &(workspace, exceptions, permitted_deps) in WORKSPACES {
+        // Skip if it's a submodule, not in a CI environment, and not initialized.
+        //
+        // This prevents enforcing developers to fetch submodules for tidy.
+        if submodules.contains(&workspace.into())
+            && !CiEnv::is_ci()
+            // If the directory is empty, we can consider it as an uninitialized submodule.
+            && read_dir(root.join(workspace)).unwrap().next().is_none()
+        {
+            continue;
+        }
+
         if !root.join(workspace).join("Cargo.lock").exists() {
             tidy_error!(bad, "the `{workspace}` workspace doesn't have a Cargo.lock");
             continue;
diff --git a/src/tools/tidy/src/extdeps.rs b/src/tools/tidy/src/extdeps.rs
index 2118de5f204..8bb80f11711 100644
--- a/src/tools/tidy/src/extdeps.rs
+++ b/src/tools/tidy/src/extdeps.rs
@@ -1,6 +1,7 @@
 //! Check for external package sources. Allow only vendorable packages.
 
-use std::fs;
+use build_helper::ci::CiEnv;
+use std::fs::{self, read_dir};
 use std::path::Path;
 
 /// List of allowed sources for packages.
@@ -13,7 +14,19 @@ const ALLOWED_SOURCES: &[&str] = &[
 /// Checks for external package sources. `root` is the path to the directory that contains the
 /// workspace `Cargo.toml`.
 pub fn check(root: &Path, bad: &mut bool) {
+    let submodules = build_helper::util::parse_gitmodules(root);
     for &(workspace, _, _) in crate::deps::WORKSPACES {
+        // Skip if it's a submodule, not in a CI environment, and not initialized.
+        //
+        // This prevents enforcing developers to fetch submodules for tidy.
+        if submodules.contains(&workspace.into())
+            && !CiEnv::is_ci()
+            // If the directory is empty, we can consider it as an uninitialized submodule.
+            && read_dir(root.join(workspace)).unwrap().next().is_none()
+        {
+            continue;
+        }
+
         // FIXME check other workspaces too
         // `Cargo.lock` of rust.
         let path = root.join(workspace).join("Cargo.lock");