Junio C Hamano <gitster@xxxxxxxxx> writes:
> SZEDER Gábor <szeder.dev@xxxxxxxxx> writes:
>
>> On Fri, Dec 1, 2017 at 6:59 AM, Kaartic Sivaraam
>> <kaartic.sivaraam@xxxxxxxxx> wrote:
>>> Sorry, missed a ';' in v4.
>>>
>>> The surprising thing I discovered in the TravisCI build for v4
>>> was that apart from the 'Documentation' build the 'Static Analysis'
>>> build passed, with the following output,
>>>
>>> -- <snip>
>>> $ ci/run-static-analysis.sh
>>> GIT_VERSION = 2.13.1.1972.g6ced3f745
>>> SPATCH contrib/coccinelle/array.cocci
>>> SPATCH result: contrib/coccinelle/array.cocci.patch
>>> SPATCH contrib/coccinelle/free.cocci
>>> SPATCH contrib/coccinelle/object_id.cocci
>>> SPATCH contrib/coccinelle/qsort.cocci
>>> SPATCH contrib/coccinelle/strbuf.cocci
>>> SPATCH result: contrib/coccinelle/strbuf.cocci.patch
>>> SPATCH contrib/coccinelle/swap.cocci
>>> SPATCH contrib/coccinelle/xstrdup_or_null.cocci
>>>
>>> The command "ci/run-static-analysis.sh" exited with 0.
>>
>> Perhaps Coccinelle should have errored out, or perhaps its 0 exit code
>> means "I didn't find any code matching any of the semantic patches that
>> required transformation".
>>
>>> I guess static analysis tools make an assumption that the source
>>> code is syntactically valid for them to work correctly. So, I guess
>>> we should at least make sure the code 'compiles' before running
>>> the static analysis tool even though we don't build it completely.
>>> I'm not sure if it's a bad thing to run the static analysis on code
>>> that isn't syntactically valid, though.
>>
>> Travis CI already runs 6 build jobs compiling Git. And that is in
>> addition to the one that you should have run yourself before even
>> thinking about submitting v4 ;) That's plenty to catch errors like
>> these. And if any of those builds fail because Git can't be built or
>> because of a test failure, then Coccinelle's success doesn't matter at
>> all, because the commit is toast anyway.
>
> Somehow this fell underneath my radar horizon. I see v4 and v5 of
> 4/4 but do not seem to find 1-3/4. Is this meant to be a standalone
> patch, or am I expected to already have 1-3 that we already are
> committed to take?
Ah, I am guessing that this would apply on top of 1-3/4 in the
thread with <20171118172648.17918-1-kaartic.sivaraam@xxxxxxxxx>
The base of the series seems to predate 16169285 ("Merge branch
'jc/branch-name-sanity'", 2017-11-28), so let me see how it looks by
applying those three plus this one on top of 'master' before that
point.
