Re: [PATCH] Makefile: avoid running curl-config unnecessarily

> On Sat, Apr 04, 2020 at 03:38:00PM +0200, Johannes Schindelin wrote:
>
> > >   [1/2]: Makefile: avoid running curl-config multiple times
> > >   [2/2]: Makefile: use curl-config --cflags
> >
> > I _suspect_ that this is responsible for the build failure
> >
> > 	make: curl-config: Command not found
> >
> > at https://github.com/git/git/runs/556459415#step:4:674
> >
> > Do we need this to fix this?
>
> Hmm, yeah. It's an unfortunate side effect of the ":=" assignment to
> stop repeatedly invoking curl-config. Now it's only invoked once, but
> it's _always_ once, even if we're not building anything that needs it.
>
> I wonder if there's a way to expand a Makefile variable lazily, but only
> once...aha, with some help from the Internet, I came up with the patch
> below.

> > -- snip --
> > diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh
> > index de41888430a..325b4cc6185 100755
> > --- a/ci/test-documentation.sh
> > +++ b/ci/test-documentation.sh
> > @@ -11,6 +11,7 @@ filter_log () {
> >  	    -e '/^    \* new asciidoc flags$/d' \
> >  	    -e '/stripped namespace before processing/d' \
> >  	    -e '/Attributed.*IDs for element/d' \
> > +	    -e '/curl-config: Command not found/d' \
> >  	    "$1"
> >  }
>
> Yes, this works, but it's rather unfortunate that we're invoking
> curl-config when it's not needed. Perhaps using NO_CURL in the
> documentation job would be better. But if the patch below isn't too
> disgusting, I think I prefer that approach (because it helps _any_
> top-level make invocation that isn't actually building http binaries).

> Junio, you may want to hold off on moving jk/build-with-right-curl to
> next until we resolve this one way or the other.
>
> -- >8 --
> Subject: [PATCH] Makefile: avoid running curl-config unnecessarily
>
> Commit 94a88e2524 (Makefile: avoid running curl-config multiple times,
> 2020-03-26) put the call to $(CURL_CONFIG) into a "simple" variable
> which is expanded immediately, rather than expanding it each time it's
> needed. However, that also means that we expand it whenever the Makefile
> is parsed, whether we need it or not.
>
> This is wasteful, but also breaks the ci/test-documentation.sh job, as
> it does not have curl at all and complains about the extra messages to
> stderr. An easy way to see it is just:
>
>   $ make CURL_CONFIG=does-not-work check-builtins
>   make: does-not-work: Command not found
>   make: does-not-work: Command not found
>   GIT_VERSION = 2.26.0.108.gb3f3f45f29
>   make: does-not-work: Command not found
>   make: does-not-work: Command not found
>   ./check-builtins.sh
>
> We can get the best of both worlds if we're willing to accept a little
> Makefile hackery. Courtesy of the article at:
>
>   http://make.mad-scientist.net/deferred-simple-variable-expansion/
>
> this patch uses a lazily-evaluated recursive variable which replaces its
> contents with an immediately assigned simple one on first use.
>
> Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> This is our first use of eval in the Makefile, but that goes back to GNU
> make v3.80, which is from 2002. I think that should be OK.
>
> If this inlining is too gross, we could probably contain it in a
> function where callers would do something like:
>
>   $(eval lazy-shell-var, CURL_LDFLAGS, $(CURL_CONFIG) --libs)
>
> That's better in the sense that there's less gobbledygook at each
> callsite, but worse in that it obscures the fact that it's a variable
> assignment. I'd probably consider going that direction if we started
> growing more use-cases than these two.
>
> We could probably also stuff this into a sh
>  Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 5099f6a8f8..97dbdcd201 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1366,12 +1366,12 @@ else
>  	endif
>
>  	ifndef CURL_LDFLAGS
> -		CURL_LDFLAGS := $(shell $(CURL_CONFIG) --libs)
> +		CURL_LDFLAGS = $(eval CURL_LDFLAGS := $$(shell $$(CURL_CONFIG) --libs))$(CURL_LDFLAGS)
>  	endif
>  	CURL_LIBCURL += $(CURL_LDFLAGS)
>
>  	ifndef CURL_CFLAGS
> -		CURL_CFLAGS := $(shell $(CURL_CONFIG) --cflags)
> +		CURL_CFLAGS = $(eval CURL_CFLAGS := $$(shell $$(CURL_CONFIG) --cflags))$(CURL_CFLAGS)
>  	endif
>  	BASIC_CFLAGS += $(CURL_CFLAGS)
>
> --
> 2.26.0.410.gc279fb3cbd
>
>