diff mbox series

[1/2] Makefile: not use mismatched curl_config to check version

Message ID 20230201113133.10195-1-worldhello.net@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/2] Makefile: not use mismatched curl_config to check version | expand

Commit Message

Jiang Xin Feb. 1, 2023, 11:31 a.m. UTC 
From: Jiang Xin <zhiyou.jx@alibaba-inc.com>

We may install different versions of curl, E.g.:

 * A system default curl, which version is below 7.34.0, is installed
   in "/usr", and the "curl_config" program is located in "/usr/bin/".

 * A higher version of curl is installed in "/opt/git/embedded/", and
   the "curl_config" program is located in "/opt/git/embedded/bin/".

If we add the path "/opt/git/embedded/bin" in search PATH, and install
git using command "make && sudo make install", the source code may be
compiled twice.

This is because when we run "make" using normal user account, make will
call "/opt/git/embedded/bin/curl_config" to check curl version, and will
set variable USE_CURL_FOR_IMAP_SEND. But when we call "make install"
using root user's account, we call the system default version of
curl_config to check curl version, and will lead to a different
"GIT-CFLAGS" file, and will recompile all source code again.

Append "$(CURLDIR)/bin" before the "CURL_CONFIG" variable to use the
specific "curl_config" program we want to check curl version, we will
get the correct "CURL_CFLAGS" and "CURL_LDFLAGS" variables, and we can
also have a stable "GIT-CFLAGS" file to prevent recompile when running
"sudo make install".

Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

Comments

Junio C Hamano Feb. 1, 2023, 6:06 p.m. UTC | #1 
Jiang Xin <worldhello.net@gmail.com> writes:

> From: Jiang Xin <zhiyou.jx@alibaba-inc.com>
>
> We may install different versions of curl, E.g.:
>
>  * A system default curl, which version is below 7.34.0, is installed
>    in "/usr", and the "curl_config" program is located in "/usr/bin/".
>
>  * A higher version of curl is installed in "/opt/git/embedded/", and
>    the "curl_config" program is located in "/opt/git/embedded/bin/".
> ...
> diff --git a/Makefile b/Makefile
> index 45bd6ac9c3..f4eaf22523 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1597,6 +1597,7 @@ else
>  		# Try "-Wl,-rpath=$(CURLDIR)/$(lib)" in such a case.
>  		CURL_CFLAGS = -I$(CURLDIR)/include
>  		CURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib)
> +		CURL_CONFIG := $(CURLDIR)/bin/$(CURL_CONFIG)
>  	else
>  		CURL_CFLAGS =
>  		CURL_LIBCURL =

The above is inside "ifdef CURLDIR/else/endif".  Is the assumption
here that any and all installation of cURL that needs CURLDIR
specified should have CURL_CONFIG binary under $(CURLDIR)/bin?

What does this patch do to folks who know the exact location of the
curl-config binary and have been using CURL_CONFIG from the command
line or in config.mak to point at it?  Doesn't the above break their
working set-up?

For that matter, if you do want to use a specific curl-config binary,
can't you use the existing mechanism to set CURL_CONFIG to the path,
which was invented for this exact purpose?

I am not opposed to make it more convenient but I am worried about
breaking people's working set-up with this change.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 45bd6ac9c3..f4eaf22523 100644
--- a/Makefile
+++ b/Makefile
@@ -1597,6 +1597,7 @@  else
 		# Try "-Wl,-rpath=$(CURLDIR)/$(lib)" in such a case.
 		CURL_CFLAGS = -I$(CURLDIR)/include
 		CURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib)
+		CURL_CONFIG := $(CURLDIR)/bin/$(CURL_CONFIG)
 	else
 		CURL_CFLAGS =
 		CURL_LIBCURL =