Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tidy: skip submodules if not present for non-CI environments #126230

Merged
merged 2 commits into from
Jun 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5697,6 +5697,7 @@ dependencies = [
name = "tidy"
version = "0.1.0"
dependencies = [
"build_helper",
"cargo_metadata 0.15.4",
"ignore",
"miropt-test-tools",
Expand Down
2 changes: 1 addition & 1 deletion src/bootstrap/src/core/build_steps/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1014,7 +1014,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));
}

Expand Down
2 changes: 0 additions & 2 deletions src/bootstrap/src/core/build_steps/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1101,8 +1101,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);
Expand Down
28 changes: 2 additions & 26 deletions src/bootstrap/src/core/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -577,7 +575,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
Expand Down Expand Up @@ -2238,28 +2236,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.
Expand Down
28 changes: 28 additions & 0 deletions src/tools/build_helper/src/util.rs
Original file line number Diff line number Diff line change
@@ -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)`
///
Expand Down Expand Up @@ -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())
}
1 change: 1 addition & 0 deletions src/tools/tidy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
14 changes: 14 additions & 0 deletions src/tools/tidy/src/deps.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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;
Expand Down
15 changes: 14 additions & 1 deletion src/tools/tidy/src/extdeps.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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");
Expand Down
Loading