Re: contrib/credential/osxkeychain: Makefile should allow to set CFLAGS

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: contrib/credential/osxkeychain: Makefile should allow to set CFLAGS

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:53:54

Jeff King [off-list ref] writes:
On Tue, May 22, 2012 at 02:00:05PM -0700, Junio C Hamano wrote:
quoted
It however seems to me that git-credential-osxkeychain.o does honor
$(CFLAGS), either from the user "make CFLAGS=..." or the default the
Makefile in question supplies.

The line you quoted is not using $(CC) as the compiler, but is using it to
link the final build product.  It may not hurt to have $(CFLAGS) on that
line, but shouldn't the line also have $(LDFLAGS) on it?
I think so. Why don't we do this?
I am OK with that.

CFLAGS/LDFLAGS that come from ../../../config.mak _might_ not be suitable
for this binary, but I do not think of a concrete example how that could
be.  Besides, this is just an example in contrib/ so I'd not worry about
it too much.

Thanks.
quoted hunk
-- >8 --
Subject: osxkeychain: pull make config from top-level directory

The default compiler and cflags were mostly "works for me"
when I built the original version. We need to be much less
careful here than usual, because we know we are building
only on OS X.  But it's only polite to at least respect the
CFLAGS and CC definitions that the user may have provided
earlier.

While we're at it, let's update our definitions and rules to
be more like the top-level Makefile; default our CFLAGS to
include -O2, and make sure we use CFLAGS and LDFLAGS when
linking.

Signed-off-by: Jeff King <redacted>
---
 contrib/credential/osxkeychain/Makefile | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/contrib/credential/osxkeychain/Makefile b/contrib/credential/osxkeychain/Makefile
index 75c07f8..4b3a08a 100644
--- a/contrib/credential/osxkeychain/Makefile
+++ b/contrib/credential/osxkeychain/Makefile
@@ -2,10 +2,13 @@ all:: git-credential-osxkeychain
 
 CC = gcc
 RM = rm -f
-CFLAGS = -g -Wall
+CFLAGS = -g -O2 -Wall
+
+-include ../../../config.mak.autogen
+-include ../../../config.mak
 
 git-credential-osxkeychain: git-credential-osxkeychain.o
-	$(CC) -o $@ $< -Wl,-framework -Wl,Security
+	$(CC) $(CFLAGS) -o $@ $< $(LDFLAGS) -Wl,-framework -Wl,Security
 
 git-credential-osxkeychain.o: git-credential-osxkeychain.c
 	$(CC) -c $(CFLAGS) $<

Re: contrib/credential/osxkeychain: Makefile should allow to set CFLAGS

From: Jeff King <hidden>
Date: 2016-06-15 22:53:54

On Thu, May 24, 2012 at 10:35:35AM -0700, Junio C Hamano wrote:
quoted
I think so. Why don't we do this?
I am OK with that.

CFLAGS/LDFLAGS that come from ../../../config.mak _might_ not be suitable
for this binary, but I do not think of a concrete example how that could
be.  Besides, this is just an example in contrib/ so I'd not worry about
it too much.
Yeah, I considered that, but I figured that what you have in config.mak
is _at least_ as sensible as the random defaults I stuck in the
Makefile.  And overriding with "make CFLAGS=..." is still available.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help