kernel/git - git - PowerEL Git System

Author	SHA1	Message	Date
Jeff King	7329d94be7	config.mak.dev: re-enable -Wformat-zero-length We recently triggered some -Wformat-zero-length warnings in the code, but no developers noticed because we suppress that warning in builds with the DEVELOPER=1 Makefile knob set. But we _don't_ suppress them in a non-developer build (and they're part of -Wall). So even though non-developers probably aren't using -Werror, they see the annoying warnings when they build. We've had back and forth discussion over the years on whether this warning is useful or not. In most cases we've seen, it's not true that the call is a mistake, since we're using its side effects (like adding a newline status_printf_ln()) or writing an empty string to a destination which is handled by the function (as in write_file()). And so we end up working around it in the source by passing ("%s", ""). There's more discussion in the subthread starting at: https://lore.kernel.org/git/xmqqtwaod7ly.fsf@gitster.mtv.corp.google.com/ The short of it is that we probably can't just disable the warning for everybody because of portability issues. And ignoring it for developers puts us in the situation we're in now, where non-dev builds are annoyed. Since the workaround is both rarely needed and fairly straight-forward, let's just commit to doing it as necessary, and re-enable the warning. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
brian m. carlson	9412759925	repository: require a build flag to use SHA-256 At this point, SHA-256 support is experimental and some behavior may change. To avoid surprising unsuspecting users, require a build flag, ENABLE_SHA256, to allow use of a non-SHA-1 algorithm. Enable this flag by default when the DEVELOPER make flag is set so that contributors can test this case adequately. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	5 years ago
Ævar Arnfjörð Bjarmason	6d5d4b4e93	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>	6 years ago
Thomas Gummerer	6163f3f1a4	config.mak.dev: add -Wall, primarily for -Wformat, to help autoconf users `801fa63a90` ("config.mak.dev: add -Wformat-security", 2018-09-08) added the "-Wformat-security" to the flags set in config.mak.dev. In the gcc man page this is documented as: If -Wformat is specified, also warn about uses of format functions that represent possible security problems. [...] The commit did however not add the "-Wformat" flag, but instead relied on the fact that "-Wall" is set in the Makefile by default and that "-Wformat" is part of "-Wall". Unfortunately, those who use config.mak.autogen generated with the autoconf to configure toolchain do not get "-Wall" in their CFLAGS and the added -Wformat-security had no effect. Worse yet, newer versions of gcc (gcc 8.2.1 in this particular case) warn about the lack of "-Wformat" and thus compilation fails only with this option set. We could fix it by adding "-Wformat", but in general we do want all checks included in "-Wall", so let's add it to config.mak.dev to cover more cases. Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com> Helped-by: Jeff King <peff@peff.net> Helped-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Jeff King	36da893114	config.mak.dev: enable -Wunused-function We explicitly omitted -Wunused-function when we added -Wextra, but there is no need: the current code compiles cleanly with it. And it's worth having, since it can let you know when there are cascading effects from a cleanup (e.g., deleting one function lets you delete its static helpers). There are cases where we may need an unused function to exist, but we can handle these easily: - macro-generated code like commit-slab; there we have the MAYBE_UNUSED annotation to silence the compiler - conditional compilation, where we may or may not need a static helper. These generally fall into one of two categories: - the call should not be conditional, but rather the function body itself should be (and may just be a no-op on one side of the #if). That keeps the conditional pollution out of the main code. - call-chains of static helpers should all be in the same #if block, so they are all-or-nothing And if there's some case that doesn't cover, we still have MAYBE_UNUSED as a fallback. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Jeff King	801fa63a90	config.mak.dev: add -Wformat-security We currently build cleanly with -Wformat-security, and it's a good idea to make sure we continue to do so (since calls that trigger the warning may be security vulnerabilities). Note that we cannot use the stronger -Wformat-nonliteral, as there are case where we are clever with passing around pointers to string literals. E.g., bisect_rev_setup() takes bad_format and good_format parameters. These ultimately come from literals, but they still trigger the warning. Some of these might be fixable (e.g., by passing flags from which we locally select a format), and might even be worth fixing (not because of security, but just because it's an easy mistake to pass the wrong format). But there are other cases which are likely quite hard to fix (we actually generate formats in a local buffer in some cases). So let's punt on that for now and start with -Wformat-security, which is supposed to catch the most important cases. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Beat Bolli	729b3925ed	Makefile: add a DEVOPTS flag to get pedantic compilation In the interest of code hygiene, make it easier to compile Git with the flag -pedantic. Pure pedantic compilation with GCC 7.3 results in one warning per use of the translation macro `N_`: warning: array initialized from parenthesized string constant [-Wpedantic] Therefore also disable the parenthesising of i18n strings with -DUSE_PARENS_AROUND_GETTEXT_N=0. Signed-off-by: Beat Bolli <dev+git@drbeat.li> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ævar Arnfjörð Bjarmason	26d2e4fb22	Makefile: add a DEVOPTS to get all of -Wextra Change DEVOPTS to understand a "extra-all" option. When the DEVELOPER flag is enabled we turn on -Wextra, but manually switch some of the warnings it turns on off. This is because we have many existing occurrences of them in the code base. This mode will stop the suppression, let the developer see and decide whether to fix them. This change is a slight alteration of Nguyễn Thái Ngọc Duy EAGER_DEVELOPER mode patch[1] 1. "[PATCH v3 3/3] Makefile: add EAGER_DEVELOPER mode" (<20180329150322.10722-4-pclouds@gmail.com>; https://public-inbox.org/git/20180329150322.10722-4-pclouds@gmail.com/) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ævar Arnfjörð Bjarmason	99f763baf5	Makefile: add a DEVOPTS to suppress -Werror under DEVELOPER Add a DEVOPTS variable that'll be used to tweak the behavior of DEVELOPER. I've long wanted to use DEVELOPER=1 in my production builds, but on some old systems I still get warnings, and thus the build would fail. However if the build/tests fail for some other reason, it would still be useful to scroll up and see what the relevant code is warning about. This change allows for that. Now setting DEVELOPER will set -Werror as before, but if DEVOPTS=no-error is provided is set you'll get the same warnings, but without -Werror. Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Nguyễn Thái Ngọc Duy	1da1580e4c	Makefile: detect compiler and enable more warnings in DEVELOPER=1 The set of extra warnings we enable when DEVELOPER has to be conservative because we can't assume any compiler version the developer may use. Detect the compiler version so we know when it's safe to enable -Wextra and maybe more. These warning settings are mostly from my custom config.mak a long time ago when I tried to enable as many warnings as possible that can still build without showing warnings. Some of those warnings are probably worth fixing instead of just suppressing in future. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago