On Fri, May 03, 2019 at 11:32:32AM +0200, Johannes Schindelin wrote:
> Hi,
>
> On Thu, 2 May 2019, SZEDER Gábor wrote:
>
> > On Wed, May 01, 2019 at 06:01:08AM -0400, Denton Liu wrote:
> > > > Is it not possible to exclude certain directories for certain semantic
> > > > patches?
> > > >
> > > > I guess we could also simply declare that *all* Coccinelle patches should
> > > > leave `compat/` alone, on the basis that those files are likely coming
> > > > from some sort of upstream. But then, `compat/mingw.c` and `compat/win32/`
> > > > seem not to fall into that category...
> > > >
> > > > Ciao,
> > > > Dscho
> > >
> > > Deciding whether this is a good idea is above my paygrade ;)
>
> :-)
>
> As a software developer, you surely have an opinion, though :-D
I don't even have an opinion yet. :)
> > FWIW, out of curiosity I've run 'make coccicheck' on Linux with
> > 'compat/mingw.c' and its friends explicitly added to C_SOURCES, and it
> > seems to work... it even found two places in 'mingw.c' where
> > COPY_ARRAY could replace memcpy() :)
>
> TBH I had not even known that those files were excluded from coccicheck by
> default.
Well, there was this line in the patch context:
C_SOURCES = $(patsubst %.o,%.c,$(C_OBJ))
> I had assumed that all of Git's sources (and not just the
> Linux-specific ones) were included in the target.
>
> Since you *could* include it, I now assume that Coccinelle does not need
> to follow the `#include`s (otherwise, it would have complained about not
> finding the `windows.h` header in your setup).
We invoke Coccinelle/spatch with the '--all-includes' option, see the
SPATCH_FLAGS make variable. And it does indeed include header files:
I've seen it find something to transform in 'cache.h', and then the
resulting 'contrib/coccinelle/<whatever>.cocci.patch' included the
exact same transformation as many times as the number of files
including 'cache.h'... which is a lot :)
I don't really know what can cause 'spatch' to error out (besides
unknown command line option or non-existing file specified on the
command line), and this is all that 'man spatch' has to say:
--all-includes
causes all available include files to be used
Since it explicitly mentions _available_ include files, I would
venture to guess that non-available include files are not used, and
since it doesn't explicitly mention that such a file causes an error,
it doesn't.
> If this new assumption is true, I wonder why we cannot make my former
> assumption true as well: why not include *all* of Git's `.c` files in
> `coccicheck`?
>
> Ciao,
> Dscho
