* Possible bug in Makefile when executing curl-config
@ 2019-07-22 19:46 Raitanen, Adam
2019-07-22 21:12 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: Raitanen, Adam @ 2019-07-22 19:46 UTC (permalink / raw
To: git@vger.kernel.org
I believe there may be a bug in the Makefile introduced by the following commit:
https://github.com/git/git/commit/23c4bbe28e61974577164db09cbd1d1c7e568ca4
The commit was merged in 2.20.0:
* The way -lcurl library gets linked has been simplified by taking
advantage of the fact that we can just ask curl-config command how.
Unfortunately it assumes that curl-config is in the path which is not always the case. When using "--with-curl=/path/to/curl" in the configure command, the path to the actual curl-config executable is ignored and the build fails around here:
CC http-fetch.o
make: curl-config: Command not found
LINK git-http-fetch
http.o: In function `fill_active_slots':
/tmp/git-2.21.0/http.c:1385: undefined reference to `curl_easy_cleanup'
.
We were able to workaround this by forcing the correct path into the make env:
make CURL_LDFLAGS="$(/path/to/curl/curl-config --libs)".
I reproduced the problem in the latest version 2.22.0.
Thanks,
Adam
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Possible bug in Makefile when executing curl-config
2019-07-22 19:46 Possible bug in Makefile when executing curl-config Raitanen, Adam
@ 2019-07-22 21:12 ` Jeff King
2019-07-23 19:43 ` [**EXTERNAL**] " Raitanen, Adam
0 siblings, 1 reply; 4+ messages in thread
From: Jeff King @ 2019-07-22 21:12 UTC (permalink / raw
To: Raitanen, Adam; +Cc: James Knight, git@vger.kernel.org
[+cc the author of that patch]
On Mon, Jul 22, 2019 at 07:46:37PM +0000, Raitanen, Adam wrote:
> I believe there may be a bug in the Makefile introduced by the following commit:
>
> https://github.com/git/git/commit/23c4bbe28e61974577164db09cbd1d1c7e568ca4
>
> The commit was merged in 2.20.0:
>
> * The way -lcurl library gets linked has been simplified by taking
> advantage of the fact that we can just ask curl-config command how.
>
> Unfortunately it assumes that curl-config is in the path which is not
> always the case. When using "--with-curl=/path/to/curl" in the
> configure command, the path to the actual curl-config executable is
> ignored and the build fails around here:
>
> CC http-fetch.o
> make: curl-config: Command not found
> LINK git-http-fetch
> http.o: In function `fill_active_slots':
> /tmp/git-2.21.0/http.c:1385: undefined reference to `curl_easy_cleanup'
> .
>
> We were able to workaround this by forcing the correct path into the make env:
>
> make CURL_LDFLAGS="$(/path/to/curl/curl-config --libs)".
>
> I reproduced the problem in the latest version 2.22.0.
For the case without autoconf, I think using CURL_LDFLAGS is the
intended safety valve. Though perhaps we should be falling back more
gracefully to the old behavior, like:
diff --git a/Makefile b/Makefile
index 11ccea4071..27e546bbfc 100644
--- a/Makefile
+++ b/Makefile
@@ -1343,7 +1343,7 @@ else
ifdef CURL_LDFLAGS
CURL_LIBCURL += $(CURL_LDFLAGS)
else
- CURL_LIBCURL += $(shell $(CURL_CONFIG) --libs)
+ CURL_LIBCURL += $(shell $(CURL_CONFIG) --libs || echo -lcurl)
endif
REMOTE_CURL_PRIMARY = git-remote-http$X
which should work on most systems.
For your specific case, where you _do_ have curl-config but it's just
not in the PATH, then I think:
make CURL_CONFIG=/path/to/curl-config
would be a slightly cleaner solution.
But it sounds like you _did_ use the autoconf script, but it did not
correctly set CURL_CONFIG. Do you have a config.mak.autogen file after
running ./configure, and if so, does it have an entry for CURL_CONFIG?
I'm not too familiar with our configure.ac, but it looks like
--with-curl might just point some paths for header/include files, and
not actually update the curl-config path.
-Peff
^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [**EXTERNAL**] Re: Possible bug in Makefile when executing curl-config
2019-07-22 21:12 ` Jeff King
@ 2019-07-23 19:43 ` Raitanen, Adam
2019-07-23 20:16 ` Jeff King
0 siblings, 1 reply; 4+ messages in thread
From: Raitanen, Adam @ 2019-07-23 19:43 UTC (permalink / raw
To: Jeff King; +Cc: James Knight, git@vger.kernel.org
Yes there is a config.mak.autogen and it does not have an entry for CURL_CONFIG, although it has a correct entry for CURLDIR.
The config.log also shows it checking for curl-config and not finding it then proceeding anyway:
configure:5917: checking for curl-config
configure:5945: result: no
...
-----Original Message-----
From: Jeff King <peff@peff.net>
Sent: Monday, July 22, 2019 5:12 PM
To: Raitanen, Adam <araitane@ciena.com>
Cc: James Knight <james.d.knight@live.com>; git@vger.kernel.org
Subject: [**EXTERNAL**] Re: Possible bug in Makefile when executing curl-config
[+cc the author of that patch]
On Mon, Jul 22, 2019 at 07:46:37PM +0000, Raitanen, Adam wrote:
> I believe there may be a bug in the Makefile introduced by the following commit:
>
> https://github.com/git/git/commit/23c4bbe28e61974577164db09cbd1d1c7e56
> 8ca4
>
> The commit was merged in 2.20.0:
>
> * The way -lcurl library gets linked has been simplified by taking
> advantage of the fact that we can just ask curl-config command how.
>
> Unfortunately it assumes that curl-config is in the path which is not
> always the case. When using "--with-curl=/path/to/curl" in the
> configure command, the path to the actual curl-config executable is
> ignored and the build fails around here:
>
> CC http-fetch.o
> make: curl-config: Command not found
> LINK git-http-fetch
> http.o: In function `fill_active_slots':
> /tmp/git-2.21.0/http.c:1385: undefined reference to `curl_easy_cleanup'
> .
>
> We were able to workaround this by forcing the correct path into the make env:
>
> make CURL_LDFLAGS="$(/path/to/curl/curl-config --libs)".
>
> I reproduced the problem in the latest version 2.22.0.
For the case without autoconf, I think using CURL_LDFLAGS is the intended safety valve. Though perhaps we should be falling back more gracefully to the old behavior, like:
diff --git a/Makefile b/Makefile
index 11ccea4071..27e546bbfc 100644
--- a/Makefile
+++ b/Makefile
@@ -1343,7 +1343,7 @@ else
ifdef CURL_LDFLAGS
CURL_LIBCURL += $(CURL_LDFLAGS)
else
- CURL_LIBCURL += $(shell $(CURL_CONFIG) --libs)
+ CURL_LIBCURL += $(shell $(CURL_CONFIG) --libs || echo -lcurl)
endif
REMOTE_CURL_PRIMARY = git-remote-http$X
which should work on most systems.
For your specific case, where you _do_ have curl-config but it's just not in the PATH, then I think:
make CURL_CONFIG=/path/to/curl-config
would be a slightly cleaner solution.
But it sounds like you _did_ use the autoconf script, but it did not correctly set CURL_CONFIG. Do you have a config.mak.autogen file after running ./configure, and if so, does it have an entry for CURL_CONFIG?
I'm not too familiar with our configure.ac, but it looks like --with-curl might just point some paths for header/include files, and not actually update the curl-config path.
-Peff
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [**EXTERNAL**] Re: Possible bug in Makefile when executing curl-config
2019-07-23 19:43 ` [**EXTERNAL**] " Raitanen, Adam
@ 2019-07-23 20:16 ` Jeff King
0 siblings, 0 replies; 4+ messages in thread
From: Jeff King @ 2019-07-23 20:16 UTC (permalink / raw
To: Raitanen, Adam; +Cc: James Knight, git@vger.kernel.org
On Tue, Jul 23, 2019 at 07:43:01PM +0000, Raitanen, Adam wrote:
> Yes there is a config.mak.autogen and it does not have an entry for CURL_CONFIG, although it has a correct entry for CURLDIR.
>
> The config.log also shows it checking for curl-config and not finding it then proceeding anyway:
>
> configure:5917: checking for curl-config
> configure:5945: result: no
OK, that makes sense from what I see in configure.ac. The "--with-curl"
flag does not interact at all with the curl-config test, but only sets
CURLDIR.
It seems like we should at least look for $CURLDIR/bin/curl-config in
such a case. But I think the fallback patch I showed earlier would
probably work for you, too (since on your system "curl-config --libs" is
likely just going to show "-lcurl" anyway).
Let's see what James says.
-Peff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-07-23 20:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-22 19:46 Possible bug in Makefile when executing curl-config Raitanen, Adam
2019-07-22 21:12 ` Jeff King
2019-07-23 19:43 ` [**EXTERNAL**] " Raitanen, Adam
2019-07-23 20:16 ` Jeff King
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.