From: Junio C Hamano <gitster@pobox.com>
To: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] makefile: hide stderr of curl-config test
Date: Mon, 26 Nov 2012 10:30:17 -0800 [thread overview]
Message-ID: <7vsj7wrzd2.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <1353554397-27162-1-git-send-email-paul.gortmaker@windriver.com> (Paul Gortmaker's message of "Wed, 21 Nov 2012 22:19:57 -0500")
Paul Gortmaker <paul.gortmaker@windriver.com> writes:
> Currently, if you don't have curl installed, you will get
>
> $ make distclean 2>&1 | grep curl
> /bin/sh: curl-config: not found
> /bin/sh: curl-config: not found
> /bin/sh: curl-config: not found
> /bin/sh: curl-config: not found
> /bin/sh: curl-config: not found
> $
>
> The intent is not to alarm the user, but just to test if there is
> a new enough curl installed. However, if you look at search engine
> suggested completions, the above "error" messages are confusing
> people into thinking curl is a hard requirement.
Good observation and identification of an issue to tackle. But why
isn't the patch like this?
PROGRAMS += $(REMOTE_CURL_NAMES)
- curl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)
+ curl_check := $(shell (echo 070908; curl-config --vernum) 2>/dev/null | sort -r | sed -ne 2p)
ifeq "$(curl_check)" "070908"
Removal of the "reject old libcURL" is logically a separate thing
regardless of the "alarming output from make", and it probably is
better done as a separate step in a two-patch series. Doing things
that way, when somebody objects to this:
> It wants to ensure curl is newer than 070908. The oldest
> machine I could find (RHEL 4.6) is 2007 vintage according
> to /proc/version data, and it has curl 070C01.
saying that their installation still cares about older libcURL, we
can still keep the "remove alarming output from make" bit.
>
> The failure here is to mask stderr in the test. However, since
> the chance of curl being installed, but too old is essentially
> nil, lets just check for existence and drop the ancient version
> threshold check, if for no other reason, than to simplifly the
> parsing of what the makefile is trying to do by humans.
>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>
> diff --git a/Makefile b/Makefile
> index 9bc5e40..56f55f6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1573,8 +1573,8 @@ else
> REMOTE_CURL_NAMES = $(REMOTE_CURL_PRIMARY) $(REMOTE_CURL_ALIASES)
> PROGRAM_OBJS += http-fetch.o
> PROGRAMS += $(REMOTE_CURL_NAMES)
> - curl_check := $(shell (echo 070908; curl-config --vernum) | sort -r | sed -ne 2p)
> - ifeq "$(curl_check)" "070908"
> + curl_check := $(shell curl-config --vernum 2>/dev/null)
> + ifneq "$(curl_check)" ""
> ifndef NO_EXPAT
> PROGRAM_OBJS += http-push.o
> endif
prev parent reply other threads:[~2012-11-26 18:30 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-22 3:19 [PATCH] makefile: hide stderr of curl-config test Paul Gortmaker
2012-11-26 18:30 ` Junio C Hamano [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vsj7wrzd2.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=paul.gortmaker@windriver.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.