The 'check-whitespace' CI script exists gracefully if no base commit is provided or if an invalid revision is provided. This is not good because if a particular CI provides an incorrect base_commit, it would fail successfully. This is exactly the case with the GitLab CI. The CI is using the "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA" variable to get the base commit SHA, but variable is only defined for _merged_ pipelines. So it is empty for regular pipelines [1]. This should've failed the check-whitespace job. Let's fix the variable used in the GitLab CI. Let's also add a check for incorrect base_commit in the 'check-whitespace.sh' script. While here, fix a small typo too. [1]: https://docs.gitlab.com/ee/ci/variables/predefined_variables.html#predefined-variables-for-merge-request-pipelines Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx> --- .gitlab-ci.yml | 2 +- ci/check-whitespace.sh | 13 ++++++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 65fd261e5e..36199893d8 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -119,7 +119,7 @@ check-whitespace: before_script: - ./ci/install-dependencies.sh script: - - ./ci/check-whitespace.sh "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA" + - ./ci/check-whitespace.sh "$CI_MERGE_REQUEST_DIFF_BASE_SHA" rules: - if: $CI_PIPELINE_SOURCE == 'merge_request_event' diff --git a/ci/check-whitespace.sh b/ci/check-whitespace.sh index db399097a5..ab023f9519 100755 --- a/ci/check-whitespace.sh +++ b/ci/check-whitespace.sh @@ -9,12 +9,19 @@ baseCommit=$1 outputFile=$2 url=$3 -if test "$#" -ne 1 && test "$#" -ne 3 +if { test "$#" -ne 1 && test "$#" -ne 3; } || test -z "$1" then echo "USAGE: $0 <BASE_COMMIT> [<OUTPUT_FILE> <URL>]" exit 1 fi +gitLogOutput=$(git log --check --pretty=format:"---% h% s" "${baseCommit}"..) +if test $? -ne 0 +then + echo -n $gitLogOutput + exit 1 +fi + problems=() commit= commitText= @@ -58,7 +65,7 @@ do echo "${dash} ${sha} ${etc}" ;; esac -done <<< "$(git log --check --pretty=format:"---% h% s" "${baseCommit}"..)" +done <<< "$gitLogOutput" if test ${#problems[*]} -gt 0 then @@ -67,7 +74,7 @@ then goodParent=${baseCommit: 0:7} fi - echo "A whitespace issue was found in onen of more of the commits." + echo "A whitespace issue was found in one of more of the commits." echo "Run the following command to resolve whitespace issues:" echo "git rebase --whitespace=fix ${goodParent}" -- 2.45.1
