{"thread":{"id":"53105","subject":"[PATCH 0/2] use `curl-config --cflags`","startedAt":"2020-03-26T08:05:43Z","lastAt":"2020-04-15T21:57:37Z","messageCount":13,"participants":["Jeff King","Junio C Hamano","Johannes Schindelin","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"394059","messageId":"20200326080540.GA2200522@coredump.intra.peff.net","threadId":"53105","inReplyTo":null,"subject":"[PATCH 0/2] use `curl-config --cflags`","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-03-26T08:05:40Z","receivedAt":"2020-03-26T08:05:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"I was recently testing Git's behavior with respect to various versions\nof libcurl. So I built a one-off libcurl and installed it in /tmp/foo,\nbut was surprised that:\n\n  make CURL_CONFIG=/tmp/foo/bin/curl-config\n\ndidn't work, since we do run \"$(CURL_CONFIG) --libs\". This fixes it,\nalong with a minor optimization to the existing \"--libs\" call.\n\n  [1/2]: Makefile: avoid running curl-config multiple times\n  [2/2]: Makefile: use curl-config --cflags\n\n Makefile | 15 ++++++++++-----\n 1 file changed, 10 insertions(+), 5 deletions(-)\n\n-Peff\n"},{"id":"394060","messageId":"20200326080645.GA2200716@coredump.intra.peff.net","threadId":"53105","inReplyTo":"20200326080540.GA2200522@coredump.intra.peff.net","subject":"[PATCH 1/2] Makefile: avoid running curl-config multiple times","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-03-26T08:06:45Z","receivedAt":"2020-03-26T08:06:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If the user hasn't set the CURL_LDFLAGS Makefile variable, we invoke\ncurl-config like this:\n\n  CURL_LIBCURL += $(shell $(CURL_CONFIG) --libs)\n\nBecause the shell function is run when the value is expanded, we invoke\ncurl-config each time we need to link something (which generally ends up\nbeing four times for a full build).\n\nInstead, let's use an immediate Makefile variable, which only needs\nexpanding once. We can't combine that with the existing \"+=\", but since\nwe only do this when CURL_LDFLAGS is undefined, we can just set that\nvariable.\n\nThat also allows us to simplify our conditional a bit, since both sides\nwill then put the result into CURL_LIBCURL. While we're touching it,\nlet's fix the indentation to match the nearby code (we're inside an\nouter conditional, so everything else is indented one level).\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n Makefile | 7 +++----\n 1 file changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex a5961113d8..93a8ef3a72 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1365,11 +1365,10 @@ else\n \t\tCURL_LIBCURL =\n \tendif\n \n-ifdef CURL_LDFLAGS\n+\tifndef CURL_LDFLAGS\n+\t\tCURL_LDFLAGS := $(shell $(CURL_CONFIG) --libs)\n+\tendif\n \tCURL_LIBCURL += $(CURL_LDFLAGS)\n-else\n-\tCURL_LIBCURL += $(shell $(CURL_CONFIG) --libs)\n-endif\n \n \tREMOTE_CURL_PRIMARY = git-remote-http$X\n \tREMOTE_CURL_ALIASES = git-remote-https$X git-remote-ftp$X git-remote-ftps$X\n-- \n2.26.0.576.gb87790c3c1\n\n"},{"id":"394061","messageId":"20200326080855.GB2200716@coredump.intra.peff.net","threadId":"53105","inReplyTo":"20200326080540.GA2200522@coredump.intra.peff.net","subject":"[PATCH 2/2] Makefile: use curl-config --cflags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-03-26T08:08:55Z","receivedAt":"2020-03-26T08:08:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We add the result of \"curl-config --libs\" when linking curl programs,\nbut we never bother calling \"curl-config --cflags\". Presumably nobody\nnoticed because:\n\n  - a system libcurl installed into /usr/include/curl wouldn't need any\n    flags (\"/usr/include\" is already in the search path, and the\n    #include lines all look <curl/curl.h>, etc).\n\n  - using CURLDIR sets up both the includes and the library path\n\nHowever, if you prefer CURL_CONFIG to CURLDIR, something simple like:\n\n  make CURL_CONFIG=/path/to/curl-config\n\ndoesn't work. We'd link against the libcurl specified by that program,\nbut not find its header files when compiling.\n\nLet's invoke \"curl-config --cflags\" similar to the way we do for\n\"--libs\". Note that we'll feed the result into BASIC_CFLAGS. The rest of\nthe Makefile doesn't distinguish which files need curl support during\ncompilation and which do not. That should be OK, though. At most this\nshould be adding a \"-I\" directive, and this is how CURLDIR already\nbehaves. And since we follow the immediate-variable pattern from\nCURL_LDFLAGS, we won't accidentally invoke curl-config once per\ncompilation.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI didn't bother touching configure.ac here, because I don't think it\nwould do anything useful. The existing configure script will run\n\"curl-config --libs\" to set CURL_LDFLAGS, suppressing the call to\ncurl-config inside the Makefile. But with the caveat that you can\nconvince it to pass something besides --libs (though what would be\nuseful there, I have no idea, and the commit introducing it didn't shed\nany light). So following that pattern, at most we'd just be trading a\ncall during configure time for one inside the Makefile.\n\nWhat _could_ be useful is if the configure script used curl-config as\npart of it's check of whether we have libcurl at all. But it doesn't. We\ndon't even look for curl-config until we've been able to link against\n-lcurl. I have a feeling this could be fixed by reversing the order of\nthe blocks, but I'm not sure of all of the subtle interactions with\nCURLDIR. And I don't care enough about autoconf to puzzle it out. We\ncertainly aren't making anything worse with this patch.\n\n Makefile | 8 +++++++-\n 1 file changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 93a8ef3a72..4550bf4a42 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1359,9 +1359,10 @@ ifdef NO_CURL\n else\n \tifdef CURLDIR\n \t\t# Try \"-Wl,-rpath=$(CURLDIR)/$(lib)\" in such a case.\n-\t\tBASIC_CFLAGS += -I$(CURLDIR)/include\n+\t\tCURL_CFLAGS = -I$(CURLDIR)/include\n \t\tCURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib)\n \telse\n+\t\tCURL_CFLAGS =\n \t\tCURL_LIBCURL =\n \tendif\n \n@@ -1370,6 +1371,11 @@ else\n \tendif\n \tCURL_LIBCURL += $(CURL_LDFLAGS)\n \n+\tifndef CURL_CFLAGS\n+\t\tCURL_CFLAGS := $(shell $(CURL_CONFIG) --cflags)\n+\tendif\n+\tBASIC_CFLAGS += $(CURL_CFLAGS)\n+\n \tREMOTE_CURL_PRIMARY = git-remote-http$X\n \tREMOTE_CURL_ALIASES = git-remote-https$X git-remote-ftp$X git-remote-ftps$X\n \tREMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES)\n-- \n2.26.0.576.gb87790c3c1\n"},{"id":"394209","messageId":"xmqq7dz5e9c9.fsf@gitster.c.googlers.com","threadId":"53105","inReplyTo":"20200326080540.GA2200522@coredump.intra.peff.net","subject":"Re: [PATCH 0/2] use `curl-config --cflags`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-27T22:12:06Z","receivedAt":"2020-03-27T22:12:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I was recently testing Git's behavior with respect to various versions\n> of libcurl. So I built a one-off libcurl and installed it in /tmp/foo,\n> but was surprised that:\n>\n>   make CURL_CONFIG=/tmp/foo/bin/curl-config\n>\n> didn't work, since we do run \"$(CURL_CONFIG) --libs\". This fixes it,\n> along with a minor optimization to the existing \"--libs\" call.\n>\n>   [1/2]: Makefile: avoid running curl-config multiple times\n>   [2/2]: Makefile: use curl-config --cflags\n>\n>  Makefile | 15 ++++++++++-----\n>  1 file changed, 10 insertions(+), 5 deletions(-)\n>\n> -Peff\n\nBoth patches made sense.  Thanks.\n"},{"id":"394765","messageId":"nycvar.QRO.7.76.6.2004041535360.46@tvgsbejvaqbjf.bet","threadId":"53105","inReplyTo":"20200326080540.GA2200522@coredump.intra.peff.net","subject":"Re: [PATCH 0/2] use `curl-config --cflags`","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-04-04T13:38:00Z","receivedAt":"2020-04-04T13:38:11Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Thu, 26 Mar 2020, Jeff King wrote:\n\n> I was recently testing Git's behavior with respect to various versions\n> of libcurl. So I built a one-off libcurl and installed it in /tmp/foo,\n> but was surprised that:\n>\n>   make CURL_CONFIG=/tmp/foo/bin/curl-config\n>\n> didn't work, since we do run \"$(CURL_CONFIG) --libs\". This fixes it,\n> along with a minor optimization to the existing \"--libs\" call.\n>\n>   [1/2]: Makefile: avoid running curl-config multiple times\n>   [2/2]: Makefile: use curl-config --cflags\n\nI _suspect_ that this is responsible for the build failure\n\n\tmake: curl-config: Command not found\n\nat https://github.com/git/git/runs/556459415#step:4:674\n\nDo we need this to fix this?\n\n-- snip --\ndiff --git a/ci/test-documentation.sh b/ci/test-documentation.sh\nindex de41888430a..325b4cc6185 100755\n--- a/ci/test-documentation.sh\n+++ b/ci/test-documentation.sh\n@@ -11,6 +11,7 @@ filter_log () {\n \t    -e '/^    \\* new asciidoc flags$/d' \\\n \t    -e '/stripped namespace before processing/d' \\\n \t    -e '/Attributed.*IDs for element/d' \\\n+\t    -e '/curl-config: Command not found/d' \\\n \t    \"$1\"\n }\n\n-- snap --\n\nCiao,\nDscho\n\n>\n>  Makefile | 15 ++++++++++-----\n>  1 file changed, 10 insertions(+), 5 deletions(-)\n>\n> -Peff\n>\n"},{"id":"394778","messageId":"20200404145829.GB679473@coredump.intra.peff.net","threadId":"53105","inReplyTo":"nycvar.QRO.7.76.6.2004041535360.46@tvgsbejvaqbjf.bet","subject":"[PATCH] Makefile: avoid running curl-config unnecessarily","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-04-04T14:58:29Z","receivedAt":"2020-04-04T14:58:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 04, 2020 at 03:38:00PM +0200, Johannes Schindelin wrote:\n\n> >   [1/2]: Makefile: avoid running curl-config multiple times\n> >   [2/2]: Makefile: use curl-config --cflags\n> \n> I _suspect_ that this is responsible for the build failure\n> \n> \tmake: curl-config: Command not found\n> \n> at https://github.com/git/git/runs/556459415#step:4:674\n> \n> Do we need this to fix this?\n\nHmm, yeah. It's an unfortunate side effect of the \":=\" assignment to\nstop repeatedly invoking curl-config. Now it's only invoked once, but\nit's _always_ once, even if we're not building anything that needs it.\n\nI wonder if there's a way to expand a Makefile variable lazily, but only\nonce...aha, with some help from the Internet, I came up with the patch\nbelow.\n\n> -- snip --\n> diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh\n> index de41888430a..325b4cc6185 100755\n> --- a/ci/test-documentation.sh\n> +++ b/ci/test-documentation.sh\n> @@ -11,6 +11,7 @@ filter_log () {\n>  \t    -e '/^    \\* new asciidoc flags$/d' \\\n>  \t    -e '/stripped namespace before processing/d' \\\n>  \t    -e '/Attributed.*IDs for element/d' \\\n> +\t    -e '/curl-config: Command not found/d' \\\n>  \t    \"$1\"\n>  }\n\nYes, this works, but it's rather unfortunate that we're invoking\ncurl-config when it's not needed. Perhaps using NO_CURL in the\ndocumentation job would be better. But if the patch below isn't too\ndisgusting, I think I prefer that approach (because it helps _any_\ntop-level make invocation that isn't actually building http binaries).\n\nJunio, you may want to hold off on moving jk/build-with-right-curl to\nnext until we resolve this one way or the other.\n\n-- >8 --\nSubject: [PATCH] Makefile: avoid running curl-config unnecessarily\n\nCommit 94a88e2524 (Makefile: avoid running curl-config multiple times,\n2020-03-26) put the call to $(CURL_CONFIG) into a \"simple\" variable\nwhich is expanded immediately, rather than expanding it each time it's\nneeded. However, that also means that we expand it whenever the Makefile\nis parsed, whether we need it or not.\n\nThis is wasteful, but also breaks the ci/test-documentation.sh job, as\nit does not have curl at all and complains about the extra messages to\nstderr. An easy way to see it is just:\n\n  $ make CURL_CONFIG=does-not-work check-builtins\n  make: does-not-work: Command not found\n  make: does-not-work: Command not found\n  GIT_VERSION = 2.26.0.108.gb3f3f45f29\n  make: does-not-work: Command not found\n  make: does-not-work: Command not found\n  ./check-builtins.sh\n\nWe can get the best of both worlds if we're willing to accept a little\nMakefile hackery. Courtesy of the article at:\n\n  http://make.mad-scientist.net/deferred-simple-variable-expansion/\n\nthis patch uses a lazily-evaluated recursive variable which replaces its\ncontents with an immediately assigned simple one on first use.\n\nReported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis is our first use of eval in the Makefile, but that goes back to GNU\nmake v3.80, which is from 2002. I think that should be OK.\n\nIf this inlining is too gross, we could probably contain it in a\nfunction where callers would do something like:\n\n  $(eval lazy-shell-var, CURL_LDFLAGS, $(CURL_CONFIG) --libs)\n\nThat's better in the sense that there's less gobbledygook at each\ncallsite, but worse in that it obscures the fact that it's a variable\nassignment. I'd probably consider going that direction if we started\ngrowing more use-cases than these two.\n\nWe could probably also stuff this into a sh\n Makefile | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 5099f6a8f8..97dbdcd201 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1366,12 +1366,12 @@ else\n \tendif\n \n \tifndef CURL_LDFLAGS\n-\t\tCURL_LDFLAGS := $(shell $(CURL_CONFIG) --libs)\n+\t\tCURL_LDFLAGS = $(eval CURL_LDFLAGS := $$(shell $$(CURL_CONFIG) --libs))$(CURL_LDFLAGS)\n \tendif\n \tCURL_LIBCURL += $(CURL_LDFLAGS)\n \n \tifndef CURL_CFLAGS\n-\t\tCURL_CFLAGS := $(shell $(CURL_CONFIG) --cflags)\n+\t\tCURL_CFLAGS = $(eval CURL_CFLAGS := $$(shell $$(CURL_CONFIG) --cflags))$(CURL_CFLAGS)\n \tendif\n \tBASIC_CFLAGS += $(CURL_CFLAGS)\n \n-- \n2.26.0.410.gc279fb3cbd\n\n"},{"id":"394787","messageId":"CAPig+cR_ViZ2GWSs=grT=5bzQq44w+AAbXFsZaM_-CVMCELXQw@mail.gmail.com","threadId":"53105","inReplyTo":"20200404145829.GB679473@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile: avoid running curl-config unnecessarily","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-04-05T01:33:51Z","receivedAt":"2020-04-05T01:34:07Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Apr 4, 2020 at 10:58 AM Jeff King <peff@peff.net> wrote:\n> This is our first use of eval in the Makefile, but that goes back to GNU\n> make v3.80, which is from 2002. I think that should be OK.\n\nUpon reading this, I was worried that it might trip up Mac OS which\nhas (often very) old versions of tools, but it's alright in this case,\nas Apple ships GNU 'make' 3.81 from 2006.\n"},{"id":"394796","messageId":"20200405184441.GA1309425@coredump.intra.peff.net","threadId":"53105","inReplyTo":"CAPig+cR_ViZ2GWSs=grT=5bzQq44w+AAbXFsZaM_-CVMCELXQw@mail.gmail.com","subject":"Re: [PATCH] Makefile: avoid running curl-config unnecessarily","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-04-05T18:44:41Z","receivedAt":"2020-04-05T18:44:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 04, 2020 at 09:33:51PM -0400, Eric Sunshine wrote:\n\n> On Sat, Apr 4, 2020 at 10:58 AM Jeff King <peff@peff.net> wrote:\n> > This is our first use of eval in the Makefile, but that goes back to GNU\n> > make v3.80, which is from 2002. I think that should be OK.\n> \n> Upon reading this, I was worried that it might trip up Mac OS which\n> has (often very) old versions of tools, but it's alright in this case,\n> as Apple ships GNU 'make' 3.81 from 2006.\n\nThey're certainly good at keeping old versions of things around. :) I\nwonder if it's due to a switch from GPLv2 to v3. My understanding is\nthat's why they'll keep bash 3 around forever, and the GPLv3 was\npublished in 2007, between GNU make 3.81 and 3.82.\n\nAnyway, I think we're good, but thanks for double-checking.\n\n-Peff\n"},{"id":"394798","messageId":"CAPig+cSgQQHTX1fyZ7ZMftg4aygXOPnWV1vA5H=GTSKDCb_czg@mail.gmail.com","threadId":"53105","inReplyTo":"20200405184441.GA1309425@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile: avoid running curl-config unnecessarily","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-04-05T19:18:13Z","receivedAt":"2020-04-05T19:18:30Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Apr 5, 2020 at 2:44 PM Jeff King <peff@peff.net> wrote:\n> On Sat, Apr 04, 2020 at 09:33:51PM -0400, Eric Sunshine wrote:\n> > On Sat, Apr 4, 2020 at 10:58 AM Jeff King <peff@peff.net> wrote:\n> > > This is our first use of eval in the Makefile, but that goes back to GNU\n> > > make v3.80, which is from 2002. I think that should be OK.\n> >\n> > Upon reading this, I was worried that it might trip up Mac OS which\n> > has (often very) old versions of tools, but it's alright in this case,\n> > as Apple ships GNU 'make' 3.81 from 2006.\n>\n> They're certainly good at keeping old versions of things around. :) I\n> wonder if it's due to a switch from GPLv2 to v3. My understanding is\n> that's why they'll keep bash 3 around forever, and the GPLv3 was\n> published in 2007, between GNU make 3.81 and 3.82.\n\nThat is my understanding, as well.\n\n> Anyway, I think we're good, but thanks for double-checking.\n\nHistory repeats itself[1].\n\n[1]: https://lore.kernel.org/git/20180802212930.GB32538@sigill.intra.peff.net/\n"},{"id":"394813","messageId":"nycvar.QRO.7.76.6.2004060009200.46@tvgsbejvaqbjf.bet","threadId":"53105","inReplyTo":"20200404145829.GB679473@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile: avoid running curl-config unnecessarily","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-04-05T22:12:06Z","receivedAt":"2020-04-05T22:12:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Sat, 4 Apr 2020, Jeff King wrote:\n\n> On Sat, Apr 04, 2020 at 03:38:00PM +0200, Johannes Schindelin wrote:\n>\n> > >   [1/2]: Makefile: avoid running curl-config multiple times\n> > >   [2/2]: Makefile: use curl-config --cflags\n> >\n> > I _suspect_ that this is responsible for the build failure\n> >\n> > \tmake: curl-config: Command not found\n> >\n> > at https://github.com/git/git/runs/556459415#step:4:674\n> >\n> > Do we need this to fix this?\n>\n> Hmm, yeah. It's an unfortunate side effect of the \":=\" assignment to\n> stop repeatedly invoking curl-config. Now it's only invoked once, but\n> it's _always_ once, even if we're not building anything that needs it.\n>\n> I wonder if there's a way to expand a Makefile variable lazily, but only\n> once...aha, with some help from the Internet, I came up with the patch\n> below.\n\nThanks, that would work.\n\n> > -- snip --\n> > diff --git a/ci/test-documentation.sh b/ci/test-documentation.sh\n> > index de41888430a..325b4cc6185 100755\n> > --- a/ci/test-documentation.sh\n> > +++ b/ci/test-documentation.sh\n> > @@ -11,6 +11,7 @@ filter_log () {\n> >  \t    -e '/^    \\* new asciidoc flags$/d' \\\n> >  \t    -e '/stripped namespace before processing/d' \\\n> >  \t    -e '/Attributed.*IDs for element/d' \\\n> > +\t    -e '/curl-config: Command not found/d' \\\n> >  \t    \"$1\"\n> >  }\n>\n> Yes, this works, but it's rather unfortunate that we're invoking\n> curl-config when it's not needed. Perhaps using NO_CURL in the\n> documentation job would be better. But if the patch below isn't too\n> disgusting, I think I prefer that approach (because it helps _any_\n> top-level make invocation that isn't actually building http binaries).\n\nI believe that we do need to avoid `NO_CURL` lest we miss out on\ndocumentation mismatches in the `check-docs` target.\n\nCiao,\nDscho\n\n> Junio, you may want to hold off on moving jk/build-with-right-curl to\n> next until we resolve this one way or the other.\n>\n> -- >8 --\n> Subject: [PATCH] Makefile: avoid running curl-config unnecessarily\n>\n> Commit 94a88e2524 (Makefile: avoid running curl-config multiple times,\n> 2020-03-26) put the call to $(CURL_CONFIG) into a \"simple\" variable\n> which is expanded immediately, rather than expanding it each time it's\n> needed. However, that also means that we expand it whenever the Makefile\n> is parsed, whether we need it or not.\n>\n> This is wasteful, but also breaks the ci/test-documentation.sh job, as\n> it does not have curl at all and complains about the extra messages to\n> stderr. An easy way to see it is just:\n>\n>   $ make CURL_CONFIG=does-not-work check-builtins\n>   make: does-not-work: Command not found\n>   make: does-not-work: Command not found\n>   GIT_VERSION = 2.26.0.108.gb3f3f45f29\n>   make: does-not-work: Command not found\n>   make: does-not-work: Command not found\n>   ./check-builtins.sh\n>\n> We can get the best of both worlds if we're willing to accept a little\n> Makefile hackery. Courtesy of the article at:\n>\n>   http://make.mad-scientist.net/deferred-simple-variable-expansion/\n>\n> this patch uses a lazily-evaluated recursive variable which replaces its\n> contents with an immediately assigned simple one on first use.\n>\n> Reported-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> This is our first use of eval in the Makefile, but that goes back to GNU\n> make v3.80, which is from 2002. I think that should be OK.\n>\n> If this inlining is too gross, we could probably contain it in a\n> function where callers would do something like:\n>\n>   $(eval lazy-shell-var, CURL_LDFLAGS, $(CURL_CONFIG) --libs)\n>\n> That's better in the sense that there's less gobbledygook at each\n> callsite, but worse in that it obscures the fact that it's a variable\n> assignment. I'd probably consider going that direction if we started\n> growing more use-cases than these two.\n>\n> We could probably also stuff this into a sh\n>  Makefile | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index 5099f6a8f8..97dbdcd201 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1366,12 +1366,12 @@ else\n>  \tendif\n>\n>  \tifndef CURL_LDFLAGS\n> -\t\tCURL_LDFLAGS := $(shell $(CURL_CONFIG) --libs)\n> +\t\tCURL_LDFLAGS = $(eval CURL_LDFLAGS := $$(shell $$(CURL_CONFIG) --libs))$(CURL_LDFLAGS)\n>  \tendif\n>  \tCURL_LIBCURL += $(CURL_LDFLAGS)\n>\n>  \tifndef CURL_CFLAGS\n> -\t\tCURL_CFLAGS := $(shell $(CURL_CONFIG) --cflags)\n> +\t\tCURL_CFLAGS = $(eval CURL_CFLAGS := $$(shell $$(CURL_CONFIG) --cflags))$(CURL_CFLAGS)\n>  \tendif\n>  \tBASIC_CFLAGS += $(CURL_CFLAGS)\n>\n> --\n> 2.26.0.410.gc279fb3cbd\n>\n>\n"},{"id":"394814","messageId":"nycvar.QRO.7.76.6.2004060012120.46@tvgsbejvaqbjf.bet","threadId":"53105","inReplyTo":"CAPig+cR_ViZ2GWSs=grT=5bzQq44w+AAbXFsZaM_-CVMCELXQw@mail.gmail.com","subject":"Re: [PATCH] Makefile: avoid running curl-config unnecessarily","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-04-05T22:12:53Z","receivedAt":"2020-04-05T22:13:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 4 Apr 2020, Eric Sunshine wrote:\n\n> On Sat, Apr 4, 2020 at 10:58 AM Jeff King <peff@peff.net> wrote:\n> > This is our first use of eval in the Makefile, but that goes back to GNU\n> > make v3.80, which is from 2002. I think that should be OK.\n>\n> Upon reading this, I was worried that it might trip up Mac OS which\n> has (often very) old versions of tools, but it's alright in this case,\n> as Apple ships GNU 'make' 3.81 from 2006.\n\nI share this concern. It may be more robust, after all, to squelch the\n`curl-config` warning specifically in the `Documentation` job of our CI/PR\nruns.\n\nCiao,\nDscho\n"},{"id":"395540","messageId":"xmqqzhbc4e0s.fsf@gitster.c.googlers.com","threadId":"53105","inReplyTo":"20200404145829.GB679473@coredump.intra.peff.net","subject":"Re: [PATCH] Makefile: avoid running curl-config unnecessarily","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-15T21:47:47Z","receivedAt":"2020-04-15T21:47:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Junio, you may want to hold off on moving jk/build-with-right-curl to\n> next until we resolve this one way or the other.\n>\n> -- >8 --\n> Subject: [PATCH] Makefile: avoid running curl-config unnecessarily\n\nI think this has been accepted favourably after all?  I am inclined\nto mark these three as ready for 'next'.\n\nThanks.\n"},{"id":"395541","messageId":"20200415215731.GA3595073@coredump.intra.peff.net","threadId":"53105","inReplyTo":"xmqqzhbc4e0s.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] Makefile: avoid running curl-config unnecessarily","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-04-15T21:57:31Z","receivedAt":"2020-04-15T21:57:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 15, 2020 at 02:47:47PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > Junio, you may want to hold off on moving jk/build-with-right-curl to\n> > next until we resolve this one way or the other.\n> >\n> > -- >8 --\n> > Subject: [PATCH] Makefile: avoid running curl-config unnecessarily\n> \n> I think this has been accepted favourably after all?  I am inclined\n> to mark these three as ready for 'next'.\n\nYeah, my \"until we resolve\" was basically \"do people find this Makefile\neval trick too gross\". And I think the answer is that it's fine.\n\n-Peff\n"}]}