On Wed, May 08, 2019 at 08:36:10AM -0400, Denton Liu wrote:

> >  %.cocci.patch: %.cocci $(COCCI_SOURCES)
> >     @echo '    ' SPATCH $<; \
> > -   if ! echo $(COCCI_SOURCES) | xargs -n $(SPATCH_BATCH_SIZE) \
> > +   if test $(SPATCH_BATCH_SIZE) = 0; then \
> > +           limit=; \
> > +   else \
> > +           limit='-n $(SPATCH_BATCH_SIZE)'; \
> > +   fi; \
> 
> Could we pull `limit` out of the recipe and into a make variable? You
> mentioned earlier that you wanted to do this but it was too complicated
> but now that it's written like this, it seem like it'd be pretty easy to
> do.

Yes, we could, because this part of it is a straight text substitution
in the recipe, and not a conditional. I don't know that it's any more
readable, though. Either that same conditional gets split out like:

  ifeq ($(SPATCH_BATCH_SIZE),0)
  SPATCH_LIMIT =
  else
  SPATCH_LIMIT = -n $(SPATCH_BATCH_SIZE)
  endif

or we do it inline, but IMHO it's still pretty cluttered:

diff --git a/Makefile b/Makefile
index 9cea614523..aedc7b80a8 100644
--- a/Makefile
+++ b/Makefile
@@ -2793,12 +2793,8 @@ endif
 
 %.cocci.patch: %.cocci $(COCCI_SOURCES)
        @echo '    ' SPATCH $<; \
-       if test $(SPATCH_BATCH_SIZE) = 0; then \
-               limit=; \
-       else \
-               limit='-n $(SPATCH_BATCH_SIZE)'; \
-       fi; \
-       if ! echo $(COCCI_SOURCES) | xargs $$limit \
+       if ! echo $(COCCI_SOURCES) | \
+            xargs $(if $(filter 0,$(SPATCH_BATCH_SIZE)),,-n 
$(SPATCH_BATCH_SIZE)) \
                $(SPATCH) --sp-file $< $(SPATCH_FLAGS) \
                >$@+ 2>$@.log; \
        then \

It gets a little better if we make the sentinel value the empty string
instead of "0" (you can drop the filter).

-Peff

Reply via email to