Re: [PATCH v3 0/4] remove extern from function declarations

From: Denton Liu <liu.denton@gmail.com>
Date: May 1, 2019, 10:01 UTC
Message-ID: <20190501100108.GA8954@archbookpro.localdomain>
In-Reply-To: <nycvar.QRO.7.76.6.1904301919580.45@tvgsbejvaqbjf.bet>

Hi Johannes,

On Tue, Apr 30, 2019 at 07:21:40PM -0400, Johannes Schindelin wrote:

Show 41 quoted lines

> Hi,
> 
> On Thu, 25 Apr 2019, SZEDER Gábor wrote:
> 
> > On Mon, Apr 22, 2019 at 05:49:01PM -0400, Jeff King wrote:
> > > On Wed, Apr 17, 2019 at 12:58:31AM -0700, Denton Liu wrote:
> >
> > > >  compat/mingw.c                    |   2 +-
> > > >  compat/mingw.h                    |   6 +-
> > > >  compat/nedmalloc/malloc.c.h       |   6 +-
> > > >  compat/obstack.h                  |  14 +-
> > > >  compat/poll/poll.h                |   2 +-
> > > >  compat/regex/regex.h              |  66 ++---
> > > >  compat/win32/pthread.h            |   8 +-
> > >
> > > We sometimes avoid touching compat/ code for style issues because it's
> > > copied from elsewhere. And diverging from upstream is more evil than a
> > > pure style issue. So potentially we could drop these hunks (though I
> > > think maybe mingw is our own thing?).
> > >
> > > >  contrib/coccinelle/noextern.cocci |   6 +
> > >
> > > I have mixed feelings on this cocci script.
> >
> > I have actual bad experience with this :)
> >
> > v4 of this patch series excluded 'compat/' from the conversion, but
> > the semantic patch is applied to 'compat/' all the same, resulting in
> > failed CI builds because of the four 'extern's in 'compat/obstack.h',
> > and will continue to do so.
> 
> 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 ;)

However, if this is a good idea, we could use this patch to make it happen:

-- >8 --
Subject: [PATCH] Makefile: filter out compat/ from coccicheck

Since most files in compat/ are pulled from external sources, ensure that they do not get modified when we run coccicheck because we do not want them to differ from upstream as much as possible.

Make exceptions for mingw.c and win32/*.c as these are files that we have created and not pulled from upstream.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 Makefile | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 9f1b6e8926..b083c038c3 100644
--- a/Makefile
+++ b/Makefile
@@ -2782,11 +2782,14 @@ check: command-list.h
 	fi
 
 C_SOURCES = $(patsubst %.o,%.c,$(C_OBJ))
+COCCI_COMPAT_SOURCES = $(addprefix compat/,mingw.c win32/%)
 ifdef DC_SHA1_SUBMODULE
-COCCI_SOURCES = $(filter-out sha1collisiondetection/%,$(C_SOURCES))
+COCCI_SOURCES := $(filter-out sha1collisiondetection/%,$(C_SOURCES))
 else
-COCCI_SOURCES = $(filter-out sha1dc/%,$(C_SOURCES))
+COCCI_SOURCES := $(filter-out sha1dc/%,$(C_SOURCES))
 endif
+COCCI_SOURCES := $(filter-out compat/%,$(COCCI_SOURCES))
+COCCI_SOURCES += $(filter $(COCCI_COMPAT_SOURCES),$(C_SOURCES))
 
 %.cocci.patch: %.cocci $(COCCI_SOURCES)
 	@echo '    ' SPATCH $<; \

-- 
2.21.0.1033.g0e8cc1100c