Skip to content

Commit 30c4f7e

Browse files
KarthikNayakgitster
authored andcommitted
check-whitespace: detect if no base_commit is provided
The 'check-whitespace' CI script exits 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 fallback to 'CI_MERGE_REQUEST_DIFF_BASE_SHA' if "CI_MERGE_REQUEST_TARGET_BRANCH_SHA" isn't available in GitLab CI, similar to the previous commit. 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 Helped-by: Junio C Hamano <[email protected]> Signed-off-by: Karthik Nayak <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent bce7e52 commit 30c4f7e

File tree

2 files changed

+14
-3
lines changed

2 files changed

+14
-3
lines changed

.gitlab-ci.yml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,13 @@ check-whitespace:
118118
image: ubuntu:latest
119119
before_script:
120120
- ./ci/install-dependencies.sh
121+
# Since $CI_MERGE_REQUEST_TARGET_BRANCH_SHA is only defined for merged
122+
# pipelines, we fallback to $CI_MERGE_REQUEST_DIFF_BASE_SHA, which should
123+
# be defined in all pipelines.
121124
script:
122-
- ./ci/check-whitespace.sh "$CI_MERGE_REQUEST_TARGET_BRANCH_SHA"
125+
- |
126+
R=${CI_MERGE_REQUEST_TARGET_BRANCH_SHA-${CI_MERGE_REQUEST_DIFF_BASE_SHA:?}} || exit
127+
./ci/check-whitespace.sh "$R"
123128
rules:
124129
- if: $CI_PIPELINE_SOURCE == 'merge_request_event'
125130

ci/check-whitespace.sh

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ baseCommit=$1
99
outputFile=$2
1010
url=$3
1111

12-
if test "$#" -ne 1 && test "$#" -ne 3
12+
if test "$#" -ne 1 && test "$#" -ne 3 || test -z "$1"
1313
then
1414
echo "USAGE: $0 <BASE_COMMIT> [<OUTPUT_FILE> <URL>]"
1515
exit 1
@@ -21,6 +21,12 @@ commitText=
2121
commitTextmd=
2222
goodParent=
2323

24+
if ! git rev-parse --quiet --verify "${baseCommit}"
25+
then
26+
echo "Invalid <BASE_COMMIT> '${baseCommit}'"
27+
exit 1
28+
fi
29+
2430
while read dash sha etc
2531
do
2632
case "${dash}" in
@@ -67,7 +73,7 @@ then
6773
goodParent=${baseCommit: 0:7}
6874
fi
6975

70-
echo "A whitespace issue was found in onen of more of the commits."
76+
echo "A whitespace issue was found in one or more of the commits."
7177
echo "Run the following command to resolve whitespace issues:"
7278
echo "git rebase --whitespace=fix ${goodParent}"
7379

0 commit comments

Comments
 (0)