{"thread":{"id":"32170","subject":"[PATCH] makefile: hide stderr of curl-config test","startedAt":"2012-11-22T03:19:57Z","lastAt":"2012-11-26T18:30:17Z","messageCount":2,"participants":["Paul Gortmaker","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"203664","messageId":"1353554397-27162-1-git-send-email-paul.gortmaker@windriver.com","threadId":"32170","inReplyTo":null,"subject":"[PATCH] makefile: hide stderr of curl-config test","fromName":"Paul Gortmaker","fromEmail":"paul.gortmaker@windriver.com","sentAt":"2012-11-22T03:19:57Z","receivedAt":"2012-11-22T03:19:57Z","isPatch":true,"sender":{"key":"paul.gortmaker@windriver.com","avatar":null},"body":"Currently, if you don't have curl installed, you will get\n\n    $ make distclean 2>&1 | grep curl\n    /bin/sh: curl-config: not found\n    /bin/sh: curl-config: not found\n    /bin/sh: curl-config: not found\n    /bin/sh: curl-config: not found\n    /bin/sh: curl-config: not found\n    $\n\nThe intent is not to alarm the user, but just to test if there is\na new enough curl installed.  However, if you look at search engine\nsuggested completions, the above \"error\" messages are confusing\npeople into thinking curl is a hard requirement.\n\nThis test dates back 7+ years to:\n\n ---------------------\n  commit 0890098780f295f2a58658d1f6b6627e40426c72\n  Author: Nick Hengeveld <nickh@reactrix.com>\n  Date:   Fri Nov 18 17:08:36 2005 -0800\n\n    Decide whether to build http-push in the Makefile\n ---------------------\n\nIt wants to ensure curl is newer than 070908.  The oldest\nmachine I could find (RHEL 4.6) is 2007 vintage according\nto /proc/version data, and it has curl 070C01.\n\nThe failure here is to mask stderr in the test.  However, since\nthe chance of curl being installed, but too old is essentially\nnil, lets just check for existence and drop the ancient version\nthreshold check, if for no other reason, than to simplifly the\nparsing of what the makefile is trying to do by humans.\n\nSigned-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>\n\ndiff --git a/Makefile b/Makefile\nindex 9bc5e40..56f55f6 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1573,8 +1573,8 @@ else\n \tREMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES)\n \tPROGRAM_OBJS += http-fetch.o\n \tPROGRAMS += $(REMOTE_CURL_NAMES)\n-\tcurl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)\n-\tifeq \"$(curl_check)\" \"070908\"\n+\tcurl_check := $(shell curl-config --vernum 2>/dev/null)\n+\tifneq \"$(curl_check)\" \"\"\n \t\tifndef NO_EXPAT\n \t\t\tPROGRAM_OBJS += http-push.o\n \t\tendif\n-- \n1.8.0\n"},{"id":"203902","messageId":"7vsj7wrzd2.fsf@alter.siamese.dyndns.org","threadId":"32170","inReplyTo":"1353554397-27162-1-git-send-email-paul.gortmaker@windriver.com","subject":"Re: [PATCH] makefile: hide stderr of curl-config test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-26T18:30:17Z","receivedAt":"2012-11-26T18:30:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Paul Gortmaker <paul.gortmaker@windriver.com> writes:\n\n> Currently, if you don't have curl installed, you will get\n>\n>     $ make distclean 2>&1 | grep curl\n>     /bin/sh: curl-config: not found\n>     /bin/sh: curl-config: not found\n>     /bin/sh: curl-config: not found\n>     /bin/sh: curl-config: not found\n>     /bin/sh: curl-config: not found\n>     $\n>\n> The intent is not to alarm the user, but just to test if there is\n> a new enough curl installed.  However, if you look at search engine\n> suggested completions, the above \"error\" messages are confusing\n> people into thinking curl is a hard requirement.\n\nGood observation and identification of an issue to tackle.  But why\nisn't the patch like this?\n\n \tPROGRAMS += $(REMOTE_CURL_NAMES)\n-\tcurl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)\n+\tcurl_check := $(shell (echo 070908; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)\n \tifeq \"$(curl_check)\" \"070908\"\n\nRemoval of the \"reject old libcURL\" is logically a separate thing\nregardless of the \"alarming output from make\", and it probably is\nbetter done as a separate step in a two-patch series.  Doing things\nthat way, when somebody objects to this:\n\n> It wants to ensure curl is newer than 070908.  The oldest\n> machine I could find (RHEL 4.6) is 2007 vintage according\n> to /proc/version data, and it has curl 070C01.\n\nsaying that their installation still cares about older libcURL, we\ncan still keep the \"remove alarming output from make\" bit.\n\n>\n> The failure here is to mask stderr in the test.  However, since\n> the chance of curl being installed, but too old is essentially\n> nil, lets just check for existence and drop the ancient version\n> threshold check, if for no other reason, than to simplifly the\n> parsing of what the makefile is trying to do by humans.\n>\n> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>\n>\n> diff --git a/Makefile b/Makefile\n> index 9bc5e40..56f55f6 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1573,8 +1573,8 @@ else\n>  \tREMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES)\n>  \tPROGRAM_OBJS += http-fetch.o\n>  \tPROGRAMS += $(REMOTE_CURL_NAMES)\n> -\tcurl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)\n> -\tifeq \"$(curl_check)\" \"070908\"\n> +\tcurl_check := $(shell curl-config --vernum 2>/dev/null)\n> +\tifneq \"$(curl_check)\" \"\"\n>  \t\tifndef NO_EXPAT\n>  \t\t\tPROGRAM_OBJS += http-push.o\n>  \t\tendif\n"}]}