From 48207895af70611186459c8a2de7c6233bb5fc59 Mon Sep 17 00:00:00 2001 From: Nicholas Sudsgaard Date: Fri, 8 Aug 2025 07:31:39 +0900 Subject: [PATCH] lint: Warn about using change IDs for merged changes This script will warn and suggest fixes when a CB: of an already merged change is found in the commit message. This should enforce the clarification that was added to the documentation in CB:88776. This script requires a JSON parser (i.e. jq) to parse Gerrit's REST API[1]. While it may be possible to grep the values, we chose to use a proper parser to ensure there would be no false-positives. TEST= Prepare a commit with the following commit message: Here are some open changes: CB:88614 CB:88717 CB:87282 Here are some abandoned changes: CB:88413 CB:84504 CB:82136 Here are some merged changes: CB:88566 CB:88598 CB:88697 Here are some old merged commits: CB:1 CB:50 CB:950 Here are some wrong stuff: CL:100 CB:TEST CB:99999 The script produces the following result (may change in the future when open changes are merged etc): Using a change ID (CB:88566) for an already merged commit; please replace it with: commit 21639c377120 ("mb/getac/p470: Use common gpio functions") Using a change ID (CB:88598) for an already merged commit; please replace it with: commit 05a38e2af3f0 ("mb/google/fatcat: Disable memory training progress bar") Using a change ID (CB:88697) for an already merged commit; please replace it with: commit 1da2f46db869 ("soc/intel/alderlake: Restore mem_init_override_channel_mask()") Using a change ID (CB:1) for an already merged commit; please replace it with: commit 140a990a612e ("Teach abuild to emit JUnit formatted build reports") Using a change ID (CB:50) for an already merged commit; please replace it with: commit 7c634ae8c18d ("msrtool: added support for Intel CPUs") Using a change ID (CB:950) for an already merged commit; please replace it with: commit c31384e62c98 ("Fix up Sandybridge C state generation code") CB:99999 does not exist [1] https://gerrit-review.googlesource.com/Documentation/rest-api.html Change-Id: I1c72f739b1f47b1227ef1e158b1553aa56945d7e Signed-off-by: Nicholas Sudsgaard Reviewed-on: https://review.coreboot.org/c/coreboot/+/88715 Tested-by: build bot (Jenkins) Reviewed-by: Matt DeVillier --- util/lint/lint-extended-025-merged-change-ids | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100755 util/lint/lint-extended-025-merged-change-ids diff --git a/util/lint/lint-extended-025-merged-change-ids b/util/lint/lint-extended-025-merged-change-ids new file mode 100755 index 0000000000..10f9d793b3 --- /dev/null +++ b/util/lint/lint-extended-025-merged-change-ids @@ -0,0 +1,97 @@ +#!/usr/bin/env sh +# SPDX-License-Identifier: GPL-2.0-or-later +# +# DESCR: Check that CB: does not refer to a merged change + + +LINTDIR="$( + cd -- "$(dirname "$0")" > /dev/null 2>&1 || return + pwd -P +)" + +# shellcheck source=helper_functions.sh +. "${LINTDIR}/helper_functions.sh" + +if [ "${IN_GIT_TREE}" -eq 0 ]; then + exit 0 +fi + +if ! command -v jq > /dev/null; then + printf 'This script requires "jq" to be installed\n' >&2 + exit 1 +fi + +GERRIT_REST_API_URL="https://review.coreboot.org" +# Excerpt from documentation[1]: +# +# > To prevent against Cross Site Script Inclusion (XSSI) attacks, the JSON +# > response body starts with a magic prefix line that must be stripped before +# > feeding the rest of the response body to a JSON parser. +# +# [1] https://gerrit-review.googlesource.com/Documentation/rest-api.html#output +GERRIT_REST_API_MAGIC_PREFIX=")]}'" + +api_request() { + api="$1" + curl --silent "${GERRIT_REST_API_URL}/${api}" \ + | sed "s/^${GERRIT_REST_API_MAGIC_PREFIX}//" +} + +# Fetch git log in the format specified by the Linux kernel guidelines[1], which +# coreboot follows, for referencing merged commits. +# +# [1] https://www.kernel.org/doc/html/v4.10/process/submitting-patches.html +search_commit() { + ${GIT} log -n 1 --abbrev=12 --pretty='commit %h ("%s")' "$@" 2> /dev/null +} + +get_commit_reference() { + change_id="$1" + response="$(api_request "changes/coreboot~${change_id}/revisions/current/commit")" + + hash="$(echo "${response}" | jq --raw-output .commit)" + reference="$(search_commit "${hash}")" + + # Earlier changes merged through Gerrit (around < CB:1000) did not + # update the patchset once being cherry-picked into the codebase. + # Therefore, the pre-merged commit remains in Gerrit's database (what we + # query with the REST API) and causes problems, as the commit hash would + # be different from the one found in the main branch. + # + # To workaround this issue, we attempt to find the corresponding commit + # by grepping for a commit message that contains a specific "Change-Id". + if [ -z "${reference}" ]; then + hash="$(echo "${response}" \ + | jq .message \ + | sed -E 's/.*Change-Id: ([[:alnum:]]+).*/\1/')" + reference="$(search_commit --grep "${hash}")" + fi + + if [ -z "${reference}" ]; then + reference="(cannot find commit; probably requires a rebase)" + fi + + echo "${reference}" +} + +# This test is mainly for the jenkins server +for change_id in $(${GIT} log -n 1 | grep -Eo 'CB:[0-9]+'); do + change_id="${change_id#CB:}" # strip "CB:" prefix from change ID + + response="$(api_request "changes/coreboot~${change_id}")" + + if echo "${response}" | grep -qF 'Not found'; then + printf 'CB:%d does not exist\n' "${change_id}" + continue + fi + + status="$(echo "${response}" | jq --raw-output .status)" + + if [ "${status}" = "MERGED" ]; then + reference="$(get_commit_reference "${change_id}")" + + printf 'Using a change ID (CB:%d) for an already merged change; please replace it with:\n' \ + "${change_id}" + printf '%s\n' "${reference}" + fi +done