{"thread":{"id":"2346","subject":"CFLAGS usage","startedAt":"2005-11-04T15:21:55Z","lastAt":"2005-11-05T07:57:00Z","messageCount":6,"participants":["Morten Welinder","David Brown","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"11125","messageId":"118833cc0511040721w7f3990fbw631feaa6a4bda936@mail.gmail.com","threadId":"2346","inReplyTo":null,"subject":"CFLAGS usage","fromName":"Morten Welinder","fromEmail":"mwelinder@gmail.com","sentAt":"2005-11-04T15:21:55Z","receivedAt":"2005-11-04T15:21:55Z","isPatch":false,"sender":{"key":"mwelinder@gmail.com","avatar":null},"body":"Various stuff is being added to CFLAGS, but CFLAGS is not being used\nafter being composed in CFLAGS_ALL.\n\nMorten\n\n\n\ngrep CFLAGS Makefile\nCFLAGS = -g -O2 -Wall\nALL_CFLAGS = $(CFLAGS) $(PLATFORM_DEFINES) $(DEFINES)\n                CFLAGS += -I$(CURLDIR)/include\n                CFLAGS += -I$(OPENSSLDIR)/include\n                CFLAGS += -I$(ICONVDIR)/include\n        $(CC) -o $*.o -c $(ALL_CFLAGS) $<\n        $(CC) -o $*.o -c $(ALL_CFLAGS) $<\n        $(CC) $(ALL_CFLAGS) -o $@ $(filter %.o,$^) $(LIBS)\n        $(CC) $(ALL_CFLAGS) -o $@ $(filter %.o,$^) $(LIB_FILE) $(SIMPLE_LIB)\n        $(CC) -c $(ALL_CFLAGS) \\\n        $(CC) $(ALL_CFLAGS) -o $@ test-date.c date.o ctype.o\n        $(CC) $(ALL_CFLAGS) -o $@ $^\n        for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i; done\n"},{"id":"11135","messageId":"20051104165903.GA13125@old.davidb.org","threadId":"2346","inReplyTo":"118833cc0511040721w7f3990fbw631feaa6a4bda936@mail.gmail.com","subject":"Re: CFLAGS usage","fromName":"David Brown","fromEmail":"git@davidb.org","sentAt":"2005-11-04T16:59:03Z","receivedAt":"2005-11-04T16:59:03Z","isPatch":false,"sender":{"key":"git@davidb.org","avatar":"https://gravatar.com/avatar/94c86a2938470a74c2eac5e2b69afc0871f79a660295c02219597aba8cb101c1?d=mp&s=160"},"body":"On Fri, Nov 04, 2005 at 10:21:55AM -0500, Morten Welinder wrote:\n\n> Various stuff is being added to CFLAGS, but CFLAGS is not being used\n> after being composed in CFLAGS_ALL.\n\n> CFLAGS = -g -O2 -Wall\n> ALL_CFLAGS = $(CFLAGS) $(PLATFORM_DEFINES) $(DEFINES)\n>                 CFLAGS += -I$(CURLDIR)/include\n\nMake expands variable lazily, so the ALL_CFLAGS expansion only happens at\nthe point of use, where all of the CFLAGS definitions have been made.\n\nDave\n"},{"id":"11139","messageId":"7voe50wblp.fsf@assigned-by-dhcp.cox.net","threadId":"2346","inReplyTo":"118833cc0511040721w7f3990fbw631feaa6a4bda936@mail.gmail.com","subject":"Re: CFLAGS usage","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-04T17:45:38Z","receivedAt":"2005-11-04T17:45:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Morten Welinder <mwelinder@gmail.com> writes:\n\n> Various stuff is being added to CFLAGS, but CFLAGS is not being used\n> after being composed in CFLAGS_ALL.\n\nTrue.\n\nWe should move ALL_CFLAGS definition at the very end; is\nanything else needed?\n"},{"id":"11140","messageId":"20051104182342.GA27711@old.davidb.org","threadId":"2346","inReplyTo":"7voe50wblp.fsf@assigned-by-dhcp.cox.net","subject":"Re: CFLAGS usage","fromName":"David Brown","fromEmail":"git@davidb.org","sentAt":"2005-11-04T18:23:44Z","receivedAt":"2005-11-04T18:23:44Z","isPatch":false,"sender":{"key":"git@davidb.org","avatar":"https://gravatar.com/avatar/94c86a2938470a74c2eac5e2b69afc0871f79a660295c02219597aba8cb101c1?d=mp&s=160"},"body":"On Fri, Nov 04, 2005 at 09:45:38AM -0800, Junio C Hamano wrote:\n> Morten Welinder <mwelinder@gmail.com> writes:\n> \n> > Various stuff is being added to CFLAGS, but CFLAGS is not being used\n> > after being composed in CFLAGS_ALL.\n> \n> True.\n> \n> We should move ALL_CFLAGS definition at the very end; is\n> anything else needed?\n\nAlthough it will work fine just as it is (it may be misleading, though).\nLazy evaluation in make is important, it allows such things as having\ndifferent CFLAGS that for different source files.\n\n  foo.o: CFLAGS += -fspecial-option\n\nAnd the additional CFLAGS will only apply to the compilation of foo.\nSince the ALL_CFLAGS is expanded lazily, it gets expanded for each target,\nand the rule above causes CFLAGS to have extra values only on that target.\n\nDave\n"},{"id":"11142","messageId":"7vek5ww8q5.fsf@assigned-by-dhcp.cox.net","threadId":"2346","inReplyTo":"20051104182342.GA27711@old.davidb.org","subject":"Re: CFLAGS usage","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-04T18:47:46Z","receivedAt":"2005-11-04T18:47:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Brown <git@davidb.org> writes:\n\n> And the additional CFLAGS will only apply to the compilation of foo.\n> Since the ALL_CFLAGS is expanded lazily, it gets expanded for each target,\n> and the rule above causes CFLAGS to have extra values only on that target.\n\nLazy expansion is true, but the reason we did ALL_CFLAGS was to\nmake CFLAGS overridable from the command line.\n\nI suspect that this would currently fail to add -I/some/where/include\non the compilation command line:\n\n\tmake CFLAGS='-Os -g' CURLDIR=/some/where\n\nwhile CURL_LIBCURL is set correctly on the linkage command line.\n"},{"id":"11166","messageId":"7vvez7r0hf.fsf_-_@assigned-by-dhcp.cox.net","threadId":"2346","inReplyTo":"7vek5ww8q5.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Simplify CFLAGS/DEFINES in Makefile","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-11-05T07:57:00Z","receivedAt":"2005-11-05T07:57:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> David Brown <git@davidb.org> writes:\n>\n>> And the additional CFLAGS will only apply to the compilation of foo.\n>> Since the ALL_CFLAGS is expanded lazily, it gets expanded for each target,\n>> and the rule above causes CFLAGS to have extra values only on that target.\n>\n> Lazy expansion is true, but the reason we did ALL_CFLAGS was to\n> make CFLAGS overridable from the command line.\n>\n> I suspect that this would currently fail to add -I/some/where/include\n> on the compilation command line:\n>\n> \tmake CFLAGS='-Os -g' CURLDIR=/some/where\n>\n> while CURL_LIBCURL is set correctly on the linkage command line.\n\nSo here is a proposed fix.  Likes, dislikes, \"you are a idiot\nand you do not understand make\"s?\n\n-jc\n\n-- >8 -- cut here -- >8 --\nI think the original intention was to make CFLAGS overridable\nfrom the make command line, but somehow we ended up accumulating\nconditional makefile sections that wrongly appends values to\nCFLAGs.  These assignments do not work when the user actually\noverride them from the make command line!\n\nDEFINES are handled the same way; it was seemingly overridable,\nbut the makefile sections had assignments, which meant\noverriding it from the command line broke things.\n\nThis simplifies things by limiting the internal futzing to\nALL_CFLAGS, and by removing DEFINES altogether.  Overriding\nCFLAGS from the command line should start working with this\nchange.\n\nSigned-off-by: Junio C Hamano <junkio@cox.net>\n\n---\n\n Makefile |   32 ++++++++++++++------------------\n 1 files changed, 14 insertions(+), 18 deletions(-)\n\napplies-to: ef3876e50bb5e91a6fb40d4e59df2178c6f40237\n3502cee71d7c0d1a0c9d792f9d185886e91d5d26\ndiff --git a/Makefile b/Makefile\nindex 6c01dc2..6064672 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -37,25 +37,21 @@\n # 1461501637330902918203684832716283019655932542976 hashes do not give you\n # sufficient guarantee that no collisions between objects will ever happen.\n \n-# DEFINES += -DCOLLISION_CHECK\n-\n # Define USE_NSEC below if you want git to care about sub-second file mtimes\n # and ctimes. Note that you need recent glibc (at least 2.2.4) for this, and\n # it will BREAK YOUR LOCAL DIFFS! show-diff and anything using it will likely\n # randomly break unless your underlying filesystem supports those sub-second\n # times (my ext3 doesn't).\n \n-# DEFINES += -DUSE_NSEC\n-\n # Define USE_STDEV below if you want git to care about the underlying device\n # change being considered an inode change from the update-cache perspective.\n \n-# DEFINES += -DUSE_STDEV\n-\n GIT_VERSION = 0.99.9.GIT\n \n+# CFLAGS is for the users to override from the command line.\n+\n CFLAGS = -g -O2 -Wall\n-ALL_CFLAGS = $(CFLAGS) $(PLATFORM_DEFINES) $(DEFINES)\n+ALL_CFLAGS = $(CFLAGS)\n \n prefix = $(HOME)\n bindir = $(prefix)/bin\n@@ -194,19 +190,19 @@ ifeq ($(uname_S),SunOS)\n \tNO_STRCASESTR = YesPlease\n \tINSTALL = ginstall\n \tTAR = gtar\n-\tPLATFORM_DEFINES += -D__EXTENSIONS__\n+\tALL_CFLAGS += -D__EXTENSIONS__\n endif\n ifeq ($(uname_O),Cygwin)\n \tNO_STRCASESTR = YesPlease\n \tNEEDS_LIBICONV = YesPlease\n \tNO_IPV6 = YesPlease\n \tX = .exe\n-\tPLATFORM_DEFINES += -DUSE_SYMLINK_HEAD=0\n+\tALL_CFLAGS += -DUSE_SYMLINK_HEAD=0\n endif\n ifeq ($(uname_S),OpenBSD)\n \tNO_STRCASESTR = YesPlease\n \tNEEDS_LIBICONV = YesPlease\n-\tPLATFORM_DEFINES += -I/usr/local/include -L/usr/local/lib\n+\tALL_CFLAGS += -I/usr/local/include -L/usr/local/lib\n endif\n ifneq (,$(findstring arm,$(uname_M)))\n \tARM_SHA1 = YesPlease\n@@ -217,7 +213,7 @@ endif\n ifndef NO_CURL\n \tifdef CURLDIR\n \t\t# This is still problematic -- gcc does not want -R.\n-\t\tCFLAGS += -I$(CURLDIR)/include\n+\t\tALL_CFLAGS += -I$(CURLDIR)/include\n \t\tCURL_LIBCURL = -L$(CURLDIR)/lib -R$(CURLDIR)/lib -lcurl\n \telse\n \t\tCURL_LIBCURL = -lcurl\n@@ -240,13 +236,13 @@ ifndef NO_OPENSSL\n \tOPENSSL_LIBSSL = -lssl\n \tifdef OPENSSLDIR\n \t\t# Again this may be problematic -- gcc does not always want -R.\n-\t\tCFLAGS += -I$(OPENSSLDIR)/include\n+\t\tALL_CFLAGS += -I$(OPENSSLDIR)/include\n \t\tOPENSSL_LINK = -L$(OPENSSLDIR)/lib -R$(OPENSSLDIR)/lib\n \telse\n \t\tOPENSSL_LINK =\n \tendif\n else\n-\tDEFINES += -DNO_OPENSSL\n+\tALL_CFLAGS += -DNO_OPENSSL\n \tMOZILLA_SHA1 = 1\n \tOPENSSL_LIBSSL =\n endif\n@@ -258,7 +254,7 @@ endif\n ifdef NEEDS_LIBICONV\n \tifdef ICONVDIR\n \t\t# Again this may be problematic -- gcc does not always want -R.\n-\t\tCFLAGS += -I$(ICONVDIR)/include\n+\t\tALL_CFLAGS += -I$(ICONVDIR)/include\n \t\tICONV_LINK = -L$(ICONVDIR)/lib -R$(ICONVDIR)/lib\n \telse\n \t\tICONV_LINK =\n@@ -276,15 +272,15 @@ ifdef NEEDS_NSL\n \tSIMPLE_LIB += -lnsl\n endif\n ifdef NO_STRCASESTR\n-\tDEFINES += -Dstrcasestr=gitstrcasestr -DNO_STRCASESTR=1\n+\tALL_CFLAGS += -Dstrcasestr=gitstrcasestr -DNO_STRCASESTR=1\n \tLIB_OBJS += compat/strcasestr.o\n endif\n ifdef NO_MMAP\n-\tDEFINES += -Dmmap=gitfakemmap -Dmunmap=gitfakemunmap -DNO_MMAP\n+\tALL_CFLAGS += -Dmmap=gitfakemmap -Dmunmap=gitfakemunmap -DNO_MMAP\n \tLIB_OBJS += compat/mmap.o\n endif\n ifdef NO_IPV6\n-\tDEFINES += -DNO_IPV6 -Dsockaddr_storage=sockaddr_in\n+\tALL_CFLAGS += -DNO_IPV6 -Dsockaddr_storage=sockaddr_in\n endif\n \n ifdef PPC_SHA1\n@@ -305,7 +301,7 @@ endif\n endif\n endif\n \n-DEFINES += -DSHA1_HEADER=$(call shellquote,$(SHA1_HEADER))\n+ALL_CFLAGS += -DSHA1_HEADER=$(call shellquote,$(SHA1_HEADER))\n \n SCRIPTS = $(patsubst %.sh,%,$(SCRIPT_SH)) \\\n \t  $(patsubst %.perl,%,$(SCRIPT_PERL)) \\\n---\n0.99.9.GIT\n"}]}