patch, 2 partstravis-ci: fail if Coccinelle found something to transform

> Coccinelle's and in turn 'make coccicheck's exit code only indicates
> that Coccinelle managed to finish its analysis without any errors
> (e.g. no unknown --options, no missing files, no syntax errors in the
> semantic patches, etc.), but it doesn't indicate whether it found any
> undesired code patterns to transform or not.  To find out the latter,
> one has to look closer at 'make coccicheck's standard output and look
> for lines like:
>
>    SPATCH result: contrib/coccinelle/<something>.cocci.patch
>
> And this only indicates that there is something to transform, but to
> see what the suggested transformations are one has to actually look
> into those '*.cocci.patch' files.
>
> This makes the automated static analysis build job on Travis CI not
> particularly useful, because it neither draws our attention to
> Coccinelle's findings, nor shows the actual findings.  Consequently,
> new topics introducing undesired code patterns graduated to master
> on several occasions without anyone noticing.
>
> The only way to draw attention in such an automated setting is to fail
> the build job.  Therefore, modify the 'ci/run-static-analysis.sh'
> build script to check all the resulting '*.cocci.patch' files, and
> fail the build job if any of them turns out to be not empty.  Include
> those files' contents, i.e. Coccinelle's suggested transformations, in
> the build job's trace log, so we'll know why it failed.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
>   ci/run-static-analysis.sh | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
>
> diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh
> index fa719c9ef9..5688f261d0 100755
> --- a/ci/run-static-analysis.sh
> +++ b/ci/run-static-analysis.sh
> @@ -7,4 +7,23 @@
>   
>   make --jobs=2 coccicheck
>   
> +set +x
> +
> +fail=
> +for cocci_patch in contrib/coccinelle/*.patch
> +do
> +	if test -s "$cocci_patch"
> +	then
> +		echo "$(tput setaf 1)Coccinelle suggests the following changes in '$cocci_patch':$(tput sgr0)"
> +		cat "$cocci_patch"
> +		fail=UnfortunatelyYes
> +	fi
> +done
> +
> +if test -n "$fail"
> +then
> +	echo "$(tput setaf 1)error: Coccinelle suggested some changes$(tput sgr0)"
> +	exit 1
> +fi
> +
>   save_good_tree

> The only way to draw attention in such an automated setting is to fail
> the build job.  Therefore, modify the 'ci/run-static-analysis.sh'
> build script to check all the resulting '*.cocci.patch' files, and
> fail the build job if any of them turns out to be not empty.  Include
> those files' contents, i.e. Coccinelle's suggested transformations, in
> the build job's trace log, so we'll know why it failed.

> On Mon, Jul 23, 2018 at 3:02 PM SZEDER Gábor <szeder.dev@gmail.com> wrote:
>
>> The only way to draw attention in such an automated setting is to fail
>> the build job.  Therefore, modify the 'ci/run-static-analysis.sh'
>> build script to check all the resulting '*.cocci.patch' files, and
>> fail the build job if any of them turns out to be not empty.  Include
>> those files' contents, i.e. Coccinelle's suggested transformations, in
>> the build job's trace log, so we'll know why it failed.
>
> And this is how it looks like "in action":
>
>   https://travis-ci.org/git/git/jobs/408269979#L570

> bisect--helper: use oid_to_hex()
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
>   builtin/bisect--helper.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index fc02f889e6..eac4c27787 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -547,7 +547,7 @@ static int bisect_start(struct bisect_terms *terms, int no_checkout,
>   		if (!get_oid(head, &head_oid) &&
>   		    !starts_with(head, "refs/heads/")) {
>   			strbuf_reset(&start_head);
> -			strbuf_addstr(&start_head, sha1_to_hex(head_oid.hash));
> +			strbuf_addstr(&start_head, oid_to_hex(&head_oid));
>   		} else if (!get_oid(head, &head_oid) &&
>   			   skip_prefix(head, "refs/heads/", &head)) {
>   			/*

> The first patch makes the static analysis build job on Travis CI
> faster by running it with 'make -j2'.
>
> The second patch makes it more more useful by failing the build job if
> Coccinelle finds something to transform, thereby drawing our attention
> to undesired code patterns trying to enter the codebase.
>
>
> With these patches applied, the static analysis build job would fail
> on current 'pu' because of two small issues on two branches:
>
>    - js/range-diff: Dscho has sent out v4 of this series over the
>      weekend, which already incorporates Coccinelle's suggestion, so
>      it's basically done.
>    
>    - pb/bisect-helper-2: this topic has not seen an update in about 9
>      months, so I'll send a followup patch 3/2 to be applied on top or
>      squashed in, whichever is deemed better.
>
>
> SZEDER Gábor (2):
>    travis-ci: run Coccinelle static analysis with two parallel jobs
>    travis-ci: fail if Coccinelle static analysis found something to
>      transform
>
>   ci/run-static-analysis.sh | 21 ++++++++++++++++++++-
>   1 file changed, 20 insertions(+), 1 deletion(-)