From 94a88e2524a7243fe77d42faa5649e1c38d1b292 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 26 Mar 2020 04:06:45 -0400 Subject: [PATCH 1/3] Makefile: avoid running curl-config multiple times If the user hasn't set the CURL_LDFLAGS Makefile variable, we invoke curl-config like this: CURL_LIBCURL += $(shell $(CURL_CONFIG) --libs) Because the shell function is run when the value is expanded, we invoke curl-config each time we need to link something (which generally ends up being four times for a full build). Instead, let's use an immediate Makefile variable, which only needs expanding once. We can't combine that with the existing "+=", but since we only do this when CURL_LDFLAGS is undefined, we can just set that variable. That also allows us to simplify our conditional a bit, since both sides will then put the result into CURL_LIBCURL. While we're touching it, let's fix the indentation to match the nearby code (we're inside an outer conditional, so everything else is indented one level). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 9804a0758b..eaf56e23aa 100644 --- a/Makefile +++ b/Makefile @@ -1364,11 +1364,10 @@ else CURL_LIBCURL = endif -ifdef CURL_LDFLAGS + ifndef CURL_LDFLAGS + CURL_LDFLAGS := $(shell $(CURL_CONFIG) --libs) + endif CURL_LIBCURL += $(CURL_LDFLAGS) -else - CURL_LIBCURL += $(shell $(CURL_CONFIG) --libs) -endif REMOTE_CURL_PRIMARY = git-remote-http$X REMOTE_CURL_ALIASES = git-remote-https$X git-remote-ftp$X git-remote-ftps$X From 897d68e7af82824e0c9e15d3c4af1a8a71afc791 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 26 Mar 2020 04:08:55 -0400 Subject: [PATCH 2/3] Makefile: use curl-config --cflags We add the result of "curl-config --libs" when linking curl programs, but we never bother calling "curl-config --cflags". Presumably nobody noticed because: - a system libcurl installed into /usr/include/curl wouldn't need any flags ("/usr/include" is already in the search path, and the #include lines all look , etc). - using CURLDIR sets up both the includes and the library path However, if you prefer CURL_CONFIG to CURLDIR, something simple like: make CURL_CONFIG=/path/to/curl-config doesn't work. We'd link against the libcurl specified by that program, but not find its header files when compiling. Let's invoke "curl-config --cflags" similar to the way we do for "--libs". Note that we'll feed the result into BASIC_CFLAGS. The rest of the Makefile doesn't distinguish which files need curl support during compilation and which do not. That should be OK, though. At most this should be adding a "-I" directive, and this is how CURLDIR already behaves. And since we follow the immediate-variable pattern from CURL_LDFLAGS, we won't accidentally invoke curl-config once per compilation. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index eaf56e23aa..3d90488dab 100644 --- a/Makefile +++ b/Makefile @@ -1358,9 +1358,10 @@ ifdef NO_CURL else ifdef CURLDIR # Try "-Wl,-rpath=$(CURLDIR)/$(lib)" in such a case. - BASIC_CFLAGS += -I$(CURLDIR)/include + CURL_CFLAGS = -I$(CURLDIR)/include CURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib) else + CURL_CFLAGS = CURL_LIBCURL = endif @@ -1369,6 +1370,11 @@ else endif CURL_LIBCURL += $(CURL_LDFLAGS) + ifndef CURL_CFLAGS + CURL_CFLAGS := $(shell $(CURL_CONFIG) --cflags) + endif + BASIC_CFLAGS += $(CURL_CFLAGS) + REMOTE_CURL_PRIMARY = git-remote-http$X REMOTE_CURL_ALIASES = git-remote-https$X git-remote-ftp$X git-remote-ftps$X REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES) From 0573831950d02db0c2932708eadd9f765e5773a6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Sat, 4 Apr 2020 10:58:29 -0400 Subject: [PATCH 3/3] 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 Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 3d90488dab..596f0268ba 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)