Erik Faye-Lund <kusmabite@xxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html