Skip to content

Deny merge in CI take 2 #106333

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

Closed
wants to merge 3 commits into from
Closed
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
23 changes: 20 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -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:
@@ -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:
@@ -588,9 +598,11 @@ jobs:
strategy:
matrix:
include:
- name: dist-x86_64-linux
os: ubuntu-20.04-xl
env: {}
- name: aarch64-gnu
os:
- self-hosted
- ARM64
- linux
timeout-minutes: 600
runs-on: "${{ matrix.os }}"
steps:
@@ -600,6 +612,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:
5 changes: 5 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
@@ -259,6 +259,10 @@ dependencies = [
"toml",
]

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

[[package]]
name = "bump-stage0"
version = "0.1.0"
@@ -5304,6 +5308,7 @@ dependencies = [
name = "tidy"
version = "0.1.0"
dependencies = [
"build_helper",
"cargo_metadata 0.14.0",
"ignore",
"lazy_static",
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -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",
5 changes: 5 additions & 0 deletions src/bootstrap/Cargo.lock
Original file line number Diff line number Diff line change
@@ -36,6 +36,7 @@ dependencies = [
name = "bootstrap"
version = "0.0.0"
dependencies = [
"build_helper",
"cc",
"cmake",
"fd-lock",
@@ -70,6 +71,10 @@ dependencies = [
"regex-automata",
]

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

[[package]]
name = "cc"
version = "1.0.73"
1 change: 1 addition & 0 deletions src/bootstrap/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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"
30 changes: 1 addition & 29 deletions src/bootstrap/format.rs
Original file line number Diff line number Diff line change
@@ -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};
@@ -95,35 +96,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>,
3 changes: 2 additions & 1 deletion src/bootstrap/lib.rs
Original file line number Diff line number Diff line change
@@ -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;
@@ -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;
4 changes: 3 additions & 1 deletion src/bootstrap/native.rs
Original file line number Diff line number Diff line change
@@ -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.
@@ -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);
29 changes: 0 additions & 29 deletions src/bootstrap/util.rs
Original file line number Diff line number Diff line change
@@ -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()[..] {
11 changes: 8 additions & 3 deletions src/ci/github-actions/ci.yml
Original file line number Diff line number Diff line change
@@ -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.
@@ -753,9 +759,8 @@ jobs:
strategy:
matrix:
include:
- &dist-x86_64-linux
name: dist-x86_64-linux
<<: *job-linux-xl
- name: aarch64-gnu
<<: *job-aarch64-linux

master:
name: master
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
@@ -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" }
1 change: 1 addition & 0 deletions src/tools/tidy/src/lib.rs
Original file line number Diff line number Diff line change
@@ -61,6 +61,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;
2 changes: 2 additions & 0 deletions src/tools/tidy/src/main.rs
Original file line number Diff line number Diff line change
@@ -107,6 +107,8 @@ fn main() {
check!(alphabetical, &compiler_path);
check!(alphabetical, &library_path);

check!(no_merge, ());

let collected = {
drain_handles(&mut handles);

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", // Ignore miri to see whether its the only thing missing or whatevers going on
":!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)
}