From 3429da7787241ab8b06ba5cd0a36eb713126757f Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Fri, 21 Jun 2024 05:11:21 -0700 Subject: [PATCH 1/2] python,tests,misc: Remove Gerrit-ID insertion from pre-commit Change-Id: I4db06415c9d0bbba7a6db56d7e9febf6491003bf --- .pre-commit-config.yaml | 8 +- ext/git-commit-msg | 194 ---------------------------------------- util/git-commit-msg.py | 25 +++--- 3 files changed, 14 insertions(+), 213 deletions(-) delete mode 100755 ext/git-commit-msg diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 5183f38110..7e17adca7f 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -98,13 +98,7 @@ repos: description: The gem5 style checker hook. - id: gem5-commit-msg-checker name: gem5 commit msg checker - entry: ext/git-commit-msg + entry: util/git-commit-msg.py language: system stages: [commit-msg] description: The gem5 commit message checker hook. - - id: gerrit-commit-msg-job - name: gerrit commit message job - entry: util/gerrit-commit-msg-hook - language: system - stages: [commit-msg] - description: Adds Change-ID to the commit message. Needed by Gerrit. diff --git a/ext/git-commit-msg b/ext/git-commit-msg deleted file mode 100755 index db83fd73c3..0000000000 --- a/ext/git-commit-msg +++ /dev/null @@ -1,194 +0,0 @@ -#!/bin/sh -# From Gerrit Code Review 2.13.5-2617-gba50ae91fd -# -# Part of Gerrit Code Review (https://www.gerritcodereview.com/) -# -# Copyright (C) 2009 The Android Open Source Project -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -# - -unset GREP_OPTIONS - -CHANGE_ID_AFTER="Bug|Depends-On|Issue|Test|Feature|Fixes|Fixed" -MSG="$1" - -# Check for, and add if missing, a unique Change-Id -# -add_ChangeId() { - clean_message=`sed -e ' - /^diff --git .*/{ - s/// - q - } - /^Signed-off-by:/d - /^#/d - ' "$MSG" | git stripspace` - if test -z "$clean_message" - then - return - fi - - # Do not add Change-Id to temp commits - if echo "$clean_message" | head -1 | grep -q '^\(fixup\|squash\)!' - then - return - fi - - if test "false" = "`git config --bool --get gerrit.createChangeId`" - then - return - fi - - # Does Change-Id: already exist? if so, exit (no change). - if grep -i '^Change-Id:' "$MSG" >/dev/null - then - return - fi - - id=`_gen_ChangeId` - T="$MSG.tmp.$$" - AWK=awk - if [ -x /usr/xpg4/bin/awk ]; then - # Solaris AWK is just too broken - AWK=/usr/xpg4/bin/awk - fi - - # Get core.commentChar from git config or use default symbol - commentChar=`git config --get core.commentChar` - commentChar=${commentChar:-#} - - # How this works: - # - parse the commit message as (textLine+ blankLine*)* - # - assume textLine+ to be a footer until proven otherwise - # - exception: the first block is not footer (as it is the title) - # - read textLine+ into a variable - # - then count blankLines - # - once the next textLine appears, print textLine+ blankLine* as these - # aren't footer - # - in END, the last textLine+ block is available for footer parsing - $AWK ' - BEGIN { - # while we start with the assumption that textLine+ - # is a footer, the first block is not. - isFooter = 0 - footerComment = 0 - blankLines = 0 - } - - # Skip lines starting with commentChar without any spaces before it. - /^'"$commentChar"'/ { next } - - # Skip the line starting with the diff command and everything after it, - # up to the end of the file, assuming it is only patch data. - # If more than one line before the diff was empty, strip all but one. - /^diff --git / { - blankLines = 0 - while (getline) { } - next - } - - # Count blank lines outside footer comments - /^$/ && (footerComment == 0) { - blankLines++ - next - } - - # Catch footer comment - /^\[[a-zA-Z0-9-]+:/ && (isFooter == 1) { - footerComment = 1 - } - - /]$/ && (footerComment == 1) { - footerComment = 2 - } - - # We have a non-blank line after blank lines. Handle this. - (blankLines > 0) { - print lines - for (i = 0; i < blankLines; i++) { - print "" - } - - lines = "" - blankLines = 0 - isFooter = 1 - footerComment = 0 - } - - # Detect that the current block is not the footer - (footerComment == 0) && (!/^\[?[a-zA-Z0-9-]+:/ || /^[a-zA-Z0-9-]+:\/\//) { - isFooter = 0 - } - - { - # We need this information about the current last comment line - if (footerComment == 2) { - footerComment = 0 - } - if (lines != "") { - lines = lines "\n"; - } - lines = lines $0 - } - - # Footer handling: - # If the last block is considered a footer, splice in the Change-Id at the - # right place. - # Look for the right place to inject Change-Id by considering - # CHANGE_ID_AFTER. Keys listed in it (case insensitive) come first, - # then Change-Id, then everything else (eg. Signed-off-by:). - # - # Otherwise just print the last block, a new line and the Change-Id as a - # block of its own. - END { - unprinted = 1 - if (isFooter == 0) { - print lines "\n" - lines = "" - } - changeIdAfter = "^(" tolower("'"$CHANGE_ID_AFTER"'") "):" - numlines = split(lines, footer, "\n") - for (line = 1; line <= numlines; line++) { - if (unprinted && match(tolower(footer[line]), changeIdAfter) != 1) { - unprinted = 0 - print "Change-Id: I'"$id"'" - } - print footer[line] - } - if (unprinted) { - print "Change-Id: I'"$id"'" - } - }' "$MSG" > "$T" && mv "$T" "$MSG" || rm -f "$T" -} -_gen_ChangeIdInput() { - echo "tree `git write-tree`" - if parent=`git rev-parse "HEAD^0" 2>/dev/null` - then - echo "parent $parent" - fi - echo "author `git var GIT_AUTHOR_IDENT`" - echo "committer `git var GIT_COMMITTER_IDENT`" - echo - printf '%s' "$clean_message" -} -_gen_ChangeId() { - _gen_ChangeIdInput | - git hash-object -t commit --stdin -} - - -add_ChangeId - -# Perform gem5-specific commit message style check -./util/git-commit-msg.py $MSG diff --git a/util/git-commit-msg.py b/util/git-commit-msg.py index d40aa68e1c..943c8cafb0 100755 --- a/util/git-commit-msg.py +++ b/util/git-commit-msg.py @@ -142,18 +142,19 @@ if len(commit_header) > max_header_size: + ")" ) -# Then there must be at least one empty line between the commit header and -# the commit description -if commit_message_lines[1] != "": - _printErrorQuit( - "Please add an empty line between the commit title and " - "its description" - ) +if len(commit_message_lines) > 1: + # Then there must be at least one empty line between the commit header and + # the commit description + if commit_message_lines[1] != "": + _printErrorQuit( + "Please add an empty line between the commit title and " + "its description" + ) -# Encourage providing descriptions -if re.search( - "^(Signed-off-by|Change-Id|Reviewed-by):", commit_message_lines[2] -): - print("Warning: Commit does not have a description") + # Encourage providing descriptions + if re.search( + "^(Signed-off-by|Change-Id|Reviewed-by):", commit_message_lines[2] + ): + print("Warning: Commit does not have a description") sys.exit(0) From 28a6ca201b0c6969cab293b23a2493fa37ebd5fa Mon Sep 17 00:00:00 2001 From: "Bobby R. Bruce" Date: Fri, 21 Jun 2024 05:12:47 -0700 Subject: [PATCH 2/2] misc,tests: Remove Gerrit ID check from CI Workflow Change-Id: I86933f3b315f3233e135de2e32498c1641f7443e --- .github/workflows/ci-tests.yaml | 41 ++++++--------------------------- 1 file changed, 7 insertions(+), 34 deletions(-) diff --git a/.github/workflows/ci-tests.yaml b/.github/workflows/ci-tests.yaml index 3b666a4370..8e828675af 100644 --- a/.github/workflows/ci-tests.yaml +++ b/.github/workflows/ci-tests.yaml @@ -21,37 +21,11 @@ jobs: - uses: actions/setup-python@v5 - uses: pre-commit/action@v3.0.1 - # ensures we have a change-id in every commit, needed for gerrit - check-for-change-id: - # runs on github hosted runner - runs-on: ubuntu-latest - if: github.event.pull_request.draft == false - steps: - - uses: actions/checkout@v4 - with: - fetch-depth: 0 - - name: Check for Change-Id - run: | - # loop through all the commits in the pull request - for commit in $(git rev-list ${{ github.event.pull_request.base.sha }}..${{ github.event.pull_request.head.sha }}); do - git checkout $commit - if (git log -1 --pretty=format:"%B" | grep -q "Change-Id: ") - then - # passes as long as at least one change-id exists in the pull request - exit 0 - fi - done - # if we reach this part, none of the commits had a change-id - echo "None of the commits in this pull request contains a Change-ID, which we require for any changes made to gem5. "\ - "To automatically insert one, run the following:\n f=`git rev-parse --git-dir`/hooks/commit-msg ; mkdir -p $(dirname $f) ; "\ - "curl -Lo $f https://gerrit-review.googlesource.com/tools/hooks/commit-msg ; chmod +x $f\n Then amend the commit with git commit --amend --no-edit, and update your pull request." - exit 1 - unittests-all-opt: runs-on: [self-hosted, linux, x64] if: github.event.pull_request.draft == false container: ghcr.io/gem5/ubuntu-24.04_all-dependencies:latest - needs: [pre-commit, check-for-change-id] # only runs if pre-commit and change-id passes + needs: [pre-commit] # only runs if pre-commit passes. timeout-minutes: 60 steps: - uses: actions/checkout@v4 @@ -66,7 +40,7 @@ jobs: # In order to make sure the environment is exactly the same, we run in # the same container we use to build gem5 and run the testlib tests. This container: ghcr.io/gem5/ubuntu-24.04_all-dependencies:latest - needs: [pre-commit, check-for-change-id] + needs: [pre-commit] steps: - uses: actions/checkout@v4 @@ -97,7 +71,7 @@ jobs: runs-on: [self-hosted, linux, x64] if: github.event.pull_request.draft == false container: ghcr.io/gem5/clang-version-18:latest - needs: [pre-commit, check-for-change-id] + needs: [pre-commit] timeout-minutes: 90 steps: - uses: actions/checkout@v4 @@ -109,7 +83,7 @@ jobs: runs-on: [self-hosted, linux, x64] if: github.event.pull_request.draft == false container: ghcr.io/gem5/ubuntu-24.04_all-dependencies:latest - needs: [pre-commit, check-for-change-id, testlib-quick-matrix] + needs: [pre-commit, testlib-quick-matrix] strategy: matrix: build-target: ${{ fromJson(needs.testlib-quick-matrix.outputs.build-matrix) }} @@ -139,7 +113,7 @@ jobs: runs-on: [self-hosted, linux, x64] if: github.event.pull_request.draft == false container: ghcr.io/gem5/ubuntu-24.04_all-dependencies:latest - needs: [pre-commit, check-for-change-id, testlib-quick-matrix, testlib-quick-gem5-builds] + needs: [pre-commit, testlib-quick-matrix, testlib-quick-gem5-builds] timeout-minutes: 360 # 6 hours strategy: fail-fast: false @@ -188,7 +162,7 @@ jobs: runs-on: [self-hosted, linux, x64] if: github.event.pull_request.draft == false container: ghcr.io/gem5/ubuntu-24.04_all-dependencies:latest - needs: [pre-commit, check-for-change-id, testlib-quick-gem5-builds] + needs: [pre-commit, testlib-quick-gem5-builds] timeout-minutes: 30 steps: @@ -225,7 +199,7 @@ jobs: runs-on: [self-hosted, linux, x64] container: ghcr.io/gem5/gcn-gpu:latest timeout-minutes: 180 - needs: [pre-commit, check-for-change-id] + needs: [pre-commit] steps: - uses: actions/checkout@v4 @@ -257,7 +231,6 @@ jobs: - clang-fast-compilation - unittests-all-opt - pre-commit - - check-for-change-id - gpu-tests steps: - run: echo "This job's status is ${{ job.status }}."