From 03388e0b4166a2be4e1b611e89db706ac973c798 Mon Sep 17 00:00:00 2001 From: unknowIfGuestInDream Date: Thu, 1 May 2025 23:52:24 +0800 Subject: [PATCH 1/2] ci: Add check-commits action Close: #2101 Signed-off-by: unknowIfGuestInDream --- .github/workflows/CheckBadMerge.groovy | 156 +++++++++++++++++++++++++ .github/workflows/check-commits.yml | 65 +++++++++++ 2 files changed, 221 insertions(+) create mode 100644 .github/workflows/CheckBadMerge.groovy create mode 100644 .github/workflows/check-commits.yml diff --git a/.github/workflows/CheckBadMerge.groovy b/.github/workflows/CheckBadMerge.groovy new file mode 100644 index 000000000..47ba3a188 --- /dev/null +++ b/.github/workflows/CheckBadMerge.groovy @@ -0,0 +1,156 @@ +/* + * Copyright (c) 2025 unknowIfGuestInDream. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of unknowIfGuestInDream, any associated website, nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE + * DISCLAIMED. IN NO EVENT SHALL UNKNOWIFGUESTINDREAM BE LIABLE FOR ANY + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +import java.util.concurrent.Callable +import java.util.concurrent.Executors +import java.util.concurrent.Future +import java.nio.file.Files +import java.nio.file.Paths + +/** + * When merging `releaseX` branch into `master`, we should only use the release note from the `master` branch, + * but sometimes changes on release notes.md was brought to master and merged unnoticed, + * + * This script is to check if there is any merge commit that brings changes from release branch to master. + * Usage: groovy CheckBadMerge.groovy ... + * If any "bad" merge commit is found, it will print the details and exit with non-zero code. + */ +class CheckBadMerge { + private static final THREAD_POOL = Executors.newCachedThreadPool() + + private static final List MONITORED_PATHS = [ + "CODE_OF_CONDUCT.md", + "CONTRIBUTING.md", + "SECURITY.md", + "LICENSE", + ".idea/codeStyles/", + ".idea/copyright/", + ".github/FUNDING.yml" + ] + + static void main(String[] args) { + if (args.length != 1) { + System.err.println("Usage: groovy CheckBadMerge.groovy ") + System.exit(1) + } + + List commits = Files.readAllLines(Paths.get(args[0])) + try { + commits.each { checkCommit(it) } + } finally { + THREAD_POOL.shutdown() + } + } + + static void checkCommit(String commit) { + List parentCommits = parentCommitsOf(commit) + if (parentCommits.size() != 2) { + println("$commit is not a merge commit we're looking for. Parents: $parentCommits") + return + } + + List commitBranches = branchesOf(commit) + if (commitBranches.contains("origin/release")) { + println("$commit is a merge commit already on release, ignoring.") + println(" Branches: $commitBranches") + return + } + + // The correct state we are looking for is: + // 1. It's a merge commit. + // 2. One of its parent commits is from master only. + // 3. Another parent commit is not from master but from release branch. + // Otherwise, skip this commit. + List p1Branches = branchesOf(parentCommits[0]) + List p2Branches = branchesOf(parentCommits[1]) + + println("$commit parents: $parentCommits") + println(" p1Branches: $p1Branches") + println(" p2Branches: $p2Branches") + if (p1Branches.contains("origin/master") && !p2Branches.contains("origin/master") && p2Branches.any { it.startsWith("origin/release") }) { + List badFiles = filesFromMerge(commit).findAll {gitFile -> MONITORED_PATHS.any { forbiddenPath -> gitFile.startsWith(forbiddenPath)} } + if (!badFiles.empty) { + System.err.println("Found bad files in merge commit $commit, run the listed commands:") + badFiles.each { + System.err.println("git restore --source=master -SW -- '$it'") + } + System.err.println("And then amend the merge commit to remove all offending changes.") + System.exit(1) + } else { + println(" -> No bad files found") + } + } else { + println(" -> is not a merge commit we're looking for.") + } + } + + static List filesFromMerge(String commit) { + getStdout("git diff --name-only $commit^1..$commit").readLines() + } + + static List branchesOf(String commit) { + return getStdout("git branch -r --contains $commit") + .readLines() + .collect { it.replace("*", "") } // remove the * from the current branch, e.g. * master -> master + .collect { it.trim() } + .grep { !it.isEmpty() } + } + + static List parentCommitsOf(String commit) { + return getStdout("git show --format=%P --no-patch --no-show-signature $commit") + .split(" ").collect { it.trim() }.grep { !it.isEmpty() } + } + + @groovy.transform.ToString + static class ExecResult { + String stdout + String stderr + int returnCode + } + + static ExecResult exec(String command) { + Process process = command.execute() + def stdoutFuture = readStreamAsync(process.inputStream) + def stderrFuture = readStreamAsync(process.errorStream) + + int returnCode = process.waitFor() + String stdout = stdoutFuture.get() + String stderr = stderrFuture.get() + return new ExecResult(stderr: stderr, stdout: stdout, returnCode: returnCode) + } + + static String getStdout(String command) { + ExecResult execResult = exec(command) + + assert execResult.returnCode == 0: "$command failed with return code: $execResult" + return execResult.stdout + } + + static Future readStreamAsync(InputStream inputStream) { + return THREAD_POOL.submit({ inputStream.text } as Callable) as Future + } +} diff --git a/.github/workflows/check-commits.yml b/.github/workflows/check-commits.yml new file mode 100644 index 000000000..8215c5607 --- /dev/null +++ b/.github/workflows/check-commits.yml @@ -0,0 +1,65 @@ +name: Check commits +on: + workflow_dispatch: + pull_request: + types: + - opened + - synchronize + +permissions: { } + +jobs: + # See .github/workflows/CheckBadMerge.groovy for explanation + check_bad_merge: + if: github.event.pull_request.base.ref == 'master' + permissions: + contents: read + issues: write + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 0 + - name: Set up JDK 17 + uses: actions/setup-java@v4 + with: + distribution: 'temurin' + java-version: '17' + - name: Install Groovy + run: sudo apt-get install groovy + - name: List PR commits + run: | + git log --pretty=format:"%H" ${{ github.event.pull_request.base.sha }}..${{ github.event.pull_request.head.sha }} > pr_commits.txt + - name: Check PR commits + id: run_check + run: | + groovy .github/workflows/CheckBadMerge.groovy pr_commits.txt > output.txt 2>&1 + - name: Read output file + id: read_output + if: ${{ always() }} + run: | + cat output.txt + OUTPUT=$(cat output.txt) + echo "OUTPUT<> $GITHUB_ENV + echo "$OUTPUT" >> $GITHUB_ENV + echo "EOF" >> $GITHUB_ENV + - name: Comment on PR if check failed + if: ${{ failure() }} + uses: actions/github-script@v7 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const output = ` + Some bad merge is found: + \`\`\` + ${{ env.OUTPUT }} + \`\`\` + `; + github.rest.issues.createComment({ + issue_number: context.issue.number, + owner: context.repo.owner, + repo: context.repo.repo, + body: output + }) From efaf17d97d5cf060d57408488eb569fa524c27de Mon Sep 17 00:00:00 2001 From: unknowIfGuestInDream Date: Fri, 2 May 2025 07:38:55 +0800 Subject: [PATCH 2/2] ci: Add check-commits action Close: #2101 Signed-off-by: unknowIfGuestInDream --- .github/workflows/CheckBadMerge.groovy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/CheckBadMerge.groovy b/.github/workflows/CheckBadMerge.groovy index 47ba3a188..6eaa7786e 100644 --- a/.github/workflows/CheckBadMerge.groovy +++ b/.github/workflows/CheckBadMerge.groovy @@ -76,14 +76,14 @@ class CheckBadMerge { List commitBranches = branchesOf(commit) if (commitBranches.contains("origin/release")) { println("$commit is a merge commit already on release, ignoring.") - println(" Branches: $commitBranches") return } + println(" Branches: $commitBranches") // The correct state we are looking for is: // 1. It's a merge commit. // 2. One of its parent commits is from master only. - // 3. Another parent commit is not from master but from release branch. + // 3. Another parent commit is not from master. // Otherwise, skip this commit. List p1Branches = branchesOf(parentCommits[0]) List p2Branches = branchesOf(parentCommits[1]) @@ -91,7 +91,7 @@ class CheckBadMerge { println("$commit parents: $parentCommits") println(" p1Branches: $p1Branches") println(" p2Branches: $p2Branches") - if (p1Branches.contains("origin/master") && !p2Branches.contains("origin/master") && p2Branches.any { it.startsWith("origin/release") }) { + if (p1Branches.contains("origin/master") && !p2Branches.contains("origin/master")) { List badFiles = filesFromMerge(commit).findAll {gitFile -> MONITORED_PATHS.any { forbiddenPath -> gitFile.startsWith(forbiddenPath)} } if (!badFiles.empty) { System.err.println("Found bad files in merge commit $commit, run the listed commands:")