Skip to content

Commit

Permalink
Merge bitcoin#31653: lint: Call more checks from test_runner
Browse files Browse the repository at this point in the history
faf8fc5 lint: Call lint_commit_msg from test_runner (MarcoFalke)
fa99728 lint: Move commit range printing to test_runner (MarcoFalke)
fa673cf lint: Call lint_scripted_diff from test_runner (MarcoFalke)

Pull request description:

  The lint `commit-script-check.sh` can not be called from the test_runner at all and must be called manually. Also, some checks require `COMMIT_RANGE` to be set.

  Fix all issues by moving two lint checks into the test_runner. Also, the proper commit range is passed to the checks by the test_runner, so that the user no longer has to do it.

ACKs for top commit:
  kevkevinpal:
    reACK [faf8fc5](bitcoin@faf8fc5)
  willcl-ark:
    tACK faf8fc5

Tree-SHA512: 78018adc618d997508c226c9eee0a4fada3899cdfd91587132ab1c0389aea69127bafc3a900e90e30aca2c6bae9dcd6e6188ef287e91413bc63ee66fb078b1af
  • Loading branch information
fanquake committed Feb 4, 2025
2 parents 1172bc4 + faf8fc5 commit 6f5ae1a
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 64 deletions.
14 changes: 2 additions & 12 deletions ci/lint/06_script.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env bash
#
# Copyright (c) 2018-2022 The Bitcoin Core developers
# Copyright (c) 2018-present The Bitcoin Core developers
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.

Expand All @@ -9,23 +9,13 @@ export LC_ALL=C
set -ex

if [ -n "$CIRRUS_PR" ]; then
COMMIT_RANGE="HEAD~..HEAD"
export COMMIT_RANGE="HEAD~..HEAD"
if [ "$(git rev-list -1 HEAD)" != "$(git rev-list -1 --merges HEAD)" ]; then
echo "Error: The top commit must be a merge commit, usually the remote 'pull/${PR_NUMBER}/merge' branch."
false
fi
else
# Otherwise, assume that a merge commit exists. This merge commit is assumed
# to be the base, after which linting will be done. If the merge commit is
# HEAD, the range will be empty.
COMMIT_RANGE="$( git rev-list --max-count=1 --merges HEAD )..HEAD"
fi
export COMMIT_RANGE

echo
git log --no-merges --oneline "$COMMIT_RANGE"
echo
test/lint/commit-script-check.sh "$COMMIT_RANGE"
RUST_BACKTRACE=1 "${LINT_RUNNER_PATH}/test_runner"

if [ "$CIRRUS_REPO_FULL_NAME" = "bitcoin/bitcoin" ] && [ "$CIRRUS_PR" = "" ] ; then
Expand Down
52 changes: 0 additions & 52 deletions test/lint/lint-git-commit-check.py

This file was deleted.

78 changes: 78 additions & 0 deletions test/lint/test_runner/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,16 @@ fn get_linter_list() -> Vec<&'static Linter> {
name: "subtree",
lint_fn: lint_subtree
},
&Linter {
description: "Check scripted-diffs",
name: "scripted_diff",
lint_fn: lint_scripted_diff
},
&Linter {
description: "Check that commit messages have a new line before the body or no body at all.",
name: "commit_msg",
lint_fn: lint_commit_msg
},
&Linter {
description: "Check that tabs are not used as whitespace",
name: "tabs_whitespace",
Expand Down Expand Up @@ -173,6 +183,21 @@ fn get_git_root() -> PathBuf {
PathBuf::from(check_output(git().args(["rev-parse", "--show-toplevel"])).unwrap())
}

/// Return the commit range, or panic
fn commit_range() -> String {
// Use the env var, if set. E.g. COMMIT_RANGE='HEAD~n..HEAD' for the last 'n' commits.
env::var("COMMIT_RANGE").unwrap_or_else(|_| {
// Otherwise, assume that a merge commit exists. This merge commit is assumed
// to be the base, after which linting will be done. If the merge commit is
// HEAD, the range will be empty.
format!(
"{}..HEAD",
check_output(git().args(["rev-list", "--max-count=1", "--merges", "HEAD"]))
.expect("check_output failed")
)
})
}

/// Return all subtree paths
fn get_subtrees() -> Vec<&'static str> {
vec![
Expand Down Expand Up @@ -210,6 +235,55 @@ fn lint_subtree() -> LintResult {
}
}

fn lint_scripted_diff() -> LintResult {
if Command::new("test/lint/commit-script-check.sh")
.arg(commit_range())
.status()
.expect("command error")
.success()
{
Ok(())
} else {
Err("".to_string())
}
}

fn lint_commit_msg() -> LintResult {
let mut good = true;
let commit_hashes = check_output(git().args(&[
"-c",
"log.showSignature=false",
"log",
&commit_range(),
"--format=%H",
]))?;
for hash in commit_hashes.lines() {
let commit_info = check_output(git().args([
"-c",
"log.showSignature=false",
"log",
"--format=%B",
"-n",
"1",
hash,
]))?;
if let Some(line) = commit_info.lines().nth(1) {
if !line.is_empty() {
println!(
"The subject line of commit hash {} is followed by a non-empty line. Subject lines should always be followed by a blank line.",
hash
);
good = false;
}
}
}
if good {
Ok(())
} else {
Err("".to_string())
}
}

fn lint_py_lint() -> LintResult {
let bin_name = "ruff";
let checks = format!(
Expand Down Expand Up @@ -650,6 +724,10 @@ fn main() -> ExitCode {
};

let git_root = get_git_root();
let commit_range = commit_range();
let commit_log = check_output(git().args(["log", "--no-merges", "--oneline", &commit_range]))
.expect("check_output failed");
println!("Checking commit range ({commit_range}):\n{commit_log}\n");

let mut test_failed = false;
for linter in linters_to_run {
Expand Down

0 comments on commit 6f5ae1a

Please sign in to comment.