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

Add tidy check to deny merge commits #105058

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
15 changes: 15 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ jobs:
uses: actions/checkout@v3
with:
fetch-depth: 2
- name: "checkout the `master` branch for tidy"
uses: actions/checkout@v3
with:
ref: master
fetch-depth: 1
- name: configure the PR in which the error message will be posted
run: "echo \"[CI_PR_NUMBER=$num]\""
env:
Expand Down Expand Up @@ -485,6 +490,11 @@ jobs:
uses: actions/checkout@v3
with:
fetch-depth: 2
- name: "checkout the `master` branch for tidy"
uses: actions/checkout@v3
with:
ref: master
fetch-depth: 1
- name: configure the PR in which the error message will be posted
run: "echo \"[CI_PR_NUMBER=$num]\""
env:
Expand Down Expand Up @@ -600,6 +610,11 @@ jobs:
uses: actions/checkout@v3
with:
fetch-depth: 2
- name: "checkout the `master` branch for tidy"
uses: actions/checkout@v3
with:
ref: master
fetch-depth: 1
- name: configure the PR in which the error message will be posted
run: "echo \"[CI_PR_NUMBER=$num]\""
env:
Expand Down
5 changes: 5 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,10 @@ dependencies = [
"toml",
]

[[package]]
name = "build_helper"
version = "0.1.0"

[[package]]
name = "bump-stage0"
version = "0.1.0"
Expand Down Expand Up @@ -5304,6 +5308,7 @@ dependencies = [
name = "tidy"
version = "0.1.0"
dependencies = [
"build_helper",
"cargo_metadata 0.14.0",
"ignore",
"lazy_static",
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ members = [
"library/std",
"library/test",
"src/rustdoc-json-types",
"src/tools/build_helper",
"src/tools/cargotest",
"src/tools/clippy",
"src/tools/clippy/clippy_dev",
Expand Down
5 changes: 5 additions & 0 deletions src/bootstrap/Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ dependencies = [
name = "bootstrap"
version = "0.0.0"
dependencies = [
"build_helper",
"cc",
"cmake",
"fd-lock",
Expand Down Expand Up @@ -70,6 +71,10 @@ dependencies = [
"regex-automata",
]

[[package]]
name = "build_helper"
version = "0.1.0"

[[package]]
name = "cc"
version = "1.0.73"
Expand Down
1 change: 1 addition & 0 deletions src/bootstrap/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ path = "bin/sccache-plus-cl.rs"
test = false

[dependencies]
build_helper = { path = "../tools/build_helper" }
cmake = "0.1.38"
fd-lock = "3.0.8"
filetime = "0.2"
Expand Down
30 changes: 1 addition & 29 deletions src/bootstrap/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use crate::builder::Builder;
use crate::util::{output, program_out_of_date, t};
use build_helper::git::get_rust_lang_rust_remote;
use ignore::WalkBuilder;
use std::collections::VecDeque;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -100,35 +101,6 @@ fn get_modified_rs_files(build: &Builder<'_>) -> Option<Vec<String>> {
)
}

/// Finds the remote for rust-lang/rust.
/// For example for these remotes it will return `upstream`.
/// ```text
/// origin https://github.com/Nilstrieb/rust.git (fetch)
/// origin https://github.com/Nilstrieb/rust.git (push)
/// upstream https://github.com/rust-lang/rust (fetch)
/// upstream https://github.com/rust-lang/rust (push)
/// ```
fn get_rust_lang_rust_remote() -> Result<String, String> {
let mut git = Command::new("git");
git.args(["config", "--local", "--get-regex", "remote\\..*\\.url"]);

let output = git.output().map_err(|err| format!("{err:?}"))?;
if !output.status.success() {
return Err("failed to execute git config command".to_owned());
}

let stdout = String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))?;

let rust_lang_remote = stdout
.lines()
.find(|remote| remote.contains("rust-lang"))
.ok_or_else(|| "rust-lang/rust remote not found".to_owned())?;

let remote_name =
rust_lang_remote.split('.').nth(1).ok_or_else(|| "remote name not found".to_owned())?;
Ok(remote_name.into())
}

#[derive(serde::Deserialize)]
struct RustfmtConfig {
ignore: Vec<String>,
Expand Down
3 changes: 2 additions & 1 deletion src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ use std::path::{Path, PathBuf};
use std::process::Command;
use std::str;

use build_helper::ci::CiEnv;
use channel::GitInfo;
use config::{DryRun, Target};
use filetime::FileTime;
Expand All @@ -121,7 +122,7 @@ use once_cell::sync::OnceCell;
use crate::builder::Kind;
use crate::config::{LlvmLibunwind, TargetSelection};
use crate::util::{
exe, libdir, mtime, output, run, run_suppressed, symlink_dir, try_run_suppressed, CiEnv,
exe, libdir, mtime, output, run, run_suppressed, symlink_dir, try_run_suppressed,
};

mod bolt;
Expand Down
4 changes: 3 additions & 1 deletion src/bootstrap/native.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ use crate::util::get_clang_cl_resource_dir;
use crate::util::{self, exe, output, t, up_to_date};
use crate::{CLang, GitRepo};

use build_helper::ci::CiEnv;

#[derive(Clone)]
pub struct LlvmResult {
/// Path to llvm-config binary.
Expand Down Expand Up @@ -217,7 +219,7 @@ pub(crate) fn is_ci_llvm_available(config: &Config, asserts: bool) -> bool {
return false;
}

if crate::util::CiEnv::is_ci() {
if CiEnv::is_ci() {
// We assume we have access to git, so it's okay to unconditionally pass
// `true` here.
let llvm_sha = detect_llvm_sha(config, true);
Expand Down
29 changes: 0 additions & 29 deletions src/bootstrap/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,35 +255,6 @@ pub enum CiEnv {
GitHubActions,
}

impl CiEnv {
/// Obtains the current CI environment.
pub fn current() -> CiEnv {
if env::var("TF_BUILD").map_or(false, |e| e == "True") {
CiEnv::AzurePipelines
} else if env::var("GITHUB_ACTIONS").map_or(false, |e| e == "true") {
CiEnv::GitHubActions
} else {
CiEnv::None
}
}

pub fn is_ci() -> bool {
Self::current() != CiEnv::None
}

/// If in a CI environment, forces the command to run with colors.
pub fn force_coloring_in_ci(self, cmd: &mut Command) {
if self != CiEnv::None {
// Due to use of stamp/docker, the output stream of rustbuild is not
// a TTY in CI, so coloring is by-default turned off.
// The explicit `TERM=xterm` environment is needed for
// `--color always` to actually work. This env var was lost when
// compiling through the Makefile. Very strange.
cmd.env("TERM", "xterm").args(&["--color", "always"]);
}
}
}

pub fn forcing_clang_based_tests() -> bool {
if let Some(var) = env::var_os("RUSTBUILD_FORCE_CLANG_BASED_TESTS") {
match &var.to_string_lossy().to_lowercase()[..] {
Expand Down
6 changes: 6 additions & 0 deletions src/ci/github-actions/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ x--expand-yaml-anchors--remove:
with:
fetch-depth: 2

- name: checkout the `master` branch for tidy
uses: actions/checkout@v3
with:
ref: master
fetch-depth: 1

# Rust Log Analyzer can't currently detect the PR number of a GitHub
# Actions build on its own, so a hint in the log message is needed to
# point it in the right direction.
Expand Down
8 changes: 8 additions & 0 deletions src/tools/build_helper/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[package]
name = "build_helper"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
40 changes: 40 additions & 0 deletions src/tools/build_helper/src/ci.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
use std::process::Command;

#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub enum CiEnv {
/// Not a CI environment.
None,
/// The Azure Pipelines environment, for Linux (including Docker), Windows, and macOS builds.
AzurePipelines,
/// The GitHub Actions environment, for Linux (including Docker), Windows and macOS builds.
GitHubActions,
}

impl CiEnv {
/// Obtains the current CI environment.
pub fn current() -> CiEnv {
if std::env::var("TF_BUILD").map_or(false, |e| e == "True") {
CiEnv::AzurePipelines
} else if std::env::var("GITHUB_ACTIONS").map_or(false, |e| e == "true") {
CiEnv::GitHubActions
} else {
CiEnv::None
}
}

pub fn is_ci() -> bool {
Self::current() != CiEnv::None
}

/// If in a CI environment, forces the command to run with colors.
pub fn force_coloring_in_ci(self, cmd: &mut Command) {
if self != CiEnv::None {
// Due to use of stamp/docker, the output stream of rustbuild is not
// a TTY in CI, so coloring is by-default turned off.
// The explicit `TERM=xterm` environment is needed for
// `--color always` to actually work. This env var was lost when
// compiling through the Makefile. Very strange.
cmd.env("TERM", "xterm").args(&["--color", "always"]);
}
}
}
30 changes: 30 additions & 0 deletions src/tools/build_helper/src/git.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
use std::process::Command;

/// Finds the remote for rust-lang/rust.
/// For example for these remotes it will return `upstream`.
/// ```text
/// origin https://github.com/Nilstrieb/rust.git (fetch)
/// origin https://github.com/Nilstrieb/rust.git (push)
/// upstream https://github.com/rust-lang/rust (fetch)
/// upstream https://github.com/rust-lang/rust (push)
/// ```
pub fn get_rust_lang_rust_remote() -> Result<String, String> {
let mut git = Command::new("git");
git.args(["config", "--local", "--get-regex", "remote\\..*\\.url"]);

let output = git.output().map_err(|err| format!("{err:?}"))?;
if !output.status.success() {
return Err("failed to execute git config command".to_owned());
}

let stdout = String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))?;

let rust_lang_remote = stdout
.lines()
.find(|remote| remote.contains("rust-lang"))
.ok_or_else(|| "rust-lang/rust remote not found".to_owned())?;

let remote_name =
rust_lang_remote.split('.').nth(1).ok_or_else(|| "remote name not found".to_owned())?;
Ok(remote_name.into())
}
2 changes: 2 additions & 0 deletions src/tools/build_helper/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
pub mod ci;
pub mod git;
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.14"
regex = "1"
miropt-test-tools = { path = "../miropt-test-tools" }
Expand Down
1 change: 1 addition & 0 deletions src/tools/tidy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ pub mod errors;
pub mod extdeps;
pub mod features;
pub mod mir_opt_tests;
pub mod no_merge;
pub mod pal;
pub mod primitive_docs;
pub mod style;
Expand Down
2 changes: 2 additions & 0 deletions src/tools/tidy/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ fn main() {
check!(alphabetical, &compiler_path);
check!(alphabetical, &library_path);

check!(no_merge, ());

let collected = {
drain_handles(&mut handles);

Expand Down
72 changes: 72 additions & 0 deletions src/tools/tidy/src/no_merge.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
//! This check makes sure that no accidental merge commits are introduced to the repository.
//! It forbids all merge commits that are not caused by rollups/bors or subtree syncs.

use std::process::Command;

use build_helper::ci::CiEnv;
use build_helper::git::get_rust_lang_rust_remote;

macro_rules! try_unwrap_in_ci {
($expr:expr) => {
match $expr {
Ok(value) => value,
Err(err) if CiEnv::is_ci() => {
panic!("Encountered error while testing Git status: {:?}", err)
}
Err(_) => return,
}
};
}

pub fn check(_: (), bad: &mut bool) {
let remote = try_unwrap_in_ci!(get_rust_lang_rust_remote());
let merge_commits = try_unwrap_in_ci!(find_merge_commits(&remote));

let mut bad_merge_commits = merge_commits.lines().filter(|commit| {
!(
// Bors is the ruler of merge commits.
commit.starts_with("Auto merge of") || commit.starts_with("Rollup merge of")
)
});

if let Some(merge) = bad_merge_commits.next() {
tidy_error!(
bad,
"found a merge commit in the history: `{merge}`.
To resolve the issue, see this: https://rustc-dev-guide.rust-lang.org/git.html#i-made-a-merge-commit-by-accident.
If you're doing a subtree sync, add your tool to the list in the code that emitted this error."
);
}
}

/// Runs `git log --merges --format=%s $REMOTE/master..HEAD` and returns all commits
fn find_merge_commits(remote: &str) -> Result<String, String> {
let mut git = Command::new("git");
git.args([
"log",
"--merges",
"--format=%s",
&format!("{remote}/master..HEAD"),
// Ignore subtree syncs. Add your new subtrees here.
":!src/tools/miri",
":!src/tools/rust-analyzer",
":!compiler/rustc_smir",
":!library/portable-simd",
":!compiler/rustc_codegen_gcc",
":!src/tools/rustfmt",
":!compiler/rustc_codegen_cranelift",
":!src/tools/clippy",
]);

let output = git.output().map_err(|err| format!("{err:?}"))?;
if !output.status.success() {
return Err(format!(
"failed to execute git log command: {}",
String::from_utf8_lossy(&output.stderr)
));
}

let stdout = String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))?;

Ok(stdout)
}