From 7789ea584232770210f169505b27eb1381b9c3b0 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Fri, 3 May 2024 12:21:03 -0500 Subject: [PATCH 1/5] ci: pre-collapse GitLab CI sections Sections of CI output defined by `begin_group()` and `end_group()` are expanded in GitLab pipelines by default. This can make CI job output rather noisy and harder to navigate. Update the behavior for GitLab pipelines to now collapse sections by default. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- ci/lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/lib.sh b/ci/lib.sh index 0a73fc7bd1..02e5e058dd 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -18,7 +18,7 @@ elif test true = "$GITLAB_CI" then begin_group () { need_to_end_group=t - printf "\e[0Ksection_start:$(date +%s):$(echo "$1" | tr ' ' _)\r\e[0K$1\n" + printf "\e[0Ksection_start:$(date +%s):$(echo "$1" | tr ' ' _)[collapsed=true]\r\e[0K$1\n" trap "end_group '$1'" EXIT set -x } From ecaacbc7a278549f31d6f77d729c49fa60eec734 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Fri, 3 May 2024 12:21:04 -0500 Subject: [PATCH 2/5] github-ci: fix link to whitespace error When the `check-whitespace` CI job detects whitespace errors, a formatted summary of the issue is generated. This summary contains links to the commits and blobs responsible for the whitespace errors. The generated links for blobs do not work and result in a 404. Instead of using the reference name in the link, use the commit ID directly. This fixes the broken link and also helps enable future generalization of the script for other CI providers by removing one of the GitHub specific CI variables used. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- .github/workflows/check-whitespace.yml | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml index a241a63428..a3a6913ecc 100644 --- a/.github/workflows/check-whitespace.yml +++ b/.github/workflows/check-whitespace.yml @@ -31,14 +31,15 @@ jobs: commit= commitText= commitTextmd= - goodparent= + goodParent= while read dash sha etc do case "${dash}" in - "---") - if test -z "${commit}" + "---") # Line contains commit information. + if test -z "${goodParent}" then - goodparent=${sha} + # Assume the commit has no whitespace errors until detected otherwise. + goodParent=${sha} fi commit="${sha}" commitText="${sha} ${etc}" @@ -46,18 +47,18 @@ jobs: ;; "") ;; - *) - if test -n "${commit}" + *) # Line contains whitespace error information for current commit. + if test -n "${goodParent}" then problems+=("1) --- ${commitTextmd}") echo "" echo "--- ${commitText}" - commit= + goodParent= fi case "${dash}" in *:[1-9]*:) # contains file and line number information dashend=${dash#*:} - problems+=("[${dash}](https://github.com/${{ github.repository }}/blob/${{github.event.pull_request.head.ref}}/${dash%%:*}#L${dashend%:}) ${sha} ${etc}") + problems+=("[${dash}](https://github.com/${{ github.repository }}/blob/${commit}/${dash%%:*}#L${dashend%:}) ${sha} ${etc}") ;; *) problems+=("\`${dash} ${sha} ${etc}\`") @@ -70,15 +71,15 @@ jobs: if test ${#problems[*]} -gt 0 then - if test -z "${commit}" + if test -z "${goodParent}" then - goodparent=${baseSha: 0:7} + goodParent=${baseSha: 0:7} fi echo "🛑 Please review the Summary output for further information." echo "### :x: A whitespace issue was found in one or more of the commits." >$GITHUB_STEP_SUMMARY echo "" >>$GITHUB_STEP_SUMMARY echo "Run these commands to correct the problem:" >>$GITHUB_STEP_SUMMARY - echo "1. \`git rebase --whitespace=fix ${goodparent}\`" >>$GITHUB_STEP_SUMMARY + echo "1. \`git rebase --whitespace=fix ${goodParent}\`" >>$GITHUB_STEP_SUMMARY echo "1. \`git push --force\`" >>$GITHUB_STEP_SUMMARY echo " " >>$GITHUB_STEP_SUMMARY echo "Errors:" >>$GITHUB_STEP_SUMMARY From 66820fb7bfc47e0356dd8c27af3abd2a27daafcd Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Fri, 3 May 2024 12:21:05 -0500 Subject: [PATCH 3/5] ci: separate whitespace check script The `check-whitespace` CI job is only available as a GitHub action. To help enable this job with other CI providers, first separate the logic performing the whitespace check into its own script. In subsequent commits, this script is further generalized allowing its reuse. Helped-by: Patrick Steinhardt Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- .github/workflows/check-whitespace.yml | 68 ++--------------------- ci/check-whitespace.sh | 74 ++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 64 deletions(-) create mode 100755 ci/check-whitespace.sh diff --git a/.github/workflows/check-whitespace.yml b/.github/workflows/check-whitespace.yml index a3a6913ecc..d0a78fc426 100644 --- a/.github/workflows/check-whitespace.yml +++ b/.github/workflows/check-whitespace.yml @@ -26,67 +26,7 @@ jobs: - name: git log --check id: check_out run: | - baseSha=${{github.event.pull_request.base.sha}} - problems=() - commit= - commitText= - commitTextmd= - goodParent= - while read dash sha etc - do - case "${dash}" in - "---") # Line contains commit information. - if test -z "${goodParent}" - then - # Assume the commit has no whitespace errors until detected otherwise. - goodParent=${sha} - fi - commit="${sha}" - commitText="${sha} ${etc}" - commitTextmd="[${sha}](https://github.com/${{ github.repository }}/commit/${sha}) ${etc}" - ;; - "") - ;; - *) # Line contains whitespace error information for current commit. - if test -n "${goodParent}" - then - problems+=("1) --- ${commitTextmd}") - echo "" - echo "--- ${commitText}" - goodParent= - fi - case "${dash}" in - *:[1-9]*:) # contains file and line number information - dashend=${dash#*:} - problems+=("[${dash}](https://github.com/${{ github.repository }}/blob/${commit}/${dash%%:*}#L${dashend%:}) ${sha} ${etc}") - ;; - *) - problems+=("\`${dash} ${sha} ${etc}\`") - ;; - esac - echo "${dash} ${sha} ${etc}" - ;; - esac - done <<< $(git log --check --pretty=format:"---% h% s" ${baseSha}..) - - if test ${#problems[*]} -gt 0 - then - if test -z "${goodParent}" - then - goodParent=${baseSha: 0:7} - fi - echo "🛑 Please review the Summary output for further information." - echo "### :x: A whitespace issue was found in one or more of the commits." >$GITHUB_STEP_SUMMARY - echo "" >>$GITHUB_STEP_SUMMARY - echo "Run these commands to correct the problem:" >>$GITHUB_STEP_SUMMARY - echo "1. \`git rebase --whitespace=fix ${goodParent}\`" >>$GITHUB_STEP_SUMMARY - echo "1. \`git push --force\`" >>$GITHUB_STEP_SUMMARY - echo " " >>$GITHUB_STEP_SUMMARY - echo "Errors:" >>$GITHUB_STEP_SUMMARY - for i in "${problems[@]}" - do - echo "${i}" >>$GITHUB_STEP_SUMMARY - done - - exit 2 - fi + ./ci/check-whitespace.sh \ + "${{github.event.pull_request.base.sha}}" \ + "$GITHUB_STEP_SUMMARY" \ + "https://github.com/${{github.repository}}" diff --git a/ci/check-whitespace.sh b/ci/check-whitespace.sh new file mode 100755 index 0000000000..9cc496da40 --- /dev/null +++ b/ci/check-whitespace.sh @@ -0,0 +1,74 @@ +#!/usr/bin/env bash + +baseCommit=$1 +outputFile=$2 +url=$3 + +problems=() +commit= +commitText= +commitTextmd= +goodParent= + +while read dash sha etc +do + case "${dash}" in + "---") # Line contains commit information. + if test -z "${goodParent}" + then + # Assume the commit has no whitespace errors until detected otherwise. + goodParent=${sha} + fi + + commit="${sha}" + commitText="${sha} ${etc}" + commitTextmd="[${sha}](${url}/commit/${sha}) ${etc}" + ;; + "") + ;; + *) # Line contains whitespace error information for current commit. + if test -n "${goodParent}" + then + problems+=("1) --- ${commitTextmd}") + echo "" + echo "--- ${commitText}" + goodParent= + fi + + case "${dash}" in + *:[1-9]*:) # contains file and line number information + dashend=${dash#*:} + problems+=("[${dash}](${url}/blob/${commit}/${dash%%:*}#L${dashend%:}) ${sha} ${etc}") + ;; + *) + problems+=("\`${dash} ${sha} ${etc}\`") + ;; + esac + echo "${dash} ${sha} ${etc}" + ;; + esac +done <<< "$(git log --check --pretty=format:"---% h% s" "${baseCommit}"..)" + +if test ${#problems[*]} -gt 0 +then + if test -z "${goodParent}" + then + goodParent=${baseCommit: 0:7} + fi + + echo "🛑 Please review the Summary output for further information." + echo "### :x: A whitespace issue was found in one or more of the commits." >"$outputFile" + echo "" >>"$outputFile" + echo "Run these commands to correct the problem:" >>"$outputFile" + echo "1. \`git rebase --whitespace=fix ${goodParent}\`" >>"$outputFile" + echo "1. \`git push --force\`" >>"$outputFile" + echo " " >>"$outputFile" + echo "Errors:" >>"$outputFile" + + for i in "${problems[@]}" + do + echo "${i}" >>"$outputFile" + done + + exit 2 +fi From 9bef98096c1a58ec0f0e793a282374b50a43eaab Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Fri, 3 May 2024 12:21:06 -0500 Subject: [PATCH 4/5] ci: make the whitespace report optional The `check-whitespace` CI job generates a formatted output file containing whitespace error information. As not all CI providers support rendering a formatted summary, make its generation optional. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- ci/check-whitespace.sh | 45 +++++++++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/ci/check-whitespace.sh b/ci/check-whitespace.sh index 9cc496da40..db399097a5 100755 --- a/ci/check-whitespace.sh +++ b/ci/check-whitespace.sh @@ -1,9 +1,20 @@ #!/usr/bin/env bash +# +# Check that commits after a specified point do not contain new or modified +# lines with whitespace errors. An optional formatted summary can be generated +# by providing an output file path and url as additional arguments. +# baseCommit=$1 outputFile=$2 url=$3 +if test "$#" -ne 1 && test "$#" -ne 3 +then + echo "USAGE: $0 [ ]" + exit 1 +fi + problems=() commit= commitText= @@ -56,19 +67,29 @@ then goodParent=${baseCommit: 0:7} fi - echo "🛑 Please review the Summary output for further information." - echo "### :x: A whitespace issue was found in one or more of the commits." >"$outputFile" - echo "" >>"$outputFile" - echo "Run these commands to correct the problem:" >>"$outputFile" - echo "1. \`git rebase --whitespace=fix ${goodParent}\`" >>"$outputFile" - echo "1. \`git push --force\`" >>"$outputFile" - echo " " >>"$outputFile" - echo "Errors:" >>"$outputFile" + echo "A whitespace issue was found in onen of more of the commits." + echo "Run the following command to resolve whitespace issues:" + echo "git rebase --whitespace=fix ${goodParent}" - for i in "${problems[@]}" - do - echo "${i}" >>"$outputFile" - done + # If target output file is provided, write formatted output. + if test -n "$outputFile" + then + echo "🛑 Please review the Summary output for further information." + ( + echo "### :x: A whitespace issue was found in one or more of the commits." + echo "" + echo "Run these commands to correct the problem:" + echo "1. \`git rebase --whitespace=fix ${goodParent}\`" + echo "1. \`git push --force\`" + echo "" + echo "Errors:" + + for i in "${problems[@]}" + do + echo "${i}" + done + ) >"$outputFile" + fi exit 2 fi From 8f19e82c5b313a1518119cc4b31c32ec3001b967 Mon Sep 17 00:00:00 2001 From: Justin Tobler Date: Fri, 3 May 2024 12:21:07 -0500 Subject: [PATCH 5/5] gitlab-ci: add whitespace error check GitLab CI does not have a job to check for whitespace errors introduced by a set of changes. Reuse the existing generic `whitespace-check.sh` to create the job for GitLab pipelines. Note that the `$CI_MERGE_REQUEST_TARGET_BRANCH_SHA` variable is only available in GitLab merge request pipelines and therefore the CI job is configured to only run as part of those pipelines. Signed-off-by: Justin Tobler Signed-off-by: Junio C Hamano --- .gitlab-ci.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c0fa2fe90b..619bf729fa 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -102,3 +102,12 @@ static-analysis: script: - ./ci/run-static-analysis.sh - ./ci/check-directional-formatting.bash + +check-whitespace: + image: ubuntu:latest + before_script: + - ./ci/install-dependencies.sh + script: + - ./ci/check-whitespace.sh "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA" + rules: + - if: $CI_PIPELINE_SOURCE == 'merge_request_event'