{"thread":{"id":"30594","subject":"contrib/credential/osxkeychain: Makefile should allow to set CFLAGS","startedAt":"2012-05-22T20:25:51Z","lastAt":"2012-05-24T17:51:43Z","messageCount":5,"participants":["Samuel John","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"191939","messageId":"AB628C35-FEC2-4425-B3D8-EEC7F8D6A66B@samueljohn.de","threadId":"30594","inReplyTo":null,"subject":"contrib/credential/osxkeychain: Makefile should allow to set CFLAGS","fromName":"Samuel John","fromEmail":"mail@samueljohn.de","sentAt":"2012-05-22T20:25:51Z","receivedAt":"2012-05-22T20:25:51Z","isPatch":false,"sender":{"key":"mail@samueljohn.de","avatar":null},"body":"Hi there,\n\nthe contrib/credential/osxkeychain/Makefile goes like this\n\n> CC = gcc\n> RM = rm -f\n> CFLAGS = -g -Wall\n> \n> git-credential-osxkeychain: git-credential-osxkeychain.o\n> \t$(CC) -o $@ $< -Wl,-framework -Wl,Security\n> \n\n\nand does not allow me to pass CFLAGS to build git-credential-osxkeychain.o.\nMy hack is to call: make CC=\"$CC $CFLAGS\"\nThat is ugly.\n\nWould you mind to add the CFLAGS to that CC call?\n\nbests,\n Samuel\n"},{"id":"191950","messageId":"7vtxz82ap6.fsf@alter.siamese.dyndns.org","threadId":"30594","inReplyTo":"AB628C35-FEC2-4425-B3D8-EEC7F8D6A66B@samueljohn.de","subject":"Re: contrib/credential/osxkeychain: Makefile should allow to set CFLAGS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-22T21:00:05Z","receivedAt":"2012-05-22T21:00:05Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Samuel John <mail@samueljohn.de> writes:\n\n> Hi there,\n>\n> the contrib/credential/osxkeychain/Makefile goes like this\n>\n>> CC = gcc\n>> RM = rm -f\n>> CFLAGS = -g -Wall\n>> \n>> git-credential-osxkeychain: git-credential-osxkeychain.o\n>> \t$(CC) -o $@ $< -Wl,-framework -Wl,Security\n>\n> and does not allow me to pass CFLAGS to build git-credential-osxkeychain.o.\n> My hack is to call: make CC=\"$CC $CFLAGS\"\n> That is ugly.\n\nYeah, that is ugly.\n\nIt however seems to me that git-credential-osxkeychain.o does honor\n$(CFLAGS), either from the user \"make CFLAGS=...\" or the default the\nMakefile in question supplies.\n\nThe line you quoted is not using $(CC) as the compiler, but is using it to\nlink the final build product.  It may not hurt to have $(CFLAGS) on that\nline, but shouldn't the line also have $(LDFLAGS) on it?\n"},{"id":"192010","messageId":"20120523173653.GA29458@sigill.intra.peff.net","threadId":"30594","inReplyTo":"7vtxz82ap6.fsf@alter.siamese.dyndns.org","subject":"Re: contrib/credential/osxkeychain: Makefile should allow to set CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-23T17:36:53Z","receivedAt":"2012-05-23T17:36:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 22, 2012 at 02:00:05PM -0700, Junio C Hamano wrote:\n\n> It however seems to me that git-credential-osxkeychain.o does honor\n> $(CFLAGS), either from the user \"make CFLAGS=...\" or the default the\n> Makefile in question supplies.\n> \n> The line you quoted is not using $(CC) as the compiler, but is using it to\n> link the final build product.  It may not hurt to have $(CFLAGS) on that\n> line, but shouldn't the line also have $(LDFLAGS) on it?\n\nI think so. Why don't we do this?\n\n-- >8 --\nSubject: osxkeychain: pull make config from top-level directory\n\nThe default compiler and cflags were mostly \"works for me\"\nwhen I built the original version. We need to be much less\ncareful here than usual, because we know we are building\nonly on OS X.  But it's only polite to at least respect the\nCFLAGS and CC definitions that the user may have provided\nearlier.\n\nWhile we're at it, let's update our definitions and rules to\nbe more like the top-level Makefile; default our CFLAGS to\ninclude -O2, and make sure we use CFLAGS and LDFLAGS when\nlinking.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n contrib/credential/osxkeychain/Makefile | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/credential/osxkeychain/Makefile b/contrib/credential/osxkeychain/Makefile\nindex 75c07f8..4b3a08a 100644\n--- a/contrib/credential/osxkeychain/Makefile\n+++ b/contrib/credential/osxkeychain/Makefile\n@@ -2,10 +2,13 @@ all:: git-credential-osxkeychain\n \n CC = gcc\n RM = rm -f\n-CFLAGS = -g -Wall\n+CFLAGS = -g -O2 -Wall\n+\n+-include ../../../config.mak.autogen\n+-include ../../../config.mak\n \n git-credential-osxkeychain: git-credential-osxkeychain.o\n-\t$(CC) -o $@ $< -Wl,-framework -Wl,Security\n+\t$(CC) $(CFLAGS) -o $@ $< $(LDFLAGS) -Wl,-framework -Wl,Security\n \n git-credential-osxkeychain.o: git-credential-osxkeychain.c\n \t$(CC) -c $(CFLAGS) $<\n-- \n1.7.9.7.35.gbeaaf11\n"},{"id":"192089","messageId":"7vaa0xxz14.fsf@alter.siamese.dyndns.org","threadId":"30594","inReplyTo":"20120523173653.GA29458@sigill.intra.peff.net","subject":"Re: contrib/credential/osxkeychain: Makefile should allow to set CFLAGS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-24T17:35:35Z","receivedAt":"2012-05-24T17:35:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, May 22, 2012 at 02:00:05PM -0700, Junio C Hamano wrote:\n>\n>> It however seems to me that git-credential-osxkeychain.o does honor\n>> $(CFLAGS), either from the user \"make CFLAGS=...\" or the default the\n>> Makefile in question supplies.\n>> \n>> The line you quoted is not using $(CC) as the compiler, but is using it to\n>> link the final build product.  It may not hurt to have $(CFLAGS) on that\n>> line, but shouldn't the line also have $(LDFLAGS) on it?\n>\n> I think so. Why don't we do this?\n\nI am OK with that.\n\nCFLAGS/LDFLAGS that come from ../../../config.mak _might_ not be suitable\nfor this binary, but I do not think of a concrete example how that could\nbe.  Besides, this is just an example in contrib/ so I'd not worry about\nit too much.\n\nThanks.\n\n> -- >8 --\n> Subject: osxkeychain: pull make config from top-level directory\n>\n> The default compiler and cflags were mostly \"works for me\"\n> when I built the original version. We need to be much less\n> careful here than usual, because we know we are building\n> only on OS X.  But it's only polite to at least respect the\n> CFLAGS and CC definitions that the user may have provided\n> earlier.\n>\n> While we're at it, let's update our definitions and rules to\n> be more like the top-level Makefile; default our CFLAGS to\n> include -O2, and make sure we use CFLAGS and LDFLAGS when\n> linking.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n>  contrib/credential/osxkeychain/Makefile | 7 +++++--\n>  1 file changed, 5 insertions(+), 2 deletions(-)\n>\n> diff --git a/contrib/credential/osxkeychain/Makefile b/contrib/credential/osxkeychain/Makefile\n> index 75c07f8..4b3a08a 100644\n> --- a/contrib/credential/osxkeychain/Makefile\n> +++ b/contrib/credential/osxkeychain/Makefile\n> @@ -2,10 +2,13 @@ all:: git-credential-osxkeychain\n>  \n>  CC = gcc\n>  RM = rm -f\n> -CFLAGS = -g -Wall\n> +CFLAGS = -g -O2 -Wall\n> +\n> +-include ../../../config.mak.autogen\n> +-include ../../../config.mak\n>  \n>  git-credential-osxkeychain: git-credential-osxkeychain.o\n> -\t$(CC) -o $@ $< -Wl,-framework -Wl,Security\n> +\t$(CC) $(CFLAGS) -o $@ $< $(LDFLAGS) -Wl,-framework -Wl,Security\n>  \n>  git-credential-osxkeychain.o: git-credential-osxkeychain.c\n>  \t$(CC) -c $(CFLAGS) $<\n"},{"id":"192092","messageId":"20120524175143.GD3161@sigill.intra.peff.net","threadId":"30594","inReplyTo":"7vaa0xxz14.fsf@alter.siamese.dyndns.org","subject":"Re: contrib/credential/osxkeychain: Makefile should allow to set CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-24T17:51:43Z","receivedAt":"2012-05-24T17:51:43Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 24, 2012 at 10:35:35AM -0700, Junio C Hamano wrote:\n\n> > I think so. Why don't we do this?\n> \n> I am OK with that.\n> \n> CFLAGS/LDFLAGS that come from ../../../config.mak _might_ not be suitable\n> for this binary, but I do not think of a concrete example how that could\n> be.  Besides, this is just an example in contrib/ so I'd not worry about\n> it too much.\n\nYeah, I considered that, but I figured that what you have in config.mak\nis _at least_ as sensible as the random defaults I stuck in the\nMakefile.  And overriding with \"make CFLAGS=...\" is still available.\n\n-Peff\n"}]}