{"thread":{"id":"50557","subject":"[PATCH] Makefile: remove an out-of-date comment","startedAt":"2019-02-22T10:57:10Z","lastAt":"2019-02-23T13:23:44Z","messageCount":20,"participants":["Ævar Arnfjörð Bjarmason","Jeff King","Eric Sunshine","Ramsay Jones","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"369902","messageId":"20190222105658.26831-1-avarab@gmail.com","threadId":"50557","inReplyTo":null,"subject":"[PATCH] Makefile: remove an out-of-date comment","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-02-22T10:56:58Z","receivedAt":"2019-02-22T10:57:10Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove a comment referring to a caveat that hasn't been applicable\nsince 18b0fc1ce1 (\"Git.pm: Kill Git.xs for now\", 2006-09-23).\n\nAt the time of 8d7f586f13 (\"Git.pm: Support for perl/ being built by a\ndifferent compiler\", 2006-06-25) some of the code in perl would be\nbuilt by a C compiler, but support for that went away a few months\nlater in 18b0fc1ce1 discussed above.\n\nSince my 20d2a30f8f (\"Makefile: replace perl/Makefile.PL with simple\nmake rules\", 2017-12-10) the perl/ directory doesn't even have its own\nbuild process.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex c5240942f2..97e922cc41 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -591,9 +591,6 @@ SPATCH_FLAGS = --all-includes --patch .\n \n ### --- END CONFIGURATION SECTION ---\n \n-# Those must not be GNU-specific; they are shared with perl/ which may\n-# be built by a different compiler. (Note that this is an artifact now\n-# but it still might be nice to keep that distinction.)\n BASIC_CFLAGS = -I.\n BASIC_LDFLAGS =\n \n-- \n2.21.0.rc2.261.ga7da99ff1b\n\n"},{"id":"369915","messageId":"20190222144127.32248-1-avarab@gmail.com","threadId":"50557","inReplyTo":"20190222105658.26831-1-avarab@gmail.com","subject":"[PATCH v2 0/6] Makefile: allow for combining DEVELOPER= and CFLAGS=\"...\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-02-22T14:41:21Z","receivedAt":"2019-02-22T14:41:43Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This fixes a long-standing annoyance of mine of not being able to set\nmy own CFLAGS combined with DEVELOPER=\"...\".\n\nThere *should* be no other functional changes, but I'm moving around\nsome core Makefile logic, so this needs to be carefully reviewed.\n\nI've tried installing with this, setting flags involved etc. It works\nas expected, but we have no tests for the Makefile itself, so I might\nhave missed something.\n\nI sent a stand-alone \"Makefile: remove an out-of-date comment\"[1] a\nbit too soon as it turns out. I thought it could be stand-alone, but\nas it turns out I needed to move some code adjacent to it. It would\nconflict with this, so it's now bundled up with it as a v2.\n\n1. https://public-inbox.org/git/20190222105658.26831-1-avarab@gmail.com/\n\nÆvar Arnfjörð Bjarmason (6):\n  Makefile: remove an out-of-date comment\n  Makefile: move \"strip\" assignment down from flags\n  Makefile: add/remove comments at top and tweak whitespace\n  Makefile: Move *_LIBS assignment into its own section\n  Makefile: move the setting of *FLAGS closer to \"include\"\n  Makefile: allow for combining DEVELOPER=1 and CFLAGS=\"...\"\n\n Makefile       | 63 ++++++++++++++++++++++++++------------------------\n config.mak.dev | 44 +++++++++++++++++------------------\n 2 files changed, 55 insertions(+), 52 deletions(-)\n\n-- \n2.21.0.rc2.1.g2d5e20a900.dirty\n\n"},{"id":"369916","messageId":"20190222144127.32248-2-avarab@gmail.com","threadId":"50557","inReplyTo":"20190222105658.26831-1-avarab@gmail.com","subject":"[PATCH v2 1/6] Makefile: remove an out-of-date comment","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-02-22T14:41:22Z","receivedAt":"2019-02-22T14:41:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Remove a comment referring to a caveat that hasn't been applicable\nsince 18b0fc1ce1 (\"Git.pm: Kill Git.xs for now\", 2006-09-23).\n\nAt the time of 8d7f586f13 (\"Git.pm: Support for perl/ being built by a\ndifferent compiler\", 2006-06-25) some of the code in perl would be\nbuilt by a C compiler, but support for that went away a few months\nlater in 18b0fc1ce1 discussed above.\n\nSince my 20d2a30f8f (\"Makefile: replace perl/Makefile.PL with simple\nmake rules\", 2017-12-10) the perl/ directory doesn't even have its own\nbuild process.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile | 3 ---\n 1 file changed, 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex c5240942f2..97e922cc41 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -591,9 +591,6 @@ SPATCH_FLAGS = --all-includes --patch .\n \n ### --- END CONFIGURATION SECTION ---\n \n-# Those must not be GNU-specific; they are shared with perl/ which may\n-# be built by a different compiler. (Note that this is an artifact now\n-# but it still might be nice to keep that distinction.)\n BASIC_CFLAGS = -I.\n BASIC_LDFLAGS =\n \n-- \n2.21.0.rc2.1.g2d5e20a900.dirty\n\n"},{"id":"369917","messageId":"20190222144127.32248-3-avarab@gmail.com","threadId":"50557","inReplyTo":"20190222105658.26831-1-avarab@gmail.com","subject":"[PATCH v2 2/6] Makefile: move \"strip\" assignment down from flags","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-02-22T14:41:23Z","receivedAt":"2019-02-22T14:41:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Move the assignment of the \"STRIP\" variable down to where we're\nsetting variables with the names of other programs.\n\nFor consistency with those use \"=\" for the assignment instead of\n\"?=\". I can't imagine why this would need to be different than the\nrest, and 4dc00021f7 (\"Makefile: add 'strip' target\", 2006-01-12)\nwhich added it doesn't provide an explanation.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 97e922cc41..c53727e44b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -512,7 +512,6 @@ CFLAGS = -g -O2 -Wall\n LDFLAGS =\n ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n ALL_LDFLAGS = $(LDFLAGS)\n-STRIP ?= strip\n \n # Create as necessary, replace existing, make ranlib unneeded.\n ARFLAGS = rcs\n@@ -576,6 +575,7 @@ CURL_CONFIG = curl-config\n PTHREAD_LIBS = -lpthread\n PTHREAD_CFLAGS =\n GCOV = gcov\n+STRIP = strip\n SPATCH = spatch\n \n export TCL_PATH TCLTK_PATH\n-- \n2.21.0.rc2.1.g2d5e20a900.dirty\n\n"},{"id":"369918","messageId":"20190222144127.32248-4-avarab@gmail.com","threadId":"50557","inReplyTo":"20190222105658.26831-1-avarab@gmail.com","subject":"[PATCH v2 3/6] Makefile: add/remove comments at top and tweak whitespace","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-02-22T14:41:24Z","receivedAt":"2019-02-22T14:41:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The top of the Makfile is mostly separated into logical steps like set\ndefault configuration, set programs etc., but there's some deviation\nfrom that.\n\nLet's add mostly comments where they're missing, remove those that\ndon't add anything. The whitespace tweaking makes subsequent patches\nsmaller.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile | 10 +++-------\n 1 file changed, 3 insertions(+), 7 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex c53727e44b..0870fd4651 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -507,15 +507,14 @@ GIT-VERSION-FILE: FORCE\n -include GIT-VERSION-FILE\n \n # CFLAGS and LDFLAGS are for the users to override from the command line.\n-\n CFLAGS = -g -O2 -Wall\n LDFLAGS =\n ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n ALL_LDFLAGS = $(LDFLAGS)\n-\n-# Create as necessary, replace existing, make ranlib unneeded.\n ARFLAGS = rcs\n \n+# Set our default configuration.\n+#\n # Among the variables below, these:\n #   gitexecdir\n #   template_dir\n@@ -560,6 +559,7 @@ perllibdir_relative = $(patsubst $(prefix)/%,%,$(perllibdir))\n \n export prefix bindir sharedir sysconfdir gitwebdir perllibdir localedir\n \n+# Set our default programs\n CC = cc\n AR = ar\n RM = rm -f\n@@ -587,10 +587,6 @@ SP_EXTRA_FLAGS =\n \n SPATCH_FLAGS = --all-includes --patch .\n \n-\n-\n-### --- END CONFIGURATION SECTION ---\n-\n BASIC_CFLAGS = -I.\n BASIC_LDFLAGS =\n \n-- \n2.21.0.rc2.1.g2d5e20a900.dirty\n\n"},{"id":"369919","messageId":"20190222144127.32248-5-avarab@gmail.com","threadId":"50557","inReplyTo":"20190222105658.26831-1-avarab@gmail.com","subject":"[PATCH v2 4/6] Makefile: Move *_LIBS assignment into its own section","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-02-22T14:41:25Z","receivedAt":"2019-02-22T14:41:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Now the only other non-program assignment in the previous list is\nPTHREAD_CFLAGS, which'll be moved elsewhere in a follow-up chang.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile | 4 +++-\n 1 file changed, 3 insertions(+), 1 deletion(-)\n\ndiff --git a/Makefile b/Makefile\nindex 0870fd4651..59674ce9d7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -572,7 +572,6 @@ TCLTK_PATH = wish\n XGETTEXT = xgettext\n MSGFMT = msgfmt\n CURL_CONFIG = curl-config\n-PTHREAD_LIBS = -lpthread\n PTHREAD_CFLAGS =\n GCOV = gcov\n STRIP = strip\n@@ -580,6 +579,9 @@ SPATCH = spatch\n \n export TCL_PATH TCLTK_PATH\n \n+# Set our default LIBS variables\n+PTHREAD_LIBS = -lpthread\n+\n # user customisation variable for 'sparse' target\n SPARSE_FLAGS ?=\n # internal/platform customisation variable for 'sparse'\n-- \n2.21.0.rc2.1.g2d5e20a900.dirty\n\n"},{"id":"369920","messageId":"20190222144127.32248-6-avarab@gmail.com","threadId":"50557","inReplyTo":"20190222105658.26831-1-avarab@gmail.com","subject":"[PATCH v2 5/6] Makefile: move the setting of *FLAGS closer to \"include\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-02-22T14:41:26Z","receivedAt":"2019-02-22T14:41:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Move the setting of variables like CFLAGS down past settings like\n\"prefix\" and default programs like \"TAR\" to just before we do the\ninclude from \"config.mak.*\".\n\nThere's no functional changes here yet, but move note that\n\"ALL_CFLAGS\" and \"ALL_LDFLAGS\" are moved below the include. A\nfollow-up change will tweak those depending on a variable set in\nconfig.mak.dev.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile | 40 ++++++++++++++++++++++------------------\n 1 file changed, 22 insertions(+), 18 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 59674ce9d7..82cfd6c2e4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -506,13 +506,6 @@ GIT-VERSION-FILE: FORCE\n \t@$(SHELL_PATH) ./GIT-VERSION-GEN\n -include GIT-VERSION-FILE\n \n-# CFLAGS and LDFLAGS are for the users to override from the command line.\n-CFLAGS = -g -O2 -Wall\n-LDFLAGS =\n-ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n-ALL_LDFLAGS = $(LDFLAGS)\n-ARFLAGS = rcs\n-\n # Set our default configuration.\n #\n # Among the variables below, these:\n@@ -572,7 +565,6 @@ TCLTK_PATH = wish\n XGETTEXT = xgettext\n MSGFMT = msgfmt\n CURL_CONFIG = curl-config\n-PTHREAD_CFLAGS =\n GCOV = gcov\n STRIP = strip\n SPATCH = spatch\n@@ -582,16 +574,6 @@ export TCL_PATH TCLTK_PATH\n # Set our default LIBS variables\n PTHREAD_LIBS = -lpthread\n \n-# user customisation variable for 'sparse' target\n-SPARSE_FLAGS ?=\n-# internal/platform customisation variable for 'sparse'\n-SP_EXTRA_FLAGS =\n-\n-SPATCH_FLAGS = --all-includes --patch .\n-\n-BASIC_CFLAGS = -I.\n-BASIC_LDFLAGS =\n-\n # Guard against environment variables\n BUILTIN_OBJS =\n BUILT_INS =\n@@ -1160,6 +1142,25 @@ ifeq ($(wildcard sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/s\n DC_SHA1_SUBMODULE = auto\n endif\n \n+# Set CFLAGS, LDFLAGS and other *FLAGS variables. These might be\n+# tweaked by config.* below as well as the command-line, both of\n+# which'll override these defaults.\n+CFLAGS = -g -O2 -Wall\n+LDFLAGS =\n+BASIC_CFLAGS = -I.\n+BASIC_LDFLAGS =\n+\n+# library flags\n+ARFLAGS = rcs\n+PTHREAD_CFLAGS =\n+\n+# For the 'sparse' target\n+SPARSE_FLAGS ?=\n+SP_EXTRA_FLAGS =\n+\n+# For the 'coccicheck' target\n+SPATCH_FLAGS = --all-includes --patch .\n+\n include config.mak.uname\n -include config.mak.autogen\n -include config.mak\n@@ -1168,6 +1169,9 @@ ifdef DEVELOPER\n include config.mak.dev\n endif\n \n+ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n+ALL_LDFLAGS = $(LDFLAGS)\n+\n comma := ,\n empty :=\n space := $(empty) $(empty)\n-- \n2.21.0.rc2.1.g2d5e20a900.dirty\n\n"},{"id":"369921","messageId":"20190222144127.32248-7-avarab@gmail.com","threadId":"50557","inReplyTo":"20190222105658.26831-1-avarab@gmail.com","subject":"[PATCH v2 6/6] Makefile: allow for combining DEVELOPER=1 and CFLAGS=\"...\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-02-22T14:41:27Z","receivedAt":"2019-02-22T14:41:55Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Ever since the DEVELOPER=1 facility introduced there's been no way to\nhave custom CFLAGS (e.g. CFLAGS=\"-O0 -g -ggdb3\") while still\nbenefiting from the set of warnings and assertions DEVELOPER=1\nenables.\n\nThis is because the semantics of variables in the Makefile are such\nthat the user setting CFLAGS overrides anything we set, including what\nwe're doing in config.mak.dev[1].\n\nSo let's introduce a \"DEVELOPER_CFLAGS\" variable in config.mak.dev and\nadd it to ALL_CFLAGS. Before this the ALL_CFLAGS variable\nwould (basically, there's some nuance we won't go into) be set to:\n\n    $(CPPFLAGS) [$(CFLAGS) *or* $(CFLAGS) in config.mak.dev] $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS)\n\nBut will now be:\n\n    $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS)\n\nThe reason for putting DEVELOPER_CFLAGS first is to allow for\nselectively overriding something DEVELOPER=1 brings in. On both GCC\nand Clang later settings override earlier ones. E.g. \"-Wextra\n-Wno-extra\" will enable no \"extra\" warnings, but not if those two\narguments are reversed.\n\nExamples of things that weren't possible before, but are now:\n\n    # Use -O0 instead of -O2 for less painful debuggng\n    DEVELOPER=1 CFLAGS=\"-O0 -g\"\n    # DEVELOPER=1 plus -Wextra, but disable some of the warnings\n    DEVELOPER=1 DEVOPTS=\"no-error extra-all\" CFLAGS=\"-O0 -g -Wno-unused-parameter\"\n\nThe reason for the patches leading up to this one re-arranged the\nvarious *FLAGS assignments and includes is just for readability. The\nMakefile supports assignments out of order, e.g.:\n\n    $ cat Makefile\n    X = $(A) $(B) $(C)\n    A = A\n    B = B\n    include c.mak\n    all:\n            @echo $(X)\n    $ cat c.mak\n    C=C\n    $ make\n    A B C\n\nSo we could have gotten away with the much smaller change of changing\n\"CFLAGS\" in config.mak.dev to \"DEVELOPER_CFLAGS\" and adding that to\nALL_CFLAGS earlier in the Makefile \"before\" the config.mak.*\nincludes. But I think it's more readable to use variables \"after\"\nthey're included.\n\n1. https://www.gnu.org/software/make/manual/html_node/Overriding.html\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile       |  8 ++++++--\n config.mak.dev | 44 ++++++++++++++++++++++----------------------\n 2 files changed, 28 insertions(+), 24 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 82cfd6c2e4..710a3475a9 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -479,7 +479,11 @@ all::\n #\n # Define DEVELOPER to enable more compiler warnings. Compiler version\n # and family are auto detected, but could be overridden by defining\n-# COMPILER_FEATURES (see config.mak.dev)\n+# COMPILER_FEATURES (see config.mak.dev). You can still set\n+# CFLAGS=\"...\" in combination with DEVELOPER enables, whether that's\n+# for tweaking something unrelated (e.g. optimization level), or for\n+# selectively overriding something DEVELOPER or one of the DEVOPTS\n+# (see just below) brings in.\n #\n # When DEVELOPER is set, DEVOPTS can be used to control compiler\n # options.  This variable contains keywords separated by\n@@ -1169,7 +1173,7 @@ ifdef DEVELOPER\n include config.mak.dev\n endif\n \n-ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n+ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)\n ALL_LDFLAGS = $(LDFLAGS)\n \n comma := ,\ndiff --git a/config.mak.dev b/config.mak.dev\nindex 7354fe15b3..bf1f3fcdee 100644\n--- a/config.mak.dev\n+++ b/config.mak.dev\n@@ -1,41 +1,41 @@\n ifeq ($(filter no-error,$(DEVOPTS)),)\n-CFLAGS += -Werror\n+DEVELOPER_CFLAGS += -Werror\n endif\n ifneq ($(filter pedantic,$(DEVOPTS)),)\n-CFLAGS += -pedantic\n+DEVELOPER_CFLAGS += -pedantic\n # don't warn for each N_ use\n-CFLAGS += -DUSE_PARENS_AROUND_GETTEXT_N=0\n-endif\n-CFLAGS += -Wall\n-CFLAGS += -Wdeclaration-after-statement\n-CFLAGS += -Wformat-security\n-CFLAGS += -Wno-format-zero-length\n-CFLAGS += -Wold-style-definition\n-CFLAGS += -Woverflow\n-CFLAGS += -Wpointer-arith\n-CFLAGS += -Wstrict-prototypes\n-CFLAGS += -Wunused\n-CFLAGS += -Wvla\n+DEVELOPER_CFLAGS += -DUSE_PARENS_AROUND_GETTEXT_N=0\n+endif\n+DEVELOPER_CFLAGS += -Wall\n+DEVELOPER_CFLAGS += -Wdeclaration-after-statement\n+DEVELOPER_CFLAGS += -Wformat-security\n+DEVELOPER_CFLAGS += -Wno-format-zero-length\n+DEVELOPER_CFLAGS += -Wold-style-definition\n+DEVELOPER_CFLAGS += -Woverflow\n+DEVELOPER_CFLAGS += -Wpointer-arith\n+DEVELOPER_CFLAGS += -Wstrict-prototypes\n+DEVELOPER_CFLAGS += -Wunused\n+DEVELOPER_CFLAGS += -Wvla\n \n ifndef COMPILER_FEATURES\n COMPILER_FEATURES := $(shell ./detect-compiler $(CC))\n endif\n \n ifneq ($(filter clang4,$(COMPILER_FEATURES)),)\n-CFLAGS += -Wtautological-constant-out-of-range-compare\n+DEVELOPER_CFLAGS += -Wtautological-constant-out-of-range-compare\n endif\n \n ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),)\n-CFLAGS += -Wextra\n+DEVELOPER_CFLAGS += -Wextra\n # if a function is public, there should be a prototype and the right\n # header file should be included. If not, it should be static.\n-CFLAGS += -Wmissing-prototypes\n+DEVELOPER_CFLAGS += -Wmissing-prototypes\n ifeq ($(filter extra-all,$(DEVOPTS)),)\n # These are disabled because we have these all over the place.\n-CFLAGS += -Wno-empty-body\n-CFLAGS += -Wno-missing-field-initializers\n-CFLAGS += -Wno-sign-compare\n-CFLAGS += -Wno-unused-parameter\n+DEVELOPER_CFLAGS += -Wno-empty-body\n+DEVELOPER_CFLAGS += -Wno-missing-field-initializers\n+DEVELOPER_CFLAGS += -Wno-sign-compare\n+DEVELOPER_CFLAGS += -Wno-unused-parameter\n endif\n endif\n \n@@ -43,6 +43,6 @@ endif\n # not worth fixing since newer compilers correctly stop complaining\n ifneq ($(filter gcc4,$(COMPILER_FEATURES)),)\n ifeq ($(filter gcc5,$(COMPILER_FEATURES)),)\n-CFLAGS += -Wno-uninitialized\n+DEVELOPER_CFLAGS += -Wno-uninitialized\n endif\n endif\n-- \n2.21.0.rc2.1.g2d5e20a900.dirty\n\n"},{"id":"369924","messageId":"20190222150946.GC5090@sigill.intra.peff.net","threadId":"50557","inReplyTo":"20190222105658.26831-1-avarab@gmail.com","subject":"Re: [PATCH] Makefile: remove an out-of-date comment","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-22T15:09:46Z","receivedAt":"2019-02-22T15:09:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 22, 2019 at 11:56:58AM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> Remove a comment referring to a caveat that hasn't been applicable\n> since 18b0fc1ce1 (\"Git.pm: Kill Git.xs for now\", 2006-09-23).\n> \n> At the time of 8d7f586f13 (\"Git.pm: Support for perl/ being built by a\n> different compiler\", 2006-06-25) some of the code in perl would be\n> built by a C compiler, but support for that went away a few months\n> later in 18b0fc1ce1 discussed above.\n> \n> Since my 20d2a30f8f (\"Makefile: replace perl/Makefile.PL with simple\n> make rules\", 2017-12-10) the perl/ directory doesn't even have its own\n> build process.\n\nI liked that commit before, but seeing that it makes this hairy corner\ncase go away, I like it even more. ;)\n\nThe patch itself seems obviously correct.\n\n-Peff\n"},{"id":"369925","messageId":"20190222151607.GD5090@sigill.intra.peff.net","threadId":"50557","inReplyTo":"20190222144127.32248-1-avarab@gmail.com","subject":"Re: [PATCH v2 0/6] Makefile: allow for combining DEVELOPER= and CFLAGS=\"...\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-22T15:16:07Z","receivedAt":"2019-02-22T15:16:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 22, 2019 at 03:41:21PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> This fixes a long-standing annoyance of mine of not being able to set\n> my own CFLAGS combined with DEVELOPER=\"...\".\n\nI already do this, via my config.mak. Are you passing these on the\ncommand-line (and thus your CFLAGS= prevents DEVELOPER= from adding\nanything to it)? That does seem worth fixing.\n\nMy personal complaint is that you cannot override DEVELOPER flags via\nconfig.mak, because of the ordering. But it looks like your patch 6\nspecifically addresses that (yay!).\n\n-Peff\n"},{"id":"369926","messageId":"20190222151834.GA21524@sigill.intra.peff.net","threadId":"50557","inReplyTo":"20190222144127.32248-3-avarab@gmail.com","subject":"Re: [PATCH v2 2/6] Makefile: move \"strip\" assignment down from flags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-22T15:18:34Z","receivedAt":"2019-02-22T15:18:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 22, 2019 at 03:41:23PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> Move the assignment of the \"STRIP\" variable down to where we're\n> setting variables with the names of other programs.\n> \n> For consistency with those use \"=\" for the assignment instead of\n> \"?=\". I can't imagine why this would need to be different than the\n> rest, and 4dc00021f7 (\"Makefile: add 'strip' target\", 2006-01-12)\n> which added it doesn't provide an explanation.\n\nThis might annoy somebody expecting $STRIP in the environment to have\nprecedence. But I agree that consistency is probably our best strategy\nhere, and I don't see any reason the same argument would not apply to\n$SPATCH, or $CC for that matter.\n\n(So I could see an argument for moving them all to \"?=\", but that can\ncreate its own confusion as environment variables accidentally start\ntaking effect).\n\n-Peff\n"},{"id":"369927","messageId":"20190222152110.GB21524@sigill.intra.peff.net","threadId":"50557","inReplyTo":"20190222144127.32248-5-avarab@gmail.com","subject":"Re: [PATCH v2 4/6] Makefile: Move *_LIBS assignment into its own section","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-22T15:21:10Z","receivedAt":"2019-02-22T15:21:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 22, 2019 at 03:41:25PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> Now the only other non-program assignment in the previous list is\n> PTHREAD_CFLAGS, which'll be moved elsewhere in a follow-up chang.\n\nHmm. I get why this organization might make sense. But it also might be\nconvenient to see all of the pthread-related flags/libs together.\n\nI dunno. I guess we are not expecting people to touch these manually\nanyway, so it may not matter.\n\n-Peff\n"},{"id":"369928","messageId":"20190222152926.GC21524@sigill.intra.peff.net","threadId":"50557","inReplyTo":"20190222144127.32248-7-avarab@gmail.com","subject":"Re: [PATCH v2 6/6] Makefile: allow for combining DEVELOPER=1 and CFLAGS=\"...\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-22T15:29:26Z","receivedAt":"2019-02-22T15:29:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 22, 2019 at 03:41:27PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> So let's introduce a \"DEVELOPER_CFLAGS\" variable in config.mak.dev and\n> add it to ALL_CFLAGS. Before this the ALL_CFLAGS variable\n> would (basically, there's some nuance we won't go into) be set to:\n\nYeah, I definitely think this is the right direction, and the patch\nlooks good to me.\n\n> Examples of things that weren't possible before, but are now:\n> \n>     # Use -O0 instead of -O2 for less painful debuggng\n>     DEVELOPER=1 CFLAGS=\"-O0 -g\"\n\nSort an aside, but in my config.mak I have:\n\n  O=0\n  CFLAGS = -g -O$(O)\n\nwhich lets me do \"make O=2\", which is nice and short. I wonder if other\npeople would want that, too.\n\n(I default to -O0 because 99% of my compiles are for development and\ndebugging, so the compile being fast is more important than the build\nresult being fast; for perf testing or daily use, I use O=2).\n\n> So we could have gotten away with the much smaller change of changing\n> \"CFLAGS\" in config.mak.dev to \"DEVELOPER_CFLAGS\" and adding that to\n> ALL_CFLAGS earlier in the Makefile \"before\" the config.mak.*\n> includes. But I think it's more readable to use variables \"after\"\n> they're included.\n\nI think the reordering does make things more readable overall. And it\nmay be more sensible if config.mak uses any \":=\" assignments.\n\n-Peff\n"},{"id":"369932","messageId":"CAPig+cTwVYPLWnAJscOuNNRoBpuheQoQgQX-K8ia4gJ-5zAyiw@mail.gmail.com","threadId":"50557","inReplyTo":"20190222144127.32248-5-avarab@gmail.com","subject":"Re: [PATCH v2 4/6] Makefile: Move *_LIBS assignment into its own section","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2019-02-22T15:49:41Z","receivedAt":"2019-02-22T15:49:53Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Feb 22, 2019 at 9:41 AM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> Now the only other non-program assignment in the previous list is\n> PTHREAD_CFLAGS, which'll be moved elsewhere in a follow-up chang.\n\ns/chang/change/\n\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n"},{"id":"369980","messageId":"4821a31a-f5ee-39ef-5302-8b71430815fe@ramsayjones.plus.com","threadId":"50557","inReplyTo":"20190222151834.GA21524@sigill.intra.peff.net","subject":"Re: [PATCH v2 2/6] Makefile: move \"strip\" assignment down from flags","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2019-02-22T21:58:14Z","receivedAt":"2019-02-22T21:58:18Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 22/02/2019 15:18, Jeff King wrote:\n> On Fri, Feb 22, 2019 at 03:41:23PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> \n>> Move the assignment of the \"STRIP\" variable down to where we're\n>> setting variables with the names of other programs.\n>>\n>> For consistency with those use \"=\" for the assignment instead of\n>> \"?=\". I can't imagine why this would need to be different than the\n>> rest, and 4dc00021f7 (\"Makefile: add 'strip' target\", 2006-01-12)\n>> which added it doesn't provide an explanation.\n> \n> This might annoy somebody expecting $STRIP in the environment to have\n> precedence. But I agree that consistency is probably our best strategy\n> here, and I don't see any reason the same argument would not apply to\n> $SPATCH, or $CC for that matter.\n> \n> (So I could see an argument for moving them all to \"?=\", but that can\n> create its own confusion as environment variables accidentally start\n> taking effect).\n\n$STRIP and $SPATCH will work OK, but you would be disappointed\nwith $CC (or any other variable from make's built-in database). ;-)\n\nTry this:\n\n  $ cat -n Makefile \n       1\t\n       2\tCC ?= gcc\n       3\t\n       4\tall:\n       5\t\t@echo \"CC is $(CC), origin \" $(origin CC)\n       6\t\n  $ \n\nThe command-line works OK:\n\n  $ make CC=cmd-line\n  CC is cmd-line, origin  command line\n  $ \n\nSo does the environment:\n\n  $ CC=env make\n  CC is env, origin  environment\n  $ \n\nBut that conditional assignment:\n\n  $ make\n  CC is cc, origin  default\n  $ \n  \n... not so much! :-D\n\nATB,\nRamsay Jones\n\n"},{"id":"369981","messageId":"87zhqn4h8n.fsf@evledraar.gmail.com","threadId":"50557","inReplyTo":"20190222151607.GD5090@sigill.intra.peff.net","subject":"Re: [PATCH v2 0/6] Makefile: allow for combining DEVELOPER= and CFLAGS=\"...\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-02-22T21:59:20Z","receivedAt":"2019-02-22T21:59:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Feb 22 2019, Jeff King wrote:\n\n> On Fri, Feb 22, 2019 at 03:41:21PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n>> This fixes a long-standing annoyance of mine of not being able to set\n>> my own CFLAGS combined with DEVELOPER=\"...\".\n>\n> I already do this, via my config.mak. Are you passing these on the\n> command-line (and thus your CFLAGS= prevents DEVELOPER= from adding\n> anything to it)? That does seem worth fixing.\n\nYes. I'll make this clearer in v2. I never use config.mak, so I didn't\neven know that worked. Having refreshed on the docs[1], it's because\ncommand-line arguments are \"override variables\".\n\n> My personal complaint is that you cannot override DEVELOPER flags via\n> config.mak, because of the ordering. But it looks like your patch 6\n> specifically addresses that (yay!).\n\nDo you mean DEVOPTS? That works for me.\n\n    $ cat config.mak\n    CFLAGS = -blah\n    DEVELOPER = 1\n    DEVOPTS = pedantic\n    $ make V=1\n    [...] cc [...]  -blah -Werror -pedantic [...]\n\nBut then of course:\n\n    $ rm config.mak\n    $ CFLAGS=-cmd DEVELOPER=1 DEVOPTS=pedantic V=1\n\nOnly includes the flags, and ignores DEVELOPER & DEVOPTS.\n\n1. https://www.gnu.org/software/make/manual/html_node/Overriding.html\n"},{"id":"370000","messageId":"xmqqva1bo1h5.fsf@gitster-ct.c.googlers.com","threadId":"50557","inReplyTo":"20190222144127.32248-6-avarab@gmail.com","subject":"Re: [PATCH v2 5/6] Makefile: move the setting of *FLAGS closer to \"include\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-22T23:19:34Z","receivedAt":"2019-02-22T23:19:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> Move the setting of variables like CFLAGS down past settings like\n> \"prefix\" and default programs like \"TAR\" to just before we do the\n> include from \"config.mak.*\".\n>\n> There's no functional changes here yet, but move note that\n> \"ALL_CFLAGS\" and \"ALL_LDFLAGS\" are moved below the include. A\n> follow-up change will tweak those depending on a variable set in\n> config.mak.dev.\n\nSorry, but cannot parse the sentence around \"move note\".\n\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n>  Makefile | 40 ++++++++++++++++++++++------------------\n>  1 file changed, 22 insertions(+), 18 deletions(-)\n>\n> diff --git a/Makefile b/Makefile\n> index 59674ce9d7..82cfd6c2e4 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -506,13 +506,6 @@ GIT-VERSION-FILE: FORCE\n>  \t@$(SHELL_PATH) ./GIT-VERSION-GEN\n>  -include GIT-VERSION-FILE\n>  \n> -# CFLAGS and LDFLAGS are for the users to override from the command line.\n> -CFLAGS = -g -O2 -Wall\n> -LDFLAGS =\n> -ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n> -ALL_LDFLAGS = $(LDFLAGS)\n> -ARFLAGS = rcs\n> -\n>  # Set our default configuration.\n>  #\n>  # Among the variables below, these:\n> @@ -572,7 +565,6 @@ TCLTK_PATH = wish\n>  XGETTEXT = xgettext\n>  MSGFMT = msgfmt\n>  CURL_CONFIG = curl-config\n> -PTHREAD_CFLAGS =\n>  GCOV = gcov\n>  STRIP = strip\n>  SPATCH = spatch\n> @@ -582,16 +574,6 @@ export TCL_PATH TCLTK_PATH\n>  # Set our default LIBS variables\n>  PTHREAD_LIBS = -lpthread\n>  \n> -# user customisation variable for 'sparse' target\n> -SPARSE_FLAGS ?=\n> -# internal/platform customisation variable for 'sparse'\n> -SP_EXTRA_FLAGS =\n> -\n> -SPATCH_FLAGS = --all-includes --patch .\n> -\n> -BASIC_CFLAGS = -I.\n> -BASIC_LDFLAGS =\n> -\n>  # Guard against environment variables\n>  BUILTIN_OBJS =\n>  BUILT_INS =\n> @@ -1160,6 +1142,25 @@ ifeq ($(wildcard sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/s\n>  DC_SHA1_SUBMODULE = auto\n>  endif\n>  \n> +# Set CFLAGS, LDFLAGS and other *FLAGS variables. These might be\n> +# tweaked by config.* below as well as the command-line, both of\n> +# which'll override these defaults.\n> +CFLAGS = -g -O2 -Wall\n> +LDFLAGS =\n> +BASIC_CFLAGS = -I.\n> +BASIC_LDFLAGS =\n> +\n> +# library flags\n> +ARFLAGS = rcs\n> +PTHREAD_CFLAGS =\n> +\n> +# For the 'sparse' target\n> +SPARSE_FLAGS ?=\n\nMakes us wonder why only this one is different and uses ?= ;-)\n\n> +SP_EXTRA_FLAGS =\n> +\n> +# For the 'coccicheck' target\n> +SPATCH_FLAGS = --all-includes --patch .\n> +\n>  include config.mak.uname\n>  -include config.mak.autogen\n>  -include config.mak\n> @@ -1168,6 +1169,9 @@ ifdef DEVELOPER\n>  include config.mak.dev\n>  endif\n>  \n> +ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS)\n> +ALL_LDFLAGS = $(LDFLAGS)\n> +\n>  comma := ,\n>  empty :=\n>  space := $(empty) $(empty)\n"},{"id":"370001","messageId":"xmqqr2bzo1ar.fsf@gitster-ct.c.googlers.com","threadId":"50557","inReplyTo":"20190222144127.32248-7-avarab@gmail.com","subject":"Re: [PATCH v2 6/6] Makefile: allow for combining DEVELOPER=1 and CFLAGS=\"...\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-02-22T23:23:24Z","receivedAt":"2019-02-22T23:23:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> diff --git a/config.mak.dev b/config.mak.dev\n> index 7354fe15b3..bf1f3fcdee 100644\n> --- a/config.mak.dev\n> +++ b/config.mak.dev\n> @@ -1,41 +1,41 @@\n>  ifeq ($(filter no-error,$(DEVOPTS)),)\n> -CFLAGS += -Werror\n> +DEVELOPER_CFLAGS += -Werror\n\nThis is the first mention of DEVELOPER_CFLAGS; what is -Werror\nappended to?  Any random thing in the environment or command line?\n\n>  endif\n"},{"id":"370015","messageId":"20190223132245.GA22403@sigill.intra.peff.net","threadId":"50557","inReplyTo":"87zhqn4h8n.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 0/6] Makefile: allow for combining DEVELOPER= and CFLAGS=\"...\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-23T13:22:45Z","receivedAt":"2019-02-23T13:22:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 22, 2019 at 10:59:20PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> > My personal complaint is that you cannot override DEVELOPER flags via\n> > config.mak, because of the ordering. But it looks like your patch 6\n> > specifically addresses that (yay!).\n> \n> Do you mean DEVOPTS? That works for me.\n\nNo, I meant overriding a developer CFLAG with a later one on the command\nline. I.e., putting this in your config.mak:\n\n  DEVELOPER = 1\n  CFLAGS += -Wno-foo\n\ndoesn't work, because the user CFLAGS comes first. Your patch 6 does the\nright thing.\n\n-Peff\n"},{"id":"370016","messageId":"20190223132340.GB22403@sigill.intra.peff.net","threadId":"50557","inReplyTo":"4821a31a-f5ee-39ef-5302-8b71430815fe@ramsayjones.plus.com","subject":"Re: [PATCH v2 2/6] Makefile: move \"strip\" assignment down from flags","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-02-23T13:23:40Z","receivedAt":"2019-02-23T13:23:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 22, 2019 at 09:58:14PM +0000, Ramsay Jones wrote:\n\n> > This might annoy somebody expecting $STRIP in the environment to have\n> > precedence. But I agree that consistency is probably our best strategy\n> > here, and I don't see any reason the same argument would not apply to\n> > $SPATCH, or $CC for that matter.\n> > \n> > (So I could see an argument for moving them all to \"?=\", but that can\n> > create its own confusion as environment variables accidentally start\n> > taking effect).\n> \n> $STRIP and $SPATCH will work OK, but you would be disappointed\n> with $CC (or any other variable from make's built-in database). ;-)\n\nOof. That's confusing. All the more reason not to go in the direction of\n\"?=\". Thanks for an interesting tidbit.\n\n-Peff\n"}]}