git/config.mak.dev

103 lines
3.4 KiB
Plaintext
Raw Normal View History

ifndef COMPILER_FEATURES
COMPILER_FEATURES := $(shell ./detect-compiler $(CC))
endif
ifeq ($(filter no-error,$(DEVOPTS)),)
Makefile: allow for combining DEVELOPER=1 and CFLAGS="..." Ever since the DEVELOPER=1 facility introduced there's been no way to have custom CFLAGS (e.g. CFLAGS="-O0 -g -ggdb3") while still benefiting from the set of warnings and assertions DEVELOPER=1 enables. This is because the semantics of variables in the Makefile are such that the user setting CFLAGS overrides anything we set, including what we're doing in config.mak.dev[1]. So let's introduce a "DEVELOPER_CFLAGS" variable in config.mak.dev and add it to ALL_CFLAGS. Before this the ALL_CFLAGS variable would (basically, there's some nuance we won't go into) be set to: $(CPPFLAGS) [$(CFLAGS) *or* $(CFLAGS) in config.mak.dev] $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) But will now be: $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) The reason for putting DEVELOPER_CFLAGS first is to allow for selectively overriding something DEVELOPER=1 brings in. On both GCC and Clang later settings override earlier ones. E.g. "-Wextra -Wno-extra" will enable no "extra" warnings, but not if those two arguments are reversed. Examples of things that weren't possible before, but are now: # Use -O0 instead of -O2 for less painful debuggng DEVELOPER=1 CFLAGS="-O0 -g" # DEVELOPER=1 plus -Wextra, but disable some of the warnings DEVELOPER=1 DEVOPTS="no-error extra-all" CFLAGS="-O0 -g -Wno-unused-parameter" The reason for the patches leading up to this one re-arranged the various *FLAGS assignments and includes is just for readability. The Makefile supports assignments out of order, e.g.: $ cat Makefile X = $(A) $(B) $(C) A = A B = B include c.mak all: @echo $(X) $ cat c.mak C=C $ make A B C So we could have gotten away with the much smaller change of changing "CFLAGS" in config.mak.dev to "DEVELOPER_CFLAGS" and adding that to ALL_CFLAGS earlier in the Makefile "before" the config.mak.* includes. But I think it's more readable to use variables "after" they're included. 1. https://www.gnu.org/software/make/manual/html_node/Overriding.html Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-22 22:41:27 +08:00
DEVELOPER_CFLAGS += -Werror
SPARSE_FLAGS += -Wsparse-error
endif
DEVELOPER_CFLAGS += -Wall
ifeq ($(filter no-pedantic,$(DEVOPTS)),)
Makefile: allow for combining DEVELOPER=1 and CFLAGS="..." Ever since the DEVELOPER=1 facility introduced there's been no way to have custom CFLAGS (e.g. CFLAGS="-O0 -g -ggdb3") while still benefiting from the set of warnings and assertions DEVELOPER=1 enables. This is because the semantics of variables in the Makefile are such that the user setting CFLAGS overrides anything we set, including what we're doing in config.mak.dev[1]. So let's introduce a "DEVELOPER_CFLAGS" variable in config.mak.dev and add it to ALL_CFLAGS. Before this the ALL_CFLAGS variable would (basically, there's some nuance we won't go into) be set to: $(CPPFLAGS) [$(CFLAGS) *or* $(CFLAGS) in config.mak.dev] $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) But will now be: $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) The reason for putting DEVELOPER_CFLAGS first is to allow for selectively overriding something DEVELOPER=1 brings in. On both GCC and Clang later settings override earlier ones. E.g. "-Wextra -Wno-extra" will enable no "extra" warnings, but not if those two arguments are reversed. Examples of things that weren't possible before, but are now: # Use -O0 instead of -O2 for less painful debuggng DEVELOPER=1 CFLAGS="-O0 -g" # DEVELOPER=1 plus -Wextra, but disable some of the warnings DEVELOPER=1 DEVOPTS="no-error extra-all" CFLAGS="-O0 -g -Wno-unused-parameter" The reason for the patches leading up to this one re-arranged the various *FLAGS assignments and includes is just for readability. The Makefile supports assignments out of order, e.g.: $ cat Makefile X = $(A) $(B) $(C) A = A B = B include c.mak all: @echo $(X) $ cat c.mak C=C $ make A B C So we could have gotten away with the much smaller change of changing "CFLAGS" in config.mak.dev to "DEVELOPER_CFLAGS" and adding that to ALL_CFLAGS earlier in the Makefile "before" the config.mak.* includes. But I think it's more readable to use variables "after" they're included. 1. https://www.gnu.org/software/make/manual/html_node/Overriding.html Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-22 22:41:27 +08:00
DEVELOPER_CFLAGS += -pedantic
ifneq (($or $(filter gcc5,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),)
DEVELOPER_CFLAGS += -Wpedantic
ifneq ($(filter gcc10,$(COMPILER_FEATURES)),)
ifeq ($(uname_S),MINGW)
DEVELOPER_CFLAGS += -Wno-pedantic-ms-format
endif
Makefile: allow for combining DEVELOPER=1 and CFLAGS="..." Ever since the DEVELOPER=1 facility introduced there's been no way to have custom CFLAGS (e.g. CFLAGS="-O0 -g -ggdb3") while still benefiting from the set of warnings and assertions DEVELOPER=1 enables. This is because the semantics of variables in the Makefile are such that the user setting CFLAGS overrides anything we set, including what we're doing in config.mak.dev[1]. So let's introduce a "DEVELOPER_CFLAGS" variable in config.mak.dev and add it to ALL_CFLAGS. Before this the ALL_CFLAGS variable would (basically, there's some nuance we won't go into) be set to: $(CPPFLAGS) [$(CFLAGS) *or* $(CFLAGS) in config.mak.dev] $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) But will now be: $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) The reason for putting DEVELOPER_CFLAGS first is to allow for selectively overriding something DEVELOPER=1 brings in. On both GCC and Clang later settings override earlier ones. E.g. "-Wextra -Wno-extra" will enable no "extra" warnings, but not if those two arguments are reversed. Examples of things that weren't possible before, but are now: # Use -O0 instead of -O2 for less painful debuggng DEVELOPER=1 CFLAGS="-O0 -g" # DEVELOPER=1 plus -Wextra, but disable some of the warnings DEVELOPER=1 DEVOPTS="no-error extra-all" CFLAGS="-O0 -g -Wno-unused-parameter" The reason for the patches leading up to this one re-arranged the various *FLAGS assignments and includes is just for readability. The Makefile supports assignments out of order, e.g.: $ cat Makefile X = $(A) $(B) $(C) A = A B = B include c.mak all: @echo $(X) $ cat c.mak C=C $ make A B C So we could have gotten away with the much smaller change of changing "CFLAGS" in config.mak.dev to "DEVELOPER_CFLAGS" and adding that to ALL_CFLAGS earlier in the Makefile "before" the config.mak.* includes. But I think it's more readable to use variables "after" they're included. 1. https://www.gnu.org/software/make/manual/html_node/Overriding.html Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-22 22:41:27 +08:00
endif
endif
endif
config.mak.dev: specify -std=gnu99 for gcc/clang The point of DEVELOPER=1 is to turn up the warnings so we can catch portability or correctness mistakes at the compiler level. But since modern compilers tend to default to modern standards like gnu17, we might miss warnings about older standards, even though we expect Git to build with compilers that use them. So it's helpful for developer builds to set the -std argument to our lowest-common denominator. Traditionally this was c89, but since we're moving to assuming c99 in 7bc341e21b (git-compat-util: add a test balloon for C99 support, 2021-12-01) that seems like a good spot to land. And as explained in that commit, we want "gnu99" because we still want to take advantage of some extensions when they're available. The new argument kicks in only for clang and gcc (which we know to support "-std=" and "gnu" standards). And only for compiler versions which default to a newer standard. That will avoid accidentally silencing any build problems that non-developers would run into on older compilers that default to c89. My digging found that the default switched to gnu11 in gcc 5.1.0. Clang's documentation is less clear, but has done so since at least clang-7. So that's what I put in the conditional here. It's OK to err on the side of not-enabling this for older compilers. Most developers (as well as CI) are using much more recent versions, so any warnings will eventually surface. A concrete example is anonymous unions, which became legal in c11. Without this patch, "gcc -pedantic" will not complain about them, but will if we add in "-std=gnu99". Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-09 03:50:54 +08:00
Makefile: FreeBSD cannot do C99-or-below build In "make DEVELOPER=YesPlease" builds, we try to help developers to catch as many potential issues as they can by using -Wall and turning compilation warnings into errors. In the same spirit, we recently started adding -std=gnu99 to their CFLAGS, so that they can notice when they accidentally used language features beyond C99. It however turns out that FreeBSD 13.0 mistakenly uses C11 extension in its system header files regardless of what __STDC_VERSION__ says, which means that the platform (unless we tweak their system headers) cannot be used for this purpose. It seems that -std=gnu99 is only added conditionally even in today's config.mak.dev, so it is fine if we dropped -std=gnu99 from there. Which means that developers on FreeBSD cannot participate in vetting use of features beyond C99, but there are developers on other platforms who will, so it's not too bad. We might want a more "fundamental" fix to make the platform capable of taking -std=gnu99, like working around the use of unconditional C11 extension in its system header files by supplying a set of "replacement" definitions in our header files. We chose not to pursue such an approach for two reasons at this point: (1) The fix belongs to the FreeBSD project, not this project, and such an upstream fix may happen hopefully in a not-too-distant future. (2) Fixing such a bug in system header files and working it around can lead to unexpected breakages (other parts of their system header files may not be expecting to see and do not work well with our "replacement" definitions). This close to the final release of this cycle, we have no time for that. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-19 01:47:39 +08:00
ifneq ($(uname_S),FreeBSD)
config.mak.dev: specify -std=gnu99 for gcc/clang The point of DEVELOPER=1 is to turn up the warnings so we can catch portability or correctness mistakes at the compiler level. But since modern compilers tend to default to modern standards like gnu17, we might miss warnings about older standards, even though we expect Git to build with compilers that use them. So it's helpful for developer builds to set the -std argument to our lowest-common denominator. Traditionally this was c89, but since we're moving to assuming c99 in 7bc341e21b (git-compat-util: add a test balloon for C99 support, 2021-12-01) that seems like a good spot to land. And as explained in that commit, we want "gnu99" because we still want to take advantage of some extensions when they're available. The new argument kicks in only for clang and gcc (which we know to support "-std=" and "gnu" standards). And only for compiler versions which default to a newer standard. That will avoid accidentally silencing any build problems that non-developers would run into on older compilers that default to c89. My digging found that the default switched to gnu11 in gcc 5.1.0. Clang's documentation is less clear, but has done so since at least clang-7. So that's what I put in the conditional here. It's OK to err on the side of not-enabling this for older compilers. Most developers (as well as CI) are using much more recent versions, so any warnings will eventually surface. A concrete example is anonymous unions, which became legal in c11. Without this patch, "gcc -pedantic" will not complain about them, but will if we add in "-std=gnu99". Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-09 03:50:54 +08:00
ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang7,$(COMPILER_FEATURES))),)
DEVELOPER_CFLAGS += -std=gnu99
endif
Makefile: FreeBSD cannot do C99-or-below build In "make DEVELOPER=YesPlease" builds, we try to help developers to catch as many potential issues as they can by using -Wall and turning compilation warnings into errors. In the same spirit, we recently started adding -std=gnu99 to their CFLAGS, so that they can notice when they accidentally used language features beyond C99. It however turns out that FreeBSD 13.0 mistakenly uses C11 extension in its system header files regardless of what __STDC_VERSION__ says, which means that the platform (unless we tweak their system headers) cannot be used for this purpose. It seems that -std=gnu99 is only added conditionally even in today's config.mak.dev, so it is fine if we dropped -std=gnu99 from there. Which means that developers on FreeBSD cannot participate in vetting use of features beyond C99, but there are developers on other platforms who will, so it's not too bad. We might want a more "fundamental" fix to make the platform capable of taking -std=gnu99, like working around the use of unconditional C11 extension in its system header files by supplying a set of "replacement" definitions in our header files. We chose not to pursue such an approach for two reasons at this point: (1) The fix belongs to the FreeBSD project, not this project, and such an upstream fix may happen hopefully in a not-too-distant future. (2) Fixing such a bug in system header files and working it around can lead to unexpected breakages (other parts of their system header files may not be expecting to see and do not work well with our "replacement" definitions). This close to the final release of this cycle, we have no time for that. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-19 01:47:39 +08:00
else
# FreeBSD cannot limit to C99 because its system headers unconditionally
# rely on C11 features.
endif
config.mak.dev: specify -std=gnu99 for gcc/clang The point of DEVELOPER=1 is to turn up the warnings so we can catch portability or correctness mistakes at the compiler level. But since modern compilers tend to default to modern standards like gnu17, we might miss warnings about older standards, even though we expect Git to build with compilers that use them. So it's helpful for developer builds to set the -std argument to our lowest-common denominator. Traditionally this was c89, but since we're moving to assuming c99 in 7bc341e21b (git-compat-util: add a test balloon for C99 support, 2021-12-01) that seems like a good spot to land. And as explained in that commit, we want "gnu99" because we still want to take advantage of some extensions when they're available. The new argument kicks in only for clang and gcc (which we know to support "-std=" and "gnu" standards). And only for compiler versions which default to a newer standard. That will avoid accidentally silencing any build problems that non-developers would run into on older compilers that default to c89. My digging found that the default switched to gnu11 in gcc 5.1.0. Clang's documentation is less clear, but has done so since at least clang-7. So that's what I put in the conditional here. It's OK to err on the side of not-enabling this for older compilers. Most developers (as well as CI) are using much more recent versions, so any warnings will eventually surface. A concrete example is anonymous unions, which became legal in c11. Without this patch, "gcc -pedantic" will not complain about them, but will if we add in "-std=gnu99". Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-09 03:50:54 +08:00
Makefile: allow for combining DEVELOPER=1 and CFLAGS="..." Ever since the DEVELOPER=1 facility introduced there's been no way to have custom CFLAGS (e.g. CFLAGS="-O0 -g -ggdb3") while still benefiting from the set of warnings and assertions DEVELOPER=1 enables. This is because the semantics of variables in the Makefile are such that the user setting CFLAGS overrides anything we set, including what we're doing in config.mak.dev[1]. So let's introduce a "DEVELOPER_CFLAGS" variable in config.mak.dev and add it to ALL_CFLAGS. Before this the ALL_CFLAGS variable would (basically, there's some nuance we won't go into) be set to: $(CPPFLAGS) [$(CFLAGS) *or* $(CFLAGS) in config.mak.dev] $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) But will now be: $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) The reason for putting DEVELOPER_CFLAGS first is to allow for selectively overriding something DEVELOPER=1 brings in. On both GCC and Clang later settings override earlier ones. E.g. "-Wextra -Wno-extra" will enable no "extra" warnings, but not if those two arguments are reversed. Examples of things that weren't possible before, but are now: # Use -O0 instead of -O2 for less painful debuggng DEVELOPER=1 CFLAGS="-O0 -g" # DEVELOPER=1 plus -Wextra, but disable some of the warnings DEVELOPER=1 DEVOPTS="no-error extra-all" CFLAGS="-O0 -g -Wno-unused-parameter" The reason for the patches leading up to this one re-arranged the various *FLAGS assignments and includes is just for readability. The Makefile supports assignments out of order, e.g.: $ cat Makefile X = $(A) $(B) $(C) A = A B = B include c.mak all: @echo $(X) $ cat c.mak C=C $ make A B C So we could have gotten away with the much smaller change of changing "CFLAGS" in config.mak.dev to "DEVELOPER_CFLAGS" and adding that to ALL_CFLAGS earlier in the Makefile "before" the config.mak.* includes. But I think it's more readable to use variables "after" they're included. 1. https://www.gnu.org/software/make/manual/html_node/Overriding.html Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-22 22:41:27 +08:00
DEVELOPER_CFLAGS += -Wdeclaration-after-statement
DEVELOPER_CFLAGS += -Wformat-security
DEVELOPER_CFLAGS += -Wold-style-definition
DEVELOPER_CFLAGS += -Woverflow
DEVELOPER_CFLAGS += -Wpointer-arith
DEVELOPER_CFLAGS += -Wstrict-prototypes
DEVELOPER_CFLAGS += -Wunused
DEVELOPER_CFLAGS += -Wvla
config.mak.dev: build with -fno-common It's an easy mistake to define a variable in a header with "int x;" when you really meant to only declare the variable as "extern int x;" instead. Clang and gcc will both allow this when building with "-fcommon"; they put these "tentative definitions" in a common block which the linker is able to resolve. This is the default in clang and was the default in gcc until gcc-10, since it helps some legacy code. However, we would prefer not to rely on this because: - using "extern" makes the intent more clear (so it's a style issue, but it's one the compiler can help us catch) - according to the gcc manpage, it may yield a speed and code size penalty So let's build explicitly with -fno-common when the DEVELOPER knob is set, which will let developers using clang and older versions of gcc notice these problems. I didn't bother making this conditional on a particular version of gcc. As far as I know, this option has been available forever in both gcc and clang, so old versions don't need to avoid it. And we already expect gcc and clang options throughout config.mak.dev, so it's unlikely anybody setting the DEVELOPER knob is using anything else. It's a noop on gcc-10, of course, but it's not worth trying to exclude it there. Note that there's nothing to fix in the code; we already don't have any issues here. But if you want to test the patch, you can add a bare "int x;" into cache.h, which will cause the link step to fail. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-16 03:30:29 +08:00
DEVELOPER_CFLAGS += -fno-common
ifneq ($(filter clang4,$(COMPILER_FEATURES)),)
Makefile: allow for combining DEVELOPER=1 and CFLAGS="..." Ever since the DEVELOPER=1 facility introduced there's been no way to have custom CFLAGS (e.g. CFLAGS="-O0 -g -ggdb3") while still benefiting from the set of warnings and assertions DEVELOPER=1 enables. This is because the semantics of variables in the Makefile are such that the user setting CFLAGS overrides anything we set, including what we're doing in config.mak.dev[1]. So let's introduce a "DEVELOPER_CFLAGS" variable in config.mak.dev and add it to ALL_CFLAGS. Before this the ALL_CFLAGS variable would (basically, there's some nuance we won't go into) be set to: $(CPPFLAGS) [$(CFLAGS) *or* $(CFLAGS) in config.mak.dev] $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) But will now be: $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) The reason for putting DEVELOPER_CFLAGS first is to allow for selectively overriding something DEVELOPER=1 brings in. On both GCC and Clang later settings override earlier ones. E.g. "-Wextra -Wno-extra" will enable no "extra" warnings, but not if those two arguments are reversed. Examples of things that weren't possible before, but are now: # Use -O0 instead of -O2 for less painful debuggng DEVELOPER=1 CFLAGS="-O0 -g" # DEVELOPER=1 plus -Wextra, but disable some of the warnings DEVELOPER=1 DEVOPTS="no-error extra-all" CFLAGS="-O0 -g -Wno-unused-parameter" The reason for the patches leading up to this one re-arranged the various *FLAGS assignments and includes is just for readability. The Makefile supports assignments out of order, e.g.: $ cat Makefile X = $(A) $(B) $(C) A = A B = B include c.mak all: @echo $(X) $ cat c.mak C=C $ make A B C So we could have gotten away with the much smaller change of changing "CFLAGS" in config.mak.dev to "DEVELOPER_CFLAGS" and adding that to ALL_CFLAGS earlier in the Makefile "before" the config.mak.* includes. But I think it's more readable to use variables "after" they're included. 1. https://www.gnu.org/software/make/manual/html_node/Overriding.html Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-22 22:41:27 +08:00
DEVELOPER_CFLAGS += -Wtautological-constant-out-of-range-compare
endif
ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),)
Makefile: allow for combining DEVELOPER=1 and CFLAGS="..." Ever since the DEVELOPER=1 facility introduced there's been no way to have custom CFLAGS (e.g. CFLAGS="-O0 -g -ggdb3") while still benefiting from the set of warnings and assertions DEVELOPER=1 enables. This is because the semantics of variables in the Makefile are such that the user setting CFLAGS overrides anything we set, including what we're doing in config.mak.dev[1]. So let's introduce a "DEVELOPER_CFLAGS" variable in config.mak.dev and add it to ALL_CFLAGS. Before this the ALL_CFLAGS variable would (basically, there's some nuance we won't go into) be set to: $(CPPFLAGS) [$(CFLAGS) *or* $(CFLAGS) in config.mak.dev] $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) But will now be: $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) The reason for putting DEVELOPER_CFLAGS first is to allow for selectively overriding something DEVELOPER=1 brings in. On both GCC and Clang later settings override earlier ones. E.g. "-Wextra -Wno-extra" will enable no "extra" warnings, but not if those two arguments are reversed. Examples of things that weren't possible before, but are now: # Use -O0 instead of -O2 for less painful debuggng DEVELOPER=1 CFLAGS="-O0 -g" # DEVELOPER=1 plus -Wextra, but disable some of the warnings DEVELOPER=1 DEVOPTS="no-error extra-all" CFLAGS="-O0 -g -Wno-unused-parameter" The reason for the patches leading up to this one re-arranged the various *FLAGS assignments and includes is just for readability. The Makefile supports assignments out of order, e.g.: $ cat Makefile X = $(A) $(B) $(C) A = A B = B include c.mak all: @echo $(X) $ cat c.mak C=C $ make A B C So we could have gotten away with the much smaller change of changing "CFLAGS" in config.mak.dev to "DEVELOPER_CFLAGS" and adding that to ALL_CFLAGS earlier in the Makefile "before" the config.mak.* includes. But I think it's more readable to use variables "after" they're included. 1. https://www.gnu.org/software/make/manual/html_node/Overriding.html Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-22 22:41:27 +08:00
DEVELOPER_CFLAGS += -Wextra
# if a function is public, there should be a prototype and the right
# header file should be included. If not, it should be static.
Makefile: allow for combining DEVELOPER=1 and CFLAGS="..." Ever since the DEVELOPER=1 facility introduced there's been no way to have custom CFLAGS (e.g. CFLAGS="-O0 -g -ggdb3") while still benefiting from the set of warnings and assertions DEVELOPER=1 enables. This is because the semantics of variables in the Makefile are such that the user setting CFLAGS overrides anything we set, including what we're doing in config.mak.dev[1]. So let's introduce a "DEVELOPER_CFLAGS" variable in config.mak.dev and add it to ALL_CFLAGS. Before this the ALL_CFLAGS variable would (basically, there's some nuance we won't go into) be set to: $(CPPFLAGS) [$(CFLAGS) *or* $(CFLAGS) in config.mak.dev] $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) But will now be: $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) The reason for putting DEVELOPER_CFLAGS first is to allow for selectively overriding something DEVELOPER=1 brings in. On both GCC and Clang later settings override earlier ones. E.g. "-Wextra -Wno-extra" will enable no "extra" warnings, but not if those two arguments are reversed. Examples of things that weren't possible before, but are now: # Use -O0 instead of -O2 for less painful debuggng DEVELOPER=1 CFLAGS="-O0 -g" # DEVELOPER=1 plus -Wextra, but disable some of the warnings DEVELOPER=1 DEVOPTS="no-error extra-all" CFLAGS="-O0 -g -Wno-unused-parameter" The reason for the patches leading up to this one re-arranged the various *FLAGS assignments and includes is just for readability. The Makefile supports assignments out of order, e.g.: $ cat Makefile X = $(A) $(B) $(C) A = A B = B include c.mak all: @echo $(X) $ cat c.mak C=C $ make A B C So we could have gotten away with the much smaller change of changing "CFLAGS" in config.mak.dev to "DEVELOPER_CFLAGS" and adding that to ALL_CFLAGS earlier in the Makefile "before" the config.mak.* includes. But I think it's more readable to use variables "after" they're included. 1. https://www.gnu.org/software/make/manual/html_node/Overriding.html Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-22 22:41:27 +08:00
DEVELOPER_CFLAGS += -Wmissing-prototypes
ifeq ($(filter extra-all,$(DEVOPTS)),)
# These are disabled because we have these all over the place.
Makefile: allow for combining DEVELOPER=1 and CFLAGS="..." Ever since the DEVELOPER=1 facility introduced there's been no way to have custom CFLAGS (e.g. CFLAGS="-O0 -g -ggdb3") while still benefiting from the set of warnings and assertions DEVELOPER=1 enables. This is because the semantics of variables in the Makefile are such that the user setting CFLAGS overrides anything we set, including what we're doing in config.mak.dev[1]. So let's introduce a "DEVELOPER_CFLAGS" variable in config.mak.dev and add it to ALL_CFLAGS. Before this the ALL_CFLAGS variable would (basically, there's some nuance we won't go into) be set to: $(CPPFLAGS) [$(CFLAGS) *or* $(CFLAGS) in config.mak.dev] $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) But will now be: $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) The reason for putting DEVELOPER_CFLAGS first is to allow for selectively overriding something DEVELOPER=1 brings in. On both GCC and Clang later settings override earlier ones. E.g. "-Wextra -Wno-extra" will enable no "extra" warnings, but not if those two arguments are reversed. Examples of things that weren't possible before, but are now: # Use -O0 instead of -O2 for less painful debuggng DEVELOPER=1 CFLAGS="-O0 -g" # DEVELOPER=1 plus -Wextra, but disable some of the warnings DEVELOPER=1 DEVOPTS="no-error extra-all" CFLAGS="-O0 -g -Wno-unused-parameter" The reason for the patches leading up to this one re-arranged the various *FLAGS assignments and includes is just for readability. The Makefile supports assignments out of order, e.g.: $ cat Makefile X = $(A) $(B) $(C) A = A B = B include c.mak all: @echo $(X) $ cat c.mak C=C $ make A B C So we could have gotten away with the much smaller change of changing "CFLAGS" in config.mak.dev to "DEVELOPER_CFLAGS" and adding that to ALL_CFLAGS earlier in the Makefile "before" the config.mak.* includes. But I think it's more readable to use variables "after" they're included. 1. https://www.gnu.org/software/make/manual/html_node/Overriding.html Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-22 22:41:27 +08:00
DEVELOPER_CFLAGS += -Wno-empty-body
DEVELOPER_CFLAGS += -Wno-missing-field-initializers
DEVELOPER_CFLAGS += -Wno-sign-compare
DEVELOPER_CFLAGS += -Wno-unused-parameter
endif
endif
# uninitialized warnings on gcc 4.9.2 in xdiff/xdiffi.c and config.c
# not worth fixing since newer compilers correctly stop complaining
#
# Likewise, gcc older than 4.9 complains about initializing a
# struct-within-a-struct using just "{ 0 }"
ifneq ($(filter gcc4,$(COMPILER_FEATURES)),)
ifeq ($(filter gcc5,$(COMPILER_FEATURES)),)
Makefile: allow for combining DEVELOPER=1 and CFLAGS="..." Ever since the DEVELOPER=1 facility introduced there's been no way to have custom CFLAGS (e.g. CFLAGS="-O0 -g -ggdb3") while still benefiting from the set of warnings and assertions DEVELOPER=1 enables. This is because the semantics of variables in the Makefile are such that the user setting CFLAGS overrides anything we set, including what we're doing in config.mak.dev[1]. So let's introduce a "DEVELOPER_CFLAGS" variable in config.mak.dev and add it to ALL_CFLAGS. Before this the ALL_CFLAGS variable would (basically, there's some nuance we won't go into) be set to: $(CPPFLAGS) [$(CFLAGS) *or* $(CFLAGS) in config.mak.dev] $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) But will now be: $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS) $(BASIC_CFLAGS) $(EXTRA_CPPFLAGS) The reason for putting DEVELOPER_CFLAGS first is to allow for selectively overriding something DEVELOPER=1 brings in. On both GCC and Clang later settings override earlier ones. E.g. "-Wextra -Wno-extra" will enable no "extra" warnings, but not if those two arguments are reversed. Examples of things that weren't possible before, but are now: # Use -O0 instead of -O2 for less painful debuggng DEVELOPER=1 CFLAGS="-O0 -g" # DEVELOPER=1 plus -Wextra, but disable some of the warnings DEVELOPER=1 DEVOPTS="no-error extra-all" CFLAGS="-O0 -g -Wno-unused-parameter" The reason for the patches leading up to this one re-arranged the various *FLAGS assignments and includes is just for readability. The Makefile supports assignments out of order, e.g.: $ cat Makefile X = $(A) $(B) $(C) A = A B = B include c.mak all: @echo $(X) $ cat c.mak C=C $ make A B C So we could have gotten away with the much smaller change of changing "CFLAGS" in config.mak.dev to "DEVELOPER_CFLAGS" and adding that to ALL_CFLAGS earlier in the Makefile "before" the config.mak.* includes. But I think it's more readable to use variables "after" they're included. 1. https://www.gnu.org/software/make/manual/html_node/Overriding.html Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-22 22:41:27 +08:00
DEVELOPER_CFLAGS += -Wno-uninitialized
DEVELOPER_CFLAGS += -Wno-missing-braces
endif
endif
perl: check for perl warnings while running tests We set "use warnings" in most of our perl code to catch problems. But as the name implies, warnings just emit a message to stderr and don't otherwise affect the program. So our tests are quite likely to miss that warnings are being spewed, as most of them do not look at stderr. We could ask perl to make all warnings fatal, but this is likely annoying for non-developers, who would rather have a running program with a warning than something that refuses to work at all. So instead, let's teach the perl code to respect an environment variable (GIT_PERL_FATAL_WARNINGS) to increase the severity of the warnings. This can be set for day-to-day running if people want to be really pedantic, but the primary use is to trigger it within the test suite. We could also trigger that for every test run, but likewise even the tests failing may be annoying to distro builders, etc (just as -Werror would be for compiling C code). So we'll tie it to a special test-mode variable (GIT_TEST_PERL_FATAL_WARNINGS) that can be set in the environment or as a Makefile knob, and we'll automatically turn the knob when DEVELOPER=1 is set. That should give developers and CI the more careful view without disrupting normal users or packagers. Note that the mapping from the GIT_TEST_* form to the GIT_* form in test-lib.sh is necessary even if they had the same name: the perl scripts need it to be normalized to a perl truth value, and we also have to make sure it's exported (we might have gotten it from the environment, but we might also have gotten it from GIT-BUILD-OPTIONS directly). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-22 11:24:00 +08:00
# Old versions of clang complain about initializaing a
# struct-within-a-struct using just "{0}" rather than "{{0}}". This
# error is considered a false-positive and not worth fixing, because
# new clang versions do not, so just disable it.
#
# The "bug" was fixed in upstream clang 9.
#
# Complicating this is that versions of clang released by Apple have
# their own version numbers (associated with the corresponding version
# of XCode) unrelated to the official clang version numbers.
#
# The bug was fixed in Apple clang 12.
#
ifneq ($(filter clang1,$(COMPILER_FEATURES)),) # if we are using clang
ifeq ($(uname_S),Darwin) # if we are on darwin
ifeq ($(filter clang12,$(COMPILER_FEATURES)),) # if version < 12
DEVELOPER_CFLAGS += -Wno-missing-braces
endif
else # not darwin
ifeq ($(filter clang9,$(COMPILER_FEATURES)),) # if version < 9
DEVELOPER_CFLAGS += -Wno-missing-braces
endif
endif
endif
# https://bugzilla.redhat.com/show_bug.cgi?id=2075786
ifneq ($(filter gcc12,$(COMPILER_FEATURES)),)
DEVELOPER_CFLAGS += -Wno-error=stringop-overread
endif
perl: check for perl warnings while running tests We set "use warnings" in most of our perl code to catch problems. But as the name implies, warnings just emit a message to stderr and don't otherwise affect the program. So our tests are quite likely to miss that warnings are being spewed, as most of them do not look at stderr. We could ask perl to make all warnings fatal, but this is likely annoying for non-developers, who would rather have a running program with a warning than something that refuses to work at all. So instead, let's teach the perl code to respect an environment variable (GIT_PERL_FATAL_WARNINGS) to increase the severity of the warnings. This can be set for day-to-day running if people want to be really pedantic, but the primary use is to trigger it within the test suite. We could also trigger that for every test run, but likewise even the tests failing may be annoying to distro builders, etc (just as -Werror would be for compiling C code). So we'll tie it to a special test-mode variable (GIT_TEST_PERL_FATAL_WARNINGS) that can be set in the environment or as a Makefile knob, and we'll automatically turn the knob when DEVELOPER=1 is set. That should give developers and CI the more careful view without disrupting normal users or packagers. Note that the mapping from the GIT_TEST_* form to the GIT_* form in test-lib.sh is necessary even if they had the same name: the perl scripts need it to be normalized to a perl truth value, and we also have to make sure it's exported (we might have gotten it from the environment, but we might also have gotten it from GIT-BUILD-OPTIONS directly). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-22 11:24:00 +08:00
GIT_TEST_PERL_FATAL_WARNINGS = YesPlease