* 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
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).