Merge bitcoin/bitcoin#31653: lint: Call more checks from test_runner

faf8fc5487 lint: Call lint_commit_msg from test_runner (MarcoFalke)
fa99728b0c lint: Move commit range printing to test_runner (MarcoFalke)
fa673cf344 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](faf8fc5487)
  willcl-ark:
    tACK faf8fc5487

Tree-SHA512: 78018adc618d997508c226c9eee0a4fada3899cdfd91587132ab1c0389aea69127bafc3a900e90e30aca2c6bae9dcd6e6188ef287e91413bc63ee66fb078b1af

ci/lint/06_script.sh

@@ -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.
 
@@ -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

test/lint/lint-git-commit-check.py

@@ -1,52 +0,0 @@
-#!/usr/bin/env python3
-#
-# Copyright (c) 2020-2022 The Bitcoin Core developers
-# Distributed under the MIT software license, see the accompanying
-# file COPYING or http://www.opensource.org/licenses/mit-license.php.
-#
-# Linter to check that commit messages have a new line before the body
-# or no body at all
-
-import argparse
-import os
-import sys
-
-from subprocess import check_output
-
-
-def parse_args():
-    """Parse command line arguments."""
-    parser = argparse.ArgumentParser(
-        description="""
-            Linter to check that commit messages have a new line before
-            the body or no body at all.
-        """,
-        epilog=f"""
-            You can manually set the commit-range with the COMMIT_RANGE
-            environment variable (e.g. "COMMIT_RANGE='HEAD~n..HEAD'
-            {sys.argv[0]}") for the last 'n' commits.
-        """)
-    return parser.parse_args()
-
-
-def main():
-    parse_args()
-    exit_code = 0
-
-    assert os.getenv("COMMIT_RANGE")  # E.g. COMMIT_RANGE='HEAD~n..HEAD'
-    commit_range = os.getenv("COMMIT_RANGE")
-
-    commit_hashes = check_output(["git", "-c", "log.showSignature=false", "log", commit_range, "--format=%H"], text=True, encoding="utf8").splitlines()
-
-    for hash in commit_hashes:
-        commit_info = check_output(["git", "-c", "log.showSignature=false", "log", "--format=%B", "-n", "1", hash], text=True, encoding="utf8").splitlines()
-        if len(commit_info) >= 2:
-            if commit_info[1]:
-                print(f"The subject line of commit hash {hash} is followed by a non-empty line. Subject lines should always be followed by a blank line.")
-                exit_code = 1
-
-    sys.exit(exit_code)
-
-
-if __name__ == "__main__":
-    main()

test/lint/test_runner/src/main.rs

@@ -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",
@@ -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![
@@ -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!(
@@ -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 {