All of lore.kernel.org
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Denton Liu <liu.denton@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Jeff King <peff@peff.net>, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 2/2] Makefile: run coccicheck on more source files
Date: Tue, 10 Sep 2019 16:18:28 +0200	[thread overview]
Message-ID: <20190910141828.GK32087@szeder.dev> (raw)
In-Reply-To: <f62b0c7d1774cefc66e519430515eeb64acad1e0.1568101393.git.liu.denton@gmail.com>

On Tue, Sep 10, 2019 at 12:44:31AM -0700, Denton Liu wrote:
> Make the "coccicheck" target run on all C sources except for those that
> are taken from some upstream. We don't want to patch these files since
> we want them to be as close to upstream as possible so that it'll be
> easier to pull in upstream updates.

> diff --git a/Makefile b/Makefile
> index 708df2c289..d468b7c9c4 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2802,12 +2802,8 @@ check: command-list.h
>  		exit 1; \
>  	fi
>  
> -C_SOURCES = $(patsubst %.o,%.c,$(C_OBJ))
> -ifdef DC_SHA1_SUBMODULE
> -COCCI_SOURCES = $(filter-out sha1collisiondetection/%,$(C_SOURCES))
> -else
> -COCCI_SOURCES = $(filter-out sha1dc/%,$(C_SOURCES))
> -endif
> +FIND_C_SOURCES = $(filter %.c,$(shell $(FIND_SOURCE_FILES)))

Hrm, so this uses FIND_SOURCE_FILES, which first attempts to run 'git
ls-files' and if that fails it falls back to run 'find'.

Unfortunately, the output of the two slightly differ: 'git ls-files'
prints 'abspath.c advice.c alias.c ...' why 'find' prints
'./upload-pack.c ./unpack-trees.c ./gpg-interface.c ...'.  Now, while
the order of files doesn't matter, the './' prefix does, because:

> +COCCI_SOURCES = $(filter-out $(UPSTREAM_SOURCES),$(FIND_C_SOURCES))

Here the paths/patterns in UPSTREAM_SOURCES don't have that './'
prefix, and thus won't match and won't filter out any of the upstream
files that they are supposed to.  IOW, if someone runs 'make
coccicheck' on a system without Git installed, then Coccinelle will
check all upstream sources as well, and will e.g. suggest using
COPY_ARRAY in 'compat/regex/regexec.c'.

Now, running 'make coccicheck' on a git.git clone without Git
installed might look like quite a pathological case on the first
sight, but I would argue that it is not that pathological: e.g.
consider someone running the recent Coccinelle version in a small-ish
Docker image containing just enough to run 'make coccicheck', but not
Git.  (yeah, you guessed right, I am that someone ;)

I don't know how to convince 'find' to omit that './' prefix from each
listed file, and in a portable way at that.  Piping its output through
'sed' or even 'cut' easily takes care of it, though.

>  %.cocci.patch: %.cocci $(COCCI_SOURCES)
>  	@echo '    ' SPATCH $<; \
> -- 
> 2.23.0.248.g3a9dd8fb08
> 

  parent reply	other threads:[~2019-09-10 14:18 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-10  7:44 [PATCH 0/2] Makefile: run coccicheck on all non-upstream sources Denton Liu
2019-09-10  7:44 ` [PATCH 1/2] Makefile: define UPSTREAM_SOURCES Denton Liu
2019-09-10  7:44 ` [PATCH 2/2] Makefile: run coccicheck on more source files Denton Liu
2019-09-10 13:28   ` SZEDER Gábor
2019-09-10 16:07     ` Denton Liu
2019-09-10 14:18   ` SZEDER Gábor [this message]
2019-09-12 17:28 ` [PATCH v2 0/3] Makefile: run coccicheck on all non-upstream sources Denton Liu
2019-09-12 17:28   ` [PATCH v2 1/3] Makefile: define THIRD_PARTY_SOURCES Denton Liu
2019-09-12 21:42     ` Junio C Hamano
2019-09-12 17:28   ` [PATCH v2 2/3] Makefile: strip leading ./ in $(FIND_SOURCE_FILES) Denton Liu
2019-09-12 18:18     ` Junio C Hamano
2019-09-13 12:05     ` SZEDER Gábor
2019-09-12 17:28   ` [PATCH v2 3/3] Makefile: run coccicheck on more source files Denton Liu
2019-09-12 18:40     ` Junio C Hamano
2019-09-13 11:49       ` SZEDER Gábor
2019-09-13 17:14         ` Denton Liu
2019-09-13 18:00           ` SZEDER Gábor
2019-09-13 21:38             ` Denton Liu
2019-09-13 17:38         ` Junio C Hamano
2019-09-16 19:23   ` [PATCH v3 0/4] Makefile: run coccicheck on all non-upstream sources Denton Liu
2019-09-16 19:23     ` [PATCH v3 1/4] Makefile: strip leading ./ in $(LIB_H) Denton Liu
2019-09-16 19:23     ` [PATCH v3 2/4] Makefile: define THIRD_PARTY_SOURCES Denton Liu
2019-09-16 20:56       ` Junio C Hamano
2019-09-16 22:00         ` [PATCH] fixup! " Denton Liu
2019-09-16 19:23     ` [PATCH v3 3/4] Makefile: strip leading ./ in $(FIND_SOURCE_FILES) Denton Liu
2019-09-16 19:23     ` [PATCH v3 4/4] Makefile: run coccicheck on more source files Denton Liu
2019-09-16 20:57     ` [PATCH v3 0/4] Makefile: run coccicheck on all non-upstream sources Junio C Hamano
2019-09-17  8:18       ` SZEDER Gábor
2019-09-17 16:13         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190910141828.GK32087@szeder.dev \
    --to=szeder.dev@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=liu.denton@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.