Subject: Re: [PATCH v4 3/4] Makefile/coccicheck: allow for setting xargs concurrency

From: Jeff King <[email-masked]>
Date: Wed, 24 Mar 2021 15:26:02 -0400

On Mon, Mar 22, 2021 at 01:11:49PM +0100, Ævar Arnfjörð Bjarmason wrote:

> Extend the SPATCH_BATCH_SIZE facility added in
> 960154b9c17 (coccicheck: optionally batch spatch invocations,
> 2019-05-06) and bcb4edf7af7 (coccicheck: make batch size of 0 mean
> "unlimited", 2019-05-08) to allow for both setting
> SPATCH_BATCH_SIZE=N, and also to have a more advanced SPATCH_XARGS
> argument.
> 
> The reason to replace the "$$limit" is that now you actually see under
> V=1 what argument your program will get invoked with.
> 
> The reason for the "9999" limit is that if you e.g. try to define an
> "$(XARGS)" which is conditionally an empty string or not depending on
> this setting then e.g.:
> 
>     echo $(FILES) | $(XARGS) $(XARGS_FLAGS) $(SPATCH)
> 
> Over multiple lines with "\" will error out. I think just setting it
> to "xargs -n 9999" as a trivial workaround is the best solution here.

I don't understand this 9999 comment. The original was sometimes setting
$limit to the empty string, and then doing:

 xargs $limit

How is that any different than setting SPATCH_XARGS to just "xargs" for
the unlimited case?

> +# For the 'coccicheck' target; SPATCH_XARGS can be used to manually
> +# tweak the xargs invocation. By default we invoke "xargs -n 1", and
> +# "xargs -n 9999" under SPATCH_BATCH_SIZE=0.
> +#
> +# Setting SPATCH_XARGS overrides SPATCH_BATCH_SIZE. To get concurrency
> +# when targeting a single contrib/coccinelle/%.patch use e.g. "-P" if
> +# your xargs(1) supports it:
> +#
> +#    make contrib/coccinelle/strbuf.cocci.patch SPATCH_XARGS="xargs -P 8 -n 8"
> +#
> +# Or a combination -jN and "xargs -P":
> +#
> +#    make -j4 coccicheck SPATCH_XARGS="xargs -P 2 -n 8"

As I mentioned in the last round, using "-P" is racy. I'm not sure if
it's something we should be recommending to people.

-Peff