Re: [PATCH 8/8] check-whitespace: detect if no base_commit is provided

Justin Tobler [off-list ref] writes:

On 24/07/08 11:23AM, Karthik Nayak wrote:

quoted

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.

s/exists/exits

If no base commit is provided, we already fail. Here is an example
GitLab CI job demonstrating this:
https://gitlab.com/gitlab-org/git/-/jobs/7289543498#L2370

If the base commit does not exist though, it currently prints that error occured
but still exits with 0. Makes sense to update and fail the job accordingly.

This example is running on the 'edb990d9', whose parent's '8d9bcf0a'
parent '57fdb00c' contains my changes, specifically the `|| test -z "$1"`
section. You can check this on master locally though.

    $ ./ci/check-whitespace.sh ""
    fatal: ..: '..' is outside repository at '/home/karthik/git'

    $ echo $?
    0

or for invalid value:

    $ ./ci/check-whitespace.sh "random"
    fatal: ambiguous argument 'random..': unknown revision or path not
in the working tree.
    Use '--' to separate paths from revisions, like this:
    'git <command> [<revision>...] -- [<file>...]'

    $ echo $status
    0

[snip]

I might be misunderstanding, but this additional check seems redundant to me.

It is required, as commented above

quoted

 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."

There is another preexisting typo:

s/one of/one or/

Thanks. will add