Skip to content

Commit

Permalink
tidy: skip submodules if not present for non-CI environments
Browse files Browse the repository at this point in the history
Signed-off-by: onur-ozkan <work@onurozkan.dev>
  • Loading branch information
onur-ozkan committed Jun 10, 2024
1 parent dd50e18 commit 7e55123
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 28 deletions.
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
3 changes: 2 additions & 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,8 @@ 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.join(".gitmodules"))
{
builder.update_submodule(Path::new(submodule));
}

Expand Down
29 changes: 3 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,8 @@ impl<'a> ShouldRun<'a> {
///
/// [`path`]: ShouldRun::path
pub fn paths(mut self, paths: &[&str]) -> Self {
let submodules_paths = self.builder.get_all_submodules();
let gitmodules = self.builder.src.join(".gitmodules");
let submodules_paths = build_helper::util::parse_gitmodules(&gitmodules);

self.paths.insert(PathSet::Set(
paths
Expand Down Expand Up @@ -2238,28 +2237,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
26 changes: 26 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::PathBuf;
use std::process::Command;
use std::sync::OnceLock;

/// Invokes `build_helper::util::detail_exit` with `cfg!(test)`
///
Expand Down Expand Up @@ -45,3 +49,25 @@ pub fn try_run(cmd: &mut Command, print_cmd_on_fail: bool) -> Result<(), ()> {
Ok(())
}
}

/// Returns submodule paths from the given gitmodules file.
pub fn parse_gitmodules(gitmodules: &PathBuf) -> &[String] {
static SUBMODULES_PATHS: OnceLock<Vec<String>> = OnceLock::new();

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
16 changes: 16 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 @@ -46,6 +48,7 @@ type ExceptionList = &'static [(&'static str, &'static str)];
/// * Optionally a tuple of:
/// * A list of crates for which dependencies need to be explicitly allowed.
/// * The list of allowed dependencies.
/// * An indication of whether it is a submodule or not.
// FIXME auto detect all cargo workspaces
pub(crate) const WORKSPACES: &[(&str, ExceptionList, Option<(&[&str], &[&str])>)] = &[
// The root workspace has to be first for check_rustfix to work.
Expand Down Expand Up @@ -515,6 +518,19 @@ pub fn check(root: &Path, cargo: &Path, bad: &mut bool) {
let mut checked_runtime_licenses = false;

for &(workspace, exceptions, permitted_deps) in WORKSPACES {
let gitmodules = root.join(".gitmodules");
let submodules = build_helper::util::parse_gitmodules(&gitmodules);
// 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
16 changes: 15 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 @@ -14,6 +15,19 @@ const ALLOWED_SOURCES: &[&str] = &[
/// workspace `Cargo.toml`.
pub fn check(root: &Path, bad: &mut bool) {
for &(workspace, _, _) in crate::deps::WORKSPACES {
let gitmodules = root.join(".gitmodules");
let submodules = build_helper::util::parse_gitmodules(&gitmodules);
// 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

0 comments on commit 7e55123

Please sign in to comment.