public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Dave Borowitz <dborowitz@google•com>
Cc: git@vger•kernel.org, kusmabite@gmail•com
Subject: Re: [PATCH v3 1/2] Makefile: use curl-config to determine curl flags
Date: Mon, 28 Apr 2014 12:56:54 -0700	[thread overview]
Message-ID: <xmqqtx9dp6rd.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1398713704-15428-1-git-send-email-dborowitz@google.com> (Dave Borowitz's message of "Mon, 28 Apr 2014 12:35:03 -0700")

Dave Borowitz <dborowitz@google•com> writes:

> Use this only when CURLDIR is not explicitly specified, to continue
> supporting older builds. Moreover, if CURL_CONFIG is unset or running
> it returns no results (e.g. because it is missing), default to the old
> behavior of blindly setting -lcurl.
>  	ifdef CURLDIR
> +		CURL_LIBCURL=
>  	else
> +		CURL_CONFIG ?= curl-config
> +		ifeq "$(CURL_CONFIG)" ""
> +			CURL_LIBCURL =
> +		else
> +			CURL_LIBCURL := $(shell $(CURL_CONFIG) --libs)
>  		endif

This "ifeq" is redundant and will never set CURL_LIBCURL to empty
without running the "else" part, I think.  In a Makefile, a variable
explicitly set to empty and a variable that is unset are treated the
same.

	$ 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)"
	EOF
	$ make -f Makefile CURL_CONFIG=""
	Empty Undefined

That does not mean the patch will give us a broken behaviour,
though.  It just means the ifeq/else part will be redundant.

>  	endif
> +
> +	ifeq "$(CURL_LIBCURL)" ""

This will catch the "$(shell $(CURL_CONFIG) --libs) assigned an
empty string to CURL_LIBCURL" case, so the result is good.

I haven't checked what it would look like if we turn this into an
incremental patch to be applied on top of 'master' (which would give
us a place to document better why we do not rely on the presense of
curl-config), but if we can do so, that would be more preferable
than having to revert the merge of the previous one and then
applying these two patches anew.

Thanks.

> +		ifdef CURLDIR
> +			# Try "-Wl,-rpath=$(CURLDIR)/$(lib)" in such a case.
> +			BASIC_CFLAGS += -I$(CURLDIR)/include
> +			CURL_LIBCURL = -L$(CURLDIR)/$(lib) $(CC_LD_DYNPATH)$(CURLDIR)/$(lib) -lcurl
> +		else
> +			CURL_LIBCURL = -lcurl
> +		endif
> +		ifdef NEEDS_SSL_WITH_CURL
> +			CURL_LIBCURL += -lssl
> +			ifdef NEEDS_CRYPTO_WITH_SSL
> +				CURL_LIBCURL += -lcrypto
> +			endif
> +		endif
> +		ifdef NEEDS_IDN_WITH_CURL
> +			CURL_LIBCURL += -lidn
> +		endif
> +	else
> +		BASIC_CFLAGS += $(shell $(CURL_CONFIG) --cflags)
>  	endif
>  
>  	REMOTE_CURL_PRIMARY = git-remote-http$X

next prev parent reply	other threads:[~2014-04-28 19:57 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-28 19:35 [PATCH v3 1/2] Makefile: use curl-config to determine curl flags Dave Borowitz
2014-04-28 19:35 ` [PATCH v3 2/2] Makefile: allow static linking against libcurl Dave Borowitz
2014-04-28 19:44 ` [PATCH v3 1/2] Makefile: use curl-config to determine curl flags Jonathan Nieder
2014-04-28 19:51   ` Dave Borowitz
2014-04-28 19:56 ` Junio C Hamano [this message]
2014-04-28 20:00   ` Junio C Hamano
2014-04-28 21:13   ` 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=xmqqtx9dp6rd.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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox