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. Gábor