Re: [PATCH v2] Makefile: default to -lcurl when no CURL_CONFIG or CURLDIR

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 30, 2014, 15:13 UTC
Message-ID: <xmqqk3a6hmv6.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CABPQNSYDD7g3nOwb2ZaOQ9M9gQnjzQyKP4Zo-i8p4o-s30bk1Q@mail.gmail.com>

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

Show 27 quoted lines

> 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.