{"thread":{"id":"49740","subject":"[PATCH] build: link with curl-defined linker flags","startedAt":"2018-11-02T07:21:55Z","lastAt":"2018-11-02T15:10:50Z","messageCount":2,"participants":["James Knight","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"362256","messageId":"SN4PR0701MB3679BA798ABD874D34A595DDA0CF0@SN4PR0701MB3679.namprd07.prod.outlook.com","threadId":"49740","inReplyTo":null,"subject":"[PATCH] build: link with curl-defined linker flags","fromName":"James Knight","fromEmail":"james.d.knight@live.com","sentAt":"2018-11-02T07:21:51Z","receivedAt":"2018-11-02T07:21:55Z","isPatch":true,"sender":{"key":"james.d.knight@live.com","avatar":null},"body":"Adjust the autotools configuration to populate libcurl-related linker\nflags from curl-config instead of manually populating flags based off\ndetected features.\n\nOriginally, the configuration would check for SSL-support in the target\ncurl library. If enabled, NEEDS_SSL_WITH_CURL would be set and used in\nthe Makefile to append additional libraries to link against. Since the\nprocess is already depending on a curl-config utility to provide\ncurl-related build information, adjusting the build to track the linker\nflags in CURL_LIBCURL and pass the configuration option into the\nMakefile.\n\nSigned-off-by: James Knight <james.d.knight@live.com>\n---\n Makefile         | 21 +++++++--------------\n config.mak.uname |  5 ++---\n configure.ac     | 17 +++++++----------\n 3 files changed, 16 insertions(+), 27 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex b08d5ea25..c3be87b0e 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -183,10 +183,6 @@ all::\n #\n # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n #\n-# Define NEEDS_SSL_WITH_CURL if you need -lssl with -lcurl (Minix).\n-#\n-# Define NEEDS_IDN_WITH_CURL if you need -lidn when using -lcurl (Minix).\n-#\n # Define NEEDS_LIBICONV if linking with libc is not enough (Darwin).\n #\n # Define NEEDS_LIBINTL_BEFORE_LIBICONV if you need libintl before libiconv.\n@@ -1305,18 +1301,15 @@ else\n \tifdef CURLDIR\n \t\t# Try \"-Wl,-rpath=$(CURLDIR)/$(lib)\" in such a case.\n \t\tBASIC_CFLAGS += -I$(CURLDIR)/include\n-\t\tCURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib) -lcurl\n+\t\tCURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib)\n \telse\n-\t\tCURL_LIBCURL = -lcurl\n-\tendif\n-\tifdef NEEDS_SSL_WITH_CURL\n-\t\tCURL_LIBCURL += -lssl\n-\t\tifdef NEEDS_CRYPTO_WITH_SSL\n-\t\t\tCURL_LIBCURL += -lcrypto\n-\t\tendif\n+\t\tCURL_LIBCURL =\n \tendif\n-\tifdef NEEDS_IDN_WITH_CURL\n-\t\tCURL_LIBCURL += -lidn\n+\n+\tifdef CURL_LDFLAGS\n+\t\tCURL_LIBCURL += $(CURL_LDFLAGS)\n+\telse\n+\t\tCURL_LIBCURL += -lcurl\n \tendif\n \n \tREMOTE_CURL_PRIMARY = git-remote-http$X\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 8acdeb71f..923b8fa09 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -431,8 +431,7 @@ ifeq ($(uname_S),Minix)\n \tNO_NSEC = YesPlease\n \tNEEDS_LIBGEN =\n \tNEEDS_CRYPTO_WITH_SSL = YesPlease\n-\tNEEDS_IDN_WITH_CURL = YesPlease\n-\tNEEDS_SSL_WITH_CURL = YesPlease\n+\tCURL_LDFLAGS = -lssl -lcrypto -lidn\n \tNEEDS_RESOLV =\n \tNO_HSTRERROR = YesPlease\n \tNO_MMAP = YesPlease\n@@ -458,7 +457,7 @@ ifeq ($(uname_S),NONSTOP_KERNEL)\n \t# Missdetected, hence commented out, see below.\n \t#NO_CURL = YesPlease\n \t# Added manually, see above.\n-\tNEEDS_SSL_WITH_CURL = YesPlease\n+\tCURL_LDFLAGS = -lssl -lcrypto\n \tHAVE_LIBCHARSET_H = YesPlease\n \tHAVE_STRINGS_H = YesPlease\n \tNEEDS_LIBICONV = YesPlease\ndiff --git a/configure.ac b/configure.ac\nindex e11b7976a..44e8c036b 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -600,17 +600,14 @@ AC_CHECK_PROG([CURL_CONFIG], [curl-config],\n \n if test $CURL_CONFIG != no; then\n     GIT_CONF_SUBST([CURL_CONFIG])\n-    if test -z \"${NO_OPENSSL}\"; then\n-      AC_MSG_CHECKING([if Curl supports SSL])\n-      if test $(curl-config --features|grep SSL) = SSL; then\n-         NEEDS_SSL_WITH_CURL=YesPlease\n-         AC_MSG_RESULT([yes])\n-      else\n-         NEEDS_SSL_WITH_CURL=\n-         AC_MSG_RESULT([no])\n-      fi\n-      GIT_CONF_SUBST([NEEDS_SSL_WITH_CURL])\n+\n+    if test -z \"$CURL_CONFIG_OPTS\"; then\n+        CURL_CONFIG_OPTS=\"--libs\"\n     fi\n+\n+    CURL_LDFLAGS=$($CURL_CONFIG $CURL_CONFIG_OPTS)\n+    AC_MSG_NOTICE([Setting CURL_LDFLAGS to '$CURL_LDFLAGS'])\n+    GIT_CONF_SUBST([CURL_LDFLAGS], [$CURL_LDFLAGS])\n fi\n \n fi\n-- \n2.19.1\n\n"},{"id":"362273","messageId":"xmqq7ehvplak.fsf@gitster-ct.c.googlers.com","threadId":"49740","inReplyTo":"SN4PR0701MB3679BA798ABD874D34A595DDA0CF0@SN4PR0701MB3679.namprd07.prod.outlook.com","subject":"Re: [PATCH] build: link with curl-defined linker flags","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-02T15:10:43Z","receivedAt":"2018-11-02T15:10:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"James Knight <james.d.knight@live.com> writes:\n\n>  Makefile         | 21 +++++++--------------\n>  config.mak.uname |  5 ++---\n>  configure.ac     | 17 +++++++----------\n>  3 files changed, 16 insertions(+), 27 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index b08d5ea25..c3be87b0e 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -183,10 +183,6 @@ all::\n>  #\n>  # Define NEEDS_SSL_WITH_CRYPTO if you need -lssl when using -lcrypto (Darwin).\n>  #\n> -# Define NEEDS_SSL_WITH_CURL if you need -lssl with -lcurl (Minix).\n> -#\n> -# Define NEEDS_IDN_WITH_CURL if you need -lidn when using -lcurl (Minix).\n> -#\n\nHmm, because the use of autoconf -> ./configure in our build\nprocedure is optional, wouldn't this change give regression to those\nof us who use these Makefile variables to configure their build,\ninstead of relying on autoconf?\n\n> diff --git a/config.mak.uname b/config.mak.uname\n> index 8acdeb71f..923b8fa09 100644\n> --- a/config.mak.uname\n> +++ b/config.mak.uname\n> @@ -431,8 +431,7 @@ ifeq ($(uname_S),Minix)\n>  \tNO_NSEC = YesPlease\n>  \tNEEDS_LIBGEN =\n>  \tNEEDS_CRYPTO_WITH_SSL = YesPlease\n> -\tNEEDS_IDN_WITH_CURL = YesPlease\n> -\tNEEDS_SSL_WITH_CURL = YesPlease\n> +\tCURL_LDFLAGS = -lssl -lcrypto -lidn\n\nOK, as long as we describe how to update their config.mak to adjust\nto the new world order, the \"regression\" I noticed earlier is not\ntoo bad, and the way the new CURL_LDFLAGS variable drives the build\nis much more direct and transparent than via the old way to specify\nNEEDS_*_WITH_CURL to affect the build indirectly.\n\nI think such \"describe how to configure in the new world order\"\nshould go to where NEEDS_*_WITH_CURL used to be, e.g. \"Define\nCURL_LDFLAGS to specify flags that you need to link using -lcurl;\nsee config.mak.uname and look for the variable for examples\"\nor something like that, perhaps.\n\nThanks.\n"}]}