public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: kusmabite@gmail•com
Cc: Dave Borowitz <dborowitz@google•com>,
	GIT Mailing-list <git@vger•kernel.org>
Subject: Re: [PATCH v2] Makefile: default to -lcurl when no CURL_CONFIG or CURLDIR
Date: Wed, 30 Apr 2014 08:13:01 -0700	[thread overview]
Message-ID: <xmqqk3a6hmv6.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CABPQNSYDD7g3nOwb2ZaOQ9M9gQnjzQyKP4Zo-i8p4o-s30bk1Q@mail.gmail.com> (Erik Faye-Lund's message of "Wed, 30 Apr 2014 15:04:51 +0200")

Erik Faye-Lund <kusmabite@gmail•com> writes:

> This is wrong, no? With CURL_CONFIG not set, it currently *does* run
> curl-config, see below.
> ...
>>         ifdef CURLDIR
>> +               CURL_LIBCURL =
>> +       else
>> +               CURL_CONFIG = curl-config
>> +               ifeq "$(CURL_CONFIG)" ""
>> +                       CURL_LIBCURL =
>> +               else
>> +                       CURL_LIBCURL := $(shell $(CURL_CONFIG) --libs)
>> +               endif
>
> Doesn't that definition just define CURL_CONFIG unconditionally? How
> are the first condition ever supposed to get triggered?
>
> $ make
> make: curl-config: Command not found
> GIT_VERSION = 1.9.2.462.gf3f11fa
> make: curl-config: Command not found
>     * new build flags
>     * new link flags
>     * new prefix flags
>     GEN common-cmds.h
> ...
>
> Yuck.

An earlier iteration of the patch used "CURL_CONFIG ?= curl-config",
but that would not have been much different:

        $ cat >Makefile <<\EOF
        CURL_CONFIG ?= curl-config
        ifeq "$(CURL_CONFIG)" ""
                X=Empty
        else
                X=NotEmpty
        endif
        ifdef CURL_CONFIG
                Z=Defined
        else
                Z=Undefined
        endif
        all::
                @echo "$(X) $(Z) CURL_CONFIG=<$(CURL_CONFIG)>"
        EOF
        $ make
        NotEmpty Defined CURL_CONFIG=<curl-config>
        $ make CURL_CONFIG=""
        Empty Undefined CURL_CONFIG=<>
        $ CURL_CONFIG="" make
        Empty Undefined CURL_CONFIG=<>

As the first one (the default) will still use curl-config and
passing an explicit CURL_CONFIG="" on the command line would be the
only way to squelch this unpleasantness.  If you change

	CURL_CONFIG ?= curl-config

to

	CURL_CONFIG = curl-config

in the above illustration, the first two would be the same result as
above, and the last one will behave the same as the first one---an
environment set to empty is still protected from the default defined
in the Makefile.

I think something along the lines of 

	ifdef CURLDIR
        	CURL_LIBCURL =
	else
		CURL_CONFIG = curl-config
		CURL_LIBCURL := $(shell sh -c '$(CURL_CONFIG) --libs' 2>/dev/null)
	fi

may be the right way to write this?

Note that $(shell $(CURL_CONFIG) --libs) when CURL_CONFIG is empty
would barf when $(CURL_CONFIG) expands to an empty string.

  reply	other threads:[~2014-04-30 15:13 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-28 21:01 [PATCH v2] Makefile: default to -lcurl when no CURL_CONFIG or CURLDIR Dave Borowitz
2014-04-28 21:32 ` Junio C Hamano
2014-04-28 23:30 ` Jonathan Nieder
2014-04-30 13:04 ` Erik Faye-Lund
2014-04-30 15:13   ` Junio C Hamano [this message]
2014-04-30 16:05     ` Erik Faye-Lund
2014-04-30 17:26       ` Junio C Hamano

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=xmqqk3a6hmv6.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=dborowitz@google$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=kusmabite@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox