RE: [**EXTERNAL**] Re: Possible bug in Makefile when executing curl-config
- From
- Raitanen, Adam <araitane@ciena.com>
- Date
- Jul 23, 2019, 19:43 UTC
- Message-ID
- <DM6PR04MB479387667E7EDEF2A27BFCA2D0C70@DM6PR04MB4793.namprd04.prod.outlook.com>
- In-Reply-To
- <20190722211219.GB31664@sigill.intra.peff.net>
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:
Show 27 quoted lines
> 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