* [PATCH] travis-ci: enable more warnings on travis linux-gcc job @ 2018-03-03 1:46 Nguyễn Thái Ngọc Duy 2018-03-16 19:33 ` [PATCH v2] " Nguyễn Thái Ngọc Duy 0 siblings, 1 reply; 47+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2018-03-03 1:46 UTC (permalink / raw) To: git; +Cc: Nguyễn Thái Ngọc Duy We have DEVELOPER config to enable more warnings, but since we can't set a fixed gcc version to all devs, that has to be a bit more conservative. On travis, we know almost exact version to be used (bumped to 6.x for more warnings), we could be more aggressive. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- .travis.yml | 3 +++ ci/run-build.sh | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/.travis.yml b/.travis.yml index 4684b3f4f3..273b1d508a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,10 +16,13 @@ compiler: addons: apt: + sources: + - ubuntu-toolchain-r-test packages: - language-pack-is - git-svn - apache2 + - gcc-6 matrix: include: diff --git a/ci/run-build.sh b/ci/run-build.sh index 4f940d1032..04e163359c 100755 --- a/ci/run-build.sh +++ b/ci/run-build.sh @@ -5,4 +5,19 @@ . ${0%/*}/lib-travisci.sh +if [ "$jobname" = linux-gcc ]; then + gcc-6 --version + cat >config.mak <<-EOF + CC=gcc-6 + CFLAGS = -g -O2 -Wall + CFLAGS += -Wextra + CFLAGS += -Wmissing-prototypes + CFLAGS += -Wno-empty-body + CFLAGS += -Wno-maybe-uninitialized + CFLAGS += -Wno-missing-field-initializers + CFLAGS += -Wno-sign-compare + CFLAGS += -Wno-unused-function + CFLAGS += -Wno-unused-parameter + EOF +fi make --jobs=2 -- 2.16.1.435.g8f24da2e1a ^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v2] travis-ci: enable more warnings on travis linux-gcc job 2018-03-03 1:46 [PATCH] travis-ci: enable more warnings on travis linux-gcc job Nguyễn Thái Ngọc Duy @ 2018-03-16 19:33 ` Nguyễn Thái Ngọc Duy 2018-03-16 21:22 ` Jeff King 0 siblings, 1 reply; 47+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2018-03-16 19:33 UTC (permalink / raw) To: git; +Cc: Nguyễn Thái Ngọc Duy We have DEVELOPER config to enable more warnings, but since we can't set a fixed gcc version to all devs, that has to be a bit more conservative. On travis, we know almost exact version to be used (bumped to 6.x for more warnings), we could be more aggressive. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- I still think it's a good thing to do. So here's the rebased version (v1 collides badly with another series, which has landed) .travis.yml | 3 +++ ci/run-build-and-tests.sh | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/.travis.yml b/.travis.yml index 5f5ee4f3bd..0b3c50f5e7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,10 +16,13 @@ compiler: addons: apt: + sources: + - ubuntu-toolchain-r-test packages: - language-pack-is - git-svn - apache2 + - gcc-6 matrix: include: diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh index 3735ce413f..f6f346c468 100755 --- a/ci/run-build-and-tests.sh +++ b/ci/run-build-and-tests.sh @@ -7,6 +7,22 @@ ln -s "$cache_dir/.prove" t/.prove +if [ "$jobname" = linux-gcc ]; then + gcc-6 --version + cat >config.mak <<-EOF + CC=gcc-6 + CFLAGS = -g -O2 -Wall + CFLAGS += -Wextra + CFLAGS += -Wmissing-prototypes + CFLAGS += -Wno-empty-body + CFLAGS += -Wno-maybe-uninitialized + CFLAGS += -Wno-missing-field-initializers + CFLAGS += -Wno-sign-compare + CFLAGS += -Wno-unused-function + CFLAGS += -Wno-unused-parameter + EOF +fi + make --jobs=2 make --quiet test if test "$jobname" = "linux-gcc" -- 2.16.2.903.gd04caf5039 ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v2] travis-ci: enable more warnings on travis linux-gcc job 2018-03-16 19:33 ` [PATCH v2] " Nguyễn Thái Ngọc Duy @ 2018-03-16 21:22 ` Jeff King 2018-03-17 8:01 ` Duy Nguyen 0 siblings, 1 reply; 47+ messages in thread From: Jeff King @ 2018-03-16 21:22 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy; +Cc: git On Fri, Mar 16, 2018 at 08:33:55PM +0100, Nguyễn Thái Ngọc Duy wrote: > We have DEVELOPER config to enable more warnings, but since we can't set > a fixed gcc version to all devs, that has to be a bit more conservative. > On travis, we know almost exact version to be used (bumped to 6.x for > more warnings), we could be more aggressive. Certainly it makes sense to dial up any static checks we can for the Travis environment, but... > diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh > index 3735ce413f..f6f346c468 100755 > --- a/ci/run-build-and-tests.sh > +++ b/ci/run-build-and-tests.sh > @@ -7,6 +7,22 @@ > > ln -s "$cache_dir/.prove" t/.prove > > +if [ "$jobname" = linux-gcc ]; then > + gcc-6 --version > + cat >config.mak <<-EOF > + CC=gcc-6 > + CFLAGS = -g -O2 -Wall > + CFLAGS += -Wextra > + CFLAGS += -Wmissing-prototypes > + CFLAGS += -Wno-empty-body > + CFLAGS += -Wno-maybe-uninitialized > + CFLAGS += -Wno-missing-field-initializers > + CFLAGS += -Wno-sign-compare > + CFLAGS += -Wno-unused-function > + CFLAGS += -Wno-unused-parameter > + EOF > +fi Why isn't this just turning on DEVELOPER=1 if we know we have a capable compiler? -Peff ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2] travis-ci: enable more warnings on travis linux-gcc job 2018-03-16 21:22 ` Jeff King @ 2018-03-17 8:01 ` Duy Nguyen 2018-03-17 14:29 ` Lars Schneider 0 siblings, 1 reply; 47+ messages in thread From: Duy Nguyen @ 2018-03-17 8:01 UTC (permalink / raw) To: Jeff King; +Cc: Git Mailing List On Fri, Mar 16, 2018 at 10:22 PM, Jeff King <peff@peff.net> wrote: >> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh >> index 3735ce413f..f6f346c468 100755 >> --- a/ci/run-build-and-tests.sh >> +++ b/ci/run-build-and-tests.sh >> @@ -7,6 +7,22 @@ >> >> ln -s "$cache_dir/.prove" t/.prove >> >> +if [ "$jobname" = linux-gcc ]; then >> + gcc-6 --version >> + cat >config.mak <<-EOF >> + CC=gcc-6 >> + CFLAGS = -g -O2 -Wall >> + CFLAGS += -Wextra >> + CFLAGS += -Wmissing-prototypes >> + CFLAGS += -Wno-empty-body >> + CFLAGS += -Wno-maybe-uninitialized >> + CFLAGS += -Wno-missing-field-initializers >> + CFLAGS += -Wno-sign-compare >> + CFLAGS += -Wno-unused-function >> + CFLAGS += -Wno-unused-parameter >> + EOF >> +fi > > Why isn't this just turning on DEVELOPER=1 if we know we have a capable > compiler? DEVELOPER=1 is always set even before this patch. It's set and exported in lib-travisci.sh. -- Duy ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2] travis-ci: enable more warnings on travis linux-gcc job 2018-03-17 8:01 ` Duy Nguyen @ 2018-03-17 14:29 ` Lars Schneider 2018-03-17 14:59 ` Duy Nguyen 2018-03-17 15:16 ` [PATCH v2] travis-ci: enable more warnings on travis linux-gcc job Jeff King 0 siblings, 2 replies; 47+ messages in thread From: Lars Schneider @ 2018-03-17 14:29 UTC (permalink / raw) To: Duy Nguyen; +Cc: Jeff King, Git Mailing List > On 17 Mar 2018, at 09:01, Duy Nguyen <pclouds@gmail.com> wrote: > > On Fri, Mar 16, 2018 at 10:22 PM, Jeff King <peff@peff.net> wrote: >>> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh >>> index 3735ce413f..f6f346c468 100755 >>> --- a/ci/run-build-and-tests.sh >>> +++ b/ci/run-build-and-tests.sh >>> @@ -7,6 +7,22 @@ >>> >>> ln -s "$cache_dir/.prove" t/.prove >>> >>> +if [ "$jobname" = linux-gcc ]; then >>> + gcc-6 --version >>> + cat >config.mak <<-EOF >>> + CC=gcc-6 >>> + CFLAGS = -g -O2 -Wall >>> + CFLAGS += -Wextra >>> + CFLAGS += -Wmissing-prototypes >>> + CFLAGS += -Wno-empty-body >>> + CFLAGS += -Wno-maybe-uninitialized >>> + CFLAGS += -Wno-missing-field-initializers >>> + CFLAGS += -Wno-sign-compare >>> + CFLAGS += -Wno-unused-function >>> + CFLAGS += -Wno-unused-parameter >>> + EOF >>> +fi >> >> Why isn't this just turning on DEVELOPER=1 if we know we have a capable >> compiler? > > DEVELOPER=1 is always set even before this patch. It's set and > exported in lib-travisci.sh. I interpreted Peff's comment like this: If DEVELOPER=1 is set and we detect a gcc-6 in the makefile, then we could set your additional flags in the makefile. This way every developer with a new compiler would run these flags locally (if DEVELOPER=1 is set). - Lars ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2] travis-ci: enable more warnings on travis linux-gcc job 2018-03-17 14:29 ` Lars Schneider @ 2018-03-17 14:59 ` Duy Nguyen 2018-03-17 16:08 ` Jeff King 2018-03-17 15:16 ` [PATCH v2] travis-ci: enable more warnings on travis linux-gcc job Jeff King 1 sibling, 1 reply; 47+ messages in thread From: Duy Nguyen @ 2018-03-17 14:59 UTC (permalink / raw) To: Lars Schneider; +Cc: Jeff King, Git Mailing List On Sat, Mar 17, 2018 at 03:29:31PM +0100, Lars Schneider wrote: > I interpreted Peff's comment like this: > > If DEVELOPER=1 is set and we detect a gcc-6 in the makefile, > then we could set your additional flags in the makefile. > > This way every developer with a new compiler would run these > flags locally (if DEVELOPER=1 is set). Aha. Something like this? I split developer cflags out to a separate file because I imagine people may start to add gcc7, clang.... -- 8< -- diff --git a/.travis.yml b/.travis.yml index 5f5ee4f3bd..0b3c50f5e7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,10 +16,13 @@ compiler: addons: apt: + sources: + - ubuntu-toolchain-r-test packages: - language-pack-is - git-svn - apache2 + - gcc-6 matrix: include: diff --git a/Makefile b/Makefile index de4b8f0c02..e8321e44d6 100644 --- a/Makefile +++ b/Makefile @@ -436,15 +436,6 @@ GIT-VERSION-FILE: FORCE # CFLAGS and LDFLAGS are for the users to override from the command line. CFLAGS = -g -O2 -Wall -DEVELOPER_CFLAGS = -Werror \ - -Wdeclaration-after-statement \ - -Wno-format-zero-length \ - -Wold-style-definition \ - -Woverflow \ - -Wpointer-arith \ - -Wstrict-prototypes \ - -Wunused \ - -Wvla LDFLAGS = ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) ALL_LDFLAGS = $(LDFLAGS) @@ -1045,7 +1036,8 @@ include config.mak.uname -include config.mak ifdef DEVELOPER -CFLAGS += $(DEVELOPER_CFLAGS) +include config.mak.dev +CFLAGS += $(DEV_CFLAGS) endif comma := , diff --git a/config.mak.dev b/config.mak.dev index e69de29bb2..644d0d581f 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -0,0 +1,23 @@ +DEV_CFLAGS = +DEV_CFLAGS += -Werror +DEV_CFLAGS += -Wdeclaration-after-statement +DEV_CFLAGS += -Wno-format-zero-length +DEV_CFLAGS += -Wold-style-definition +DEV_CFLAGS += -Woverflow +DEV_CFLAGS += -Wpointer-arith +DEV_CFLAGS += -Wstrict-prototypes +DEV_CFLAGS += -Wunused +DEV_CFLAGS += -Wvla + +GCC_VER := $(shell sh -c '$(CC) --version | grep ^gcc >/dev/null && $(CC) -dumpversion | cut -f 1 -d .') + +ifeq ($(GCC_VER),6) +DEV_CFLAGS += -Wextra +DEV_CFLAGS += -Wmissing-prototypes +DEV_CFLAGS += -Wno-empty-body +DEV_CFLAGS += -Wno-maybe-uninitialized +DEV_CFLAGS += -Wno-missing-field-initializers +DEV_CFLAGS += -Wno-sign-compare +DEV_CFLAGS += -Wno-unused-function +DEV_CFLAGS += -Wno-unused-parameter +endif -- 8< -- ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v2] travis-ci: enable more warnings on travis linux-gcc job 2018-03-17 14:59 ` Duy Nguyen @ 2018-03-17 16:08 ` Jeff King 2018-03-17 16:12 ` Jeff King ` (2 more replies) 0 siblings, 3 replies; 47+ messages in thread From: Jeff King @ 2018-03-17 16:08 UTC (permalink / raw) To: Duy Nguyen; +Cc: Lars Schneider, Git Mailing List On Sat, Mar 17, 2018 at 03:59:23PM +0100, Duy Nguyen wrote: > On Sat, Mar 17, 2018 at 03:29:31PM +0100, Lars Schneider wrote: > > I interpreted Peff's comment like this: > > > > If DEVELOPER=1 is set and we detect a gcc-6 in the makefile, > > then we could set your additional flags in the makefile. > > > > This way every developer with a new compiler would run these > > flags locally (if DEVELOPER=1 is set). > > Aha. Something like this? I split developer cflags out to a separate > file because I imagine people may start to add gcc7, clang.... Yeah, gcc 7 is already standard on debian unstable, so... > +GCC_VER := $(shell sh -c '$(CC) --version | grep ^gcc >/dev/null && $(CC) -dumpversion | cut -f 1 -d .') > + > +ifeq ($(GCC_VER),6) ...this probably needs to be "greater than or equal to". Unfortunately I think that's hard to do in pure make. But we're already relying on $(shell) here, so we could just move the logic there. Something like the patch below, perhaps. It should do the right thing on clang and gcc, and I added in an extra clang-only warning I've found useful. Otherwise the list of flags comes from your patch. The "clang4" limit is just what I happened to have on my system to test with. Some of those flags might work for clang3, and some of the gcc6-only flags might work on newer versions of clang. I was puzzled by -Wno-maybe-uninitialized, though. I think that has found bugs for us before (but also false positives, but we usually squelch those in the code). And it's part of -Wall, not -Wextra, so we shouldn't need it as part of this patch, should we? diff --git a/Makefile b/Makefile index a1d8775adb..9dfd152a1e 100644 --- a/Makefile +++ b/Makefile @@ -442,15 +442,6 @@ GIT-VERSION-FILE: FORCE # CFLAGS and LDFLAGS are for the users to override from the command line. CFLAGS = -g -O2 -Wall -DEVELOPER_CFLAGS = -Werror \ - -Wdeclaration-after-statement \ - -Wno-format-zero-length \ - -Wold-style-definition \ - -Woverflow \ - -Wpointer-arith \ - -Wstrict-prototypes \ - -Wunused \ - -Wvla LDFLAGS = ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) ALL_LDFLAGS = $(LDFLAGS) @@ -1051,7 +1042,7 @@ include config.mak.uname -include config.mak ifdef DEVELOPER -CFLAGS += $(DEVELOPER_CFLAGS) +include config.mak.dev endif comma := , diff --git a/config.mak.dev b/config.mak.dev new file mode 100644 index 0000000000..59aef342c4 --- /dev/null +++ b/config.mak.dev @@ -0,0 +1,28 @@ +CFLAGS += -Werror +CFLAGS += -Wdeclaration-after-statement +CFLAGS += -Wno-format-zero-length +CFLAGS += -Wold-style-definition +CFLAGS += -Woverflow +CFLAGS += -Wpointer-arith +CFLAGS += -Wstrict-prototypes +CFLAGS += -Wunused +CFLAGS += -Wvla + +COMPILER_FEATURES := $(shell ./detect-compiler $(CC)) + +ifneq ($(filter clang4,$(COMPILER_FEATURES)),) +CFLAGS += -Wtautological-constant-out-of-range-compare +endif + +ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),) +CFLAGS += -Wextra +CFLAGS += -Wmissing-prototypes +CFLAGS += -Wno-empty-body +CFLAGS += -Wno-missing-field-initializers +CFLAGS += -Wno-sign-compare +CFLAGS += -Wno-unused-function +CFLAGS += -Wno-unused-parameter +ifneq ($(filter gcc6,$(COMPILER_FEATURES)),) +CFLAGS += -Wno-maybe-uninitialized +endif +endif diff --git a/detect-compiler b/detect-compiler new file mode 100755 index 0000000000..043367828c --- /dev/null +++ b/detect-compiler @@ -0,0 +1,32 @@ +#!/bin/sh +# +# Probe the compiler for vintage, version, etc. This is used for setting +# optional make knobs under the DEVELOPER knob. + +CC="$*" + +print_flags() { + family=$1 + version=$($CC -dumpversion | cut -f 1 -d .) + + # Print a feature flag not only for the current version, but also + # for any prior versions we encompass. This avoids needing to do + # numeric comparisons in make, which are awkward. + while test "$version" -gt 0 + do + echo $family$version + version=$((version - 1)) + done +} + +case "$($CC --version 2>&1)" in +gcc*) + print_flags gcc + ;; +clang*) + print_flags clang + ;; +*) + : unknown compiler family + ;; +esac -- 2.17.0.rc0.402.ged0b3fd1ee ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v2] travis-ci: enable more warnings on travis linux-gcc job 2018-03-17 16:08 ` Jeff King @ 2018-03-17 16:12 ` Jeff King 2018-03-17 23:50 ` Junio C Hamano 2018-03-18 8:18 ` [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 Nguyễn Thái Ngọc Duy 2 siblings, 0 replies; 47+ messages in thread From: Jeff King @ 2018-03-17 16:12 UTC (permalink / raw) To: Duy Nguyen; +Cc: Lars Schneider, Git Mailing List On Sat, Mar 17, 2018 at 12:08:32PM -0400, Jeff King wrote: > +case "$($CC --version 2>&1)" in > +gcc*) > + print_flags gcc > + ;; > +clang*) > + print_flags clang > + ;; > +*) > + : unknown compiler family > + ;; > +esac By the way, one annoying thing is that running "cc --version" when "cc" is actually "gcc" will print something like: cc (Debian 7.3.0-11) 7.3.0 even though it supports all of the gcc knobs. This means that: make DEVELOPER=1 without further config won't get these knobs, because we (rightly) default CC=cc. Probably this detection could be a bit more clever, like: cc*Free Software Foundation") print_flags gcc or something. I don't have any non-gcc/clang compilers to test with, so I'm not sure what they even print for "--version" (if anything). -Peff ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2] travis-ci: enable more warnings on travis linux-gcc job 2018-03-17 16:08 ` Jeff King 2018-03-17 16:12 ` Jeff King @ 2018-03-17 23:50 ` Junio C Hamano 2018-03-18 8:18 ` [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 Nguyễn Thái Ngọc Duy 2 siblings, 0 replies; 47+ messages in thread From: Junio C Hamano @ 2018-03-17 23:50 UTC (permalink / raw) To: Jeff King; +Cc: Duy Nguyen, Lars Schneider, Git Mailing List Jeff King <peff@peff.net> writes: > Unfortunately I think that's hard to do in pure make. But we're already > relying on $(shell) here, so we could just move the logic there. > > Something like the patch below, perhaps. It should do the right thing on > clang and gcc, and I added in an extra clang-only warning I've found > useful. Otherwise the list of flags comes from your patch. I see this discussion is going in the right direction, and like the approach taken by this "how about this" patch. Thanks. ^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-17 16:08 ` Jeff King 2018-03-17 16:12 ` Jeff King 2018-03-17 23:50 ` Junio C Hamano @ 2018-03-18 8:18 ` Nguyễn Thái Ngọc Duy 2018-03-18 9:06 ` Eric Sunshine ` (3 more replies) 2 siblings, 4 replies; 47+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2018-03-18 8:18 UTC (permalink / raw) To: peff; +Cc: git, larsxschneider, pclouds, Junio C Hamano 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. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- -dumpversion does not work correctly for clang. So I use "-v" instead which seems to produce the same output for both gcc and clang (with a minor difference in freebsd where it says "FreeBSD clang" instead of just "clang"). Not sure if it helps your "cc on debian" case though Tested with clang-5.0.1 and gcc-6.4.0 (too lazy to test on freebsd clang 3.4.1 but I don't expect surprises there) I will still need to pull more modern gcc/clang on travis, but that can wait until this patch settles. Makefile | 11 +---------- config.mak.dev | 28 +++++++++++++++++++++++++++ detect-compiler | 50 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 10 deletions(-) create mode 100644 config.mak.dev create mode 100755 detect-compiler diff --git a/Makefile b/Makefile index a1d8775adb..9dfd152a1e 100644 --- a/Makefile +++ b/Makefile @@ -442,15 +442,6 @@ GIT-VERSION-FILE: FORCE # CFLAGS and LDFLAGS are for the users to override from the command line. CFLAGS = -g -O2 -Wall -DEVELOPER_CFLAGS = -Werror \ - -Wdeclaration-after-statement \ - -Wno-format-zero-length \ - -Wold-style-definition \ - -Woverflow \ - -Wpointer-arith \ - -Wstrict-prototypes \ - -Wunused \ - -Wvla LDFLAGS = ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) ALL_LDFLAGS = $(LDFLAGS) @@ -1051,7 +1042,7 @@ include config.mak.uname -include config.mak ifdef DEVELOPER -CFLAGS += $(DEVELOPER_CFLAGS) +include config.mak.dev endif comma := , diff --git a/config.mak.dev b/config.mak.dev new file mode 100644 index 0000000000..59aef342c4 --- /dev/null +++ b/config.mak.dev @@ -0,0 +1,28 @@ +CFLAGS += -Werror +CFLAGS += -Wdeclaration-after-statement +CFLAGS += -Wno-format-zero-length +CFLAGS += -Wold-style-definition +CFLAGS += -Woverflow +CFLAGS += -Wpointer-arith +CFLAGS += -Wstrict-prototypes +CFLAGS += -Wunused +CFLAGS += -Wvla + +COMPILER_FEATURES := $(shell ./detect-compiler $(CC)) + +ifneq ($(filter clang4,$(COMPILER_FEATURES)),) +CFLAGS += -Wtautological-constant-out-of-range-compare +endif + +ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),) +CFLAGS += -Wextra +CFLAGS += -Wmissing-prototypes +CFLAGS += -Wno-empty-body +CFLAGS += -Wno-missing-field-initializers +CFLAGS += -Wno-sign-compare +CFLAGS += -Wno-unused-function +CFLAGS += -Wno-unused-parameter +ifneq ($(filter gcc6,$(COMPILER_FEATURES)),) +CFLAGS += -Wno-maybe-uninitialized +endif +endif diff --git a/detect-compiler b/detect-compiler new file mode 100755 index 0000000000..bc2ea39ef5 --- /dev/null +++ b/detect-compiler @@ -0,0 +1,50 @@ +#!/bin/sh +# +# Probe the compiler for vintage, version, etc. This is used for setting +# optional make knobs under the DEVELOPER knob. + +CC="$*" + +# we get something like (this is at least true for gcc and clang) +# +# FreeBSD clang version 3.4.1 (tags/RELEASE...) +get_version_line() { + "$CC" -v 2>&1 | grep ' version ' +} + +get_family() { + get_version_line | sed 's/^\(.*\) version [0-9][^ ]* .*/\1/' +} + +get_version() { + get_version_line | sed 's/^.* version \([0-9][^ ]*\) .*/\1/' +} + +print_flags() { + family=$1 + version=$(get_version | cut -f 1 -d .) + + # Print a feature flag not only for the current version, but also + # for any prior versions we encompass. This avoids needing to do + # numeric comparisons in make, which are awkward. + while test "$version" -gt 0 + do + echo $family$version + version=$((version - 1)) + done +} + +case "$(get_family)" in +gcc) + print_flags gcc + ;; +*clang) + print_flags clang + ;; +"FreeBSD clang") + print_flags clang + ;; +*) + : unknown compiler family + ;; +esac -- 2.17.0.rc0.347.gf9cf61673a ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-18 8:18 ` [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 Nguyễn Thái Ngọc Duy @ 2018-03-18 9:06 ` Eric Sunshine 2018-03-18 9:17 ` Duy Nguyen 2018-03-18 9:28 ` Jeff King 2018-03-18 9:26 ` Jeff King ` (2 subsequent siblings) 3 siblings, 2 replies; 47+ messages in thread From: Eric Sunshine @ 2018-03-18 9:06 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy Cc: peff, git, larsxschneider, Junio C Hamano On Sun, Mar 18, 2018 at 09:18:34AM +0100, Nguyễn Thái Ngọc Duy wrote: > 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. > > Helped-by: Jeff King <peff@peff.net> > Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > --- > diff --git a/detect-compiler b/detect-compiler > --- /dev/null > +++ b/detect-compiler > @@ -0,0 +1,50 @@ > +get_version_line() { > + "$CC" -v 2>&1 | grep ' version ' > +} On MacOS, "cc -v" output is: --- >8 --- Apple LLVM version 9.0.0 (clang-900.0.39.2) Target: x86_64-apple-darwin16.7.0 Thread model: posix InstalledDir: ... --- >8 --- > +get_family() { > + get_version_line | sed 's/^\(.*\) version [0-9][^ ]* .*/\1/' > +} So, this returns "Apple LLVM". > +case "$(get_family)" in > +gcc) > + print_flags gcc > + ;; > +*clang) > + print_flags clang > + ;; > +"FreeBSD clang") > + print_flags clang > + ;; > +*) > + : unknown compiler family > + ;; > +esac Which means you probably want to squash in: --- >8 --- diff --git a/detect-compiler b/detect-compiler index bc2ea39ef5..3140416b40 100755 --- a/detect-compiler +++ b/detect-compiler @@ -41,6 +41,9 @@ gcc) *clang) print_flags clang ;; +"Apple LLVM") + print_flags clang + ;; "FreeBSD clang") print_flags clang ;; --- >8 --- ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-18 9:06 ` Eric Sunshine @ 2018-03-18 9:17 ` Duy Nguyen 2018-03-18 9:20 ` Jeff King 2018-03-18 9:24 ` Eric Sunshine 2018-03-18 9:28 ` Jeff King 1 sibling, 2 replies; 47+ messages in thread From: Duy Nguyen @ 2018-03-18 9:17 UTC (permalink / raw) To: Eric Sunshine; +Cc: Jeff King, Git Mailing List, Lars Schneider, Junio C Hamano On Sun, Mar 18, 2018 at 10:06 AM, Eric Sunshine <sunshine@sunshineco.com> wrote: > On Sun, Mar 18, 2018 at 09:18:34AM +0100, Nguyễn Thái Ngọc Duy wrote: >> 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. >> >> Helped-by: Jeff King <peff@peff.net> >> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> >> --- >> diff --git a/detect-compiler b/detect-compiler >> --- /dev/null >> +++ b/detect-compiler >> @@ -0,0 +1,50 @@ >> +get_version_line() { >> + "$CC" -v 2>&1 | grep ' version ' >> +} > > On MacOS, "cc -v" output is: > > --- >8 --- > Apple LLVM version 9.0.0 (clang-900.0.39.2) > Target: x86_64-apple-darwin16.7.0 > Thread model: posix > InstalledDir: ... > --- >8 --- Does it still build ok for you with your changes squashed in? I think the check for clang4/gcc6 and setting -Wextra in config.mak.dev may backfire because clang9 probably has a lot more warnings enabled and some of them we don't want and cause compile error... -- Duy ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-18 9:17 ` Duy Nguyen @ 2018-03-18 9:20 ` Jeff King 2018-03-18 9:24 ` Eric Sunshine 1 sibling, 0 replies; 47+ messages in thread From: Jeff King @ 2018-03-18 9:20 UTC (permalink / raw) To: Duy Nguyen Cc: Eric Sunshine, Git Mailing List, Lars Schneider, Junio C Hamano On Sun, Mar 18, 2018 at 10:17:41AM +0100, Duy Nguyen wrote: > On Sun, Mar 18, 2018 at 10:06 AM, Eric Sunshine <sunshine@sunshineco.com> wrote: > > On Sun, Mar 18, 2018 at 09:18:34AM +0100, Nguyễn Thái Ngọc Duy wrote: > >> 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. > >> > >> Helped-by: Jeff King <peff@peff.net> > >> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> > >> --- > >> diff --git a/detect-compiler b/detect-compiler > >> --- /dev/null > >> +++ b/detect-compiler > >> @@ -0,0 +1,50 @@ > >> +get_version_line() { > >> + "$CC" -v 2>&1 | grep ' version ' > >> +} > > > > On MacOS, "cc -v" output is: > > > > --- >8 --- > > Apple LLVM version 9.0.0 (clang-900.0.39.2) > > Target: x86_64-apple-darwin16.7.0 > > Thread model: posix > > InstalledDir: ... > > --- >8 --- > > Does it still build ok for you with your changes squashed in? I think > the check for clang4/gcc6 and setting -Wextra in config.mak.dev may > backfire because clang9 probably has a lot more warnings enabled and > some of them we don't want and cause compile error... I don't think we can predict that going forward, though. We'd have to wait for people to use the new compiler and then send a patch disabling whatever causes a problem (or better yet, fixing the code if appropriate). This framework can only help us write those patches; I don't think it can save us from having to do them in the first place. -Peff ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-18 9:17 ` Duy Nguyen 2018-03-18 9:20 ` Jeff King @ 2018-03-18 9:24 ` Eric Sunshine 1 sibling, 0 replies; 47+ messages in thread From: Eric Sunshine @ 2018-03-18 9:24 UTC (permalink / raw) To: Duy Nguyen; +Cc: Jeff King, Git Mailing List, Lars Schneider, Junio C Hamano On Sun, Mar 18, 2018 at 5:17 AM, Duy Nguyen <pclouds@gmail.com> wrote: > On Sun, Mar 18, 2018 at 10:06 AM, Eric Sunshine <sunshine@sunshineco.com> wrote: >> On MacOS, "cc -v" output is: >> --- >8 --- >> Apple LLVM version 9.0.0 (clang-900.0.39.2) >> Target: x86_64-apple-darwin16.7.0 >> Thread model: posix >> InstalledDir: ... >> --- >8 --- > > Does it still build ok for you with your changes squashed in? I think > the check for clang4/gcc6 and setting -Wextra in config.mak.dev may > backfire because clang9 probably has a lot more warnings enabled and > some of them we don't want and cause compile error... The project does still compile cleanly for me. (And I used "make V=1" to verify that the new flags are used.) ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-18 9:06 ` Eric Sunshine 2018-03-18 9:17 ` Duy Nguyen @ 2018-03-18 9:28 ` Jeff King 2018-03-18 9:37 ` Eric Sunshine 1 sibling, 1 reply; 47+ messages in thread From: Jeff King @ 2018-03-18 9:28 UTC (permalink / raw) To: Eric Sunshine Cc: Nguyễn Thái Ngọc Duy, git, larsxschneider, Junio C Hamano On Sun, Mar 18, 2018 at 05:06:07AM -0400, Eric Sunshine wrote: > On MacOS, "cc -v" output is: > > --- >8 --- > Apple LLVM version 9.0.0 (clang-900.0.39.2) > Target: x86_64-apple-darwin16.7.0 > Thread model: posix > InstalledDir: ... > --- >8 --- Is that really way ahead of the clang releases (which are at 7.0, AFAIK), or do they use a totally different number scheme? If the latter, I guess we'd have to treat it separately from clang and have all of the conditionals treat it independently? Yuck. -Peff ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-18 9:28 ` Jeff King @ 2018-03-18 9:37 ` Eric Sunshine 0 siblings, 0 replies; 47+ messages in thread From: Eric Sunshine @ 2018-03-18 9:37 UTC (permalink / raw) To: Jeff King Cc: Nguyễn Thái Ngọc Duy, Git List, Lars Schneider, Junio C Hamano On Sun, Mar 18, 2018 at 5:28 AM, Jeff King <peff@peff.net> wrote: > On Sun, Mar 18, 2018 at 05:06:07AM -0400, Eric Sunshine wrote: >> On MacOS, "cc -v" output is: >> --- >8 --- >> Apple LLVM version 9.0.0 (clang-900.0.39.2) >> Target: x86_64-apple-darwin16.7.0 >> Thread model: posix >> InstalledDir: ... >> --- >8 --- > > Is that really way ahead of the clang releases (which are at 7.0, > AFAIK), or do they use a totally different number scheme? I have no idea, but from past experience with Apple, I'd guess that the version number is an Apple invention. {...goes and researches a bit...} Indeed, there doesn't seem to be any correlation between what Apple reports and the actual clang version number[1]. [1]: https://stackoverflow.com/questions/33603027/get-apple-clang-version-and-corresponding-upstream-llvm-version ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-18 8:18 ` [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 Nguyễn Thái Ngọc Duy 2018-03-18 9:06 ` Eric Sunshine @ 2018-03-18 9:26 ` Jeff King 2018-03-18 9:45 ` Duy Nguyen 2018-03-18 15:55 ` Duy Nguyen 2018-03-24 12:53 ` [PATCH v2] " Nguyễn Thái Ngọc Duy 3 siblings, 1 reply; 47+ messages in thread From: Jeff King @ 2018-03-18 9:26 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy; +Cc: git, larsxschneider, Junio C Hamano On Sun, Mar 18, 2018 at 09:18:34AM +0100, Nguyễn Thái Ngọc Duy wrote: > 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. Hrm, I was planning to expand my patch into a series. In particular, in my experiments some of those warnings do not need to be disabled under -Wextra (I tested going back to gcc 5 and clang 4.0). I think it's worth splitting up the boilerplate changes from the decisions on individual warnings, because I suspect we'll need to revise the latter going forward. I also noticed that all of those options seem to go back at least to gcc-5. It makes me wonder how much we even need this version-detecting framework. There are one or two "clang vs gcc" things I've found, but I haven't found a case where old versions of gcc don't support a particular option. IOW, could we get away with just adding these to the DEVELOPER knob and assuming that people who use it have a reasonably modern gcc or clang setup? That's more or less what the existing knobs do. But I didn't follow whether there was any earlier discussion on specific problems. > -dumpversion does not work correctly for clang. So I use "-v" instead > which seems to produce the same output for both gcc and clang (with a > minor difference in freebsd where it says "FreeBSD clang" instead of > just "clang"). Not sure if it helps your "cc on debian" case though Heh. At first I was confused, as it seems to work for me: $ clang-4.0 -dumpversion 4.2.1 But then I tried this: $ clang-5.0 -dumpversion 4.2.1 Whoops. It returns the same value up through clang 7. -Peff ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-18 9:26 ` Jeff King @ 2018-03-18 9:45 ` Duy Nguyen 0 siblings, 0 replies; 47+ messages in thread From: Duy Nguyen @ 2018-03-18 9:45 UTC (permalink / raw) To: Jeff King; +Cc: Git Mailing List, Lars Schneider, Junio C Hamano On Sun, Mar 18, 2018 at 10:26 AM, Jeff King <peff@peff.net> wrote: > On Sun, Mar 18, 2018 at 09:18:34AM +0100, Nguyễn Thái Ngọc Duy wrote: > >> 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. > > Hrm, I was planning to expand my patch into a series. I started this so I think people expect me to move it forward. But I'll gladly hand it to you. It looks like you have more compilers to play with anyway (I'm on gentoo and installing a new compiler, especially llvm-based, takes ages). -- Duy ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-18 8:18 ` [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 Nguyễn Thái Ngọc Duy 2018-03-18 9:06 ` Eric Sunshine 2018-03-18 9:26 ` Jeff King @ 2018-03-18 15:55 ` Duy Nguyen 2018-03-18 18:56 ` Ramsay Jones 2018-03-20 5:32 ` Jeff King 2018-03-24 12:53 ` [PATCH v2] " Nguyễn Thái Ngọc Duy 3 siblings, 2 replies; 47+ messages in thread From: Duy Nguyen @ 2018-03-18 15:55 UTC (permalink / raw) To: Jeff King; +Cc: Git Mailing List, Lars Schneider, Duy Nguyen, Junio C Hamano On Sun, Mar 18, 2018 at 9:18 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote: > +ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),) > +CFLAGS += -Wextra Another thing we can add here is -Og instead of standard -O2 (or -O0 in my build), which is supported since gcc 4.8. clang seems to support it too (mapped to -O1 at least for clang5) but I don't know what version added that flag. -- Duy ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-18 15:55 ` Duy Nguyen @ 2018-03-18 18:56 ` Ramsay Jones 2018-03-19 16:30 ` Duy Nguyen 2018-03-20 5:32 ` Jeff King 1 sibling, 1 reply; 47+ messages in thread From: Ramsay Jones @ 2018-03-18 18:56 UTC (permalink / raw) To: Duy Nguyen, Jeff King; +Cc: Git Mailing List, Lars Schneider, Junio C Hamano On 18/03/18 15:55, Duy Nguyen wrote: > On Sun, Mar 18, 2018 at 9:18 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote: >> +ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),) >> +CFLAGS += -Wextra > > Another thing we can add here is -Og instead of standard -O2 (or -O0 > in my build), which is supported since gcc 4.8. clang seems to support > it too (mapped to -O1 at least for clang5) but I don't know what > version added that flag. I've been doing a lot of compiling recently, using 10 'different' versions of clang and gcc ('different' if you count 64-bit and 32-bit compilers with the same version number as different!) However, I can tell you that clang version 3.4 and 3.8.0 don't support -Og, but clang version 5.0.1 does. ATB, Ramsay Jones ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-18 18:56 ` Ramsay Jones @ 2018-03-19 16:30 ` Duy Nguyen 0 siblings, 0 replies; 47+ messages in thread From: Duy Nguyen @ 2018-03-19 16:30 UTC (permalink / raw) To: Ramsay Jones; +Cc: Jeff King, Git Mailing List, Lars Schneider, Junio C Hamano On Sun, Mar 18, 2018 at 7:56 PM, Ramsay Jones <ramsay@ramsayjones.plus.com> wrote: > > > On 18/03/18 15:55, Duy Nguyen wrote: >> On Sun, Mar 18, 2018 at 9:18 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote: >>> +ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),) >>> +CFLAGS += -Wextra >> >> Another thing we can add here is -Og instead of standard -O2 (or -O0 >> in my build), which is supported since gcc 4.8. clang seems to support >> it too (mapped to -O1 at least for clang5) but I don't know what >> version added that flag. > > I've been doing a lot of compiling recently, using 10 'different' > versions of clang and gcc ('different' if you count 64-bit and 32-bit > compilers with the same version number as different!) > > However, I can tell you that clang version 3.4 and 3.8.0 don't > support -Og, but clang version 5.0.1 does. Yeah. I checked clang git mirror and this -Og is in 4.x release branch (couldn't nail down exact release) so 5.x should be a safe bet. -- Duy ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-18 15:55 ` Duy Nguyen 2018-03-18 18:56 ` Ramsay Jones @ 2018-03-20 5:32 ` Jeff King 1 sibling, 0 replies; 47+ messages in thread From: Jeff King @ 2018-03-20 5:32 UTC (permalink / raw) To: Duy Nguyen; +Cc: Git Mailing List, Lars Schneider, Junio C Hamano On Sun, Mar 18, 2018 at 04:55:25PM +0100, Duy Nguyen wrote: > On Sun, Mar 18, 2018 at 9:18 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote: > > +ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),) > > +CFLAGS += -Wextra > > Another thing we can add here is -Og instead of standard -O2 (or -O0 > in my build), which is supported since gcc 4.8. clang seems to support > it too (mapped to -O1 at least for clang5) but I don't know what > version added that flag. I don't know, that feels kind of weird to me. I thought DEVELOPER was supposed to mean "ratchet up the linting, I want to know if I've broken something". But tweaking -O is about "I am in an edit-compile-debug cycle". Sometimes you are and sometimes you aren't, but you'd presumably want to have extra warnings regardless (especially because some warnings only trigger under particular optimization settings). Personally, I default to -O0, but then crank up to -O2 for performance testing or for my daily-driver builds. But I _always_ have as many warnings enabled as possible[1]. -Peff [1] I do have some pretty horrific magic to turn on -Werror when I'm visiting a "historical" commit, such as during a bisect. ^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v2] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-18 8:18 ` [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 Nguyễn Thái Ngọc Duy ` (2 preceding siblings ...) 2018-03-18 15:55 ` Duy Nguyen @ 2018-03-24 12:53 ` Nguyễn Thái Ngọc Duy 2018-03-25 0:40 ` Eric Sunshine ` (2 more replies) 3 siblings, 3 replies; 47+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2018-03-24 12:53 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jeff King, Ramsay Jones, larsxschneider, Nguyễn Thái Ngọc Duy 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 them 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> --- v2 improves a bit over v1: - apple clang support (though I suspect we may want to make it a separate compiler family since apple clang has different versioning, but I can't and won't work on that front) - support CC='ccache gcc' (yes it breaks now if you have spaces in your cc path) - allow to skip detect-compiler by setting COMPILER_FEATURES in config.mak I notice Ramsay is working on clean -Wmaybe-uninitialized, if his series is merged first I'll stop disabling it here. Interdiff diff --git a/Makefile b/Makefile index 9dfd152a1e..04b2a39bab 100644 --- a/Makefile +++ b/Makefile @@ -434,6 +434,10 @@ all:: # # When cross-compiling, define HOST_CPU as the canonical name of the CPU on # which the built Git will run (for instance "x86_64"). +# +# Define DEVELOPER to enable more compiler warnings. Compiler version +# and faimily are auto detected, but could be overridden by defining +# COMPILER_FEATURES (see config.mak.dev) GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN diff --git a/config.mak.dev b/config.mak.dev index 59aef342c4..d8beaf9347 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -8,7 +8,9 @@ CFLAGS += -Wstrict-prototypes CFLAGS += -Wunused CFLAGS += -Wvla +ifndef COMPILER_FEATURES COMPILER_FEATURES := $(shell ./detect-compiler $(CC)) +endif ifneq ($(filter clang4,$(COMPILER_FEATURES)),) CFLAGS += -Wtautological-constant-out-of-range-compare diff --git a/detect-compiler b/detect-compiler index bc2ea39ef5..70b754481c 100755 --- a/detect-compiler +++ b/detect-compiler @@ -9,7 +9,7 @@ CC="$*" # # FreeBSD clang version 3.4.1 (tags/RELEASE...) get_version_line() { - "$CC" -v 2>&1 | grep ' version ' + $CC -v 2>&1 | grep ' version ' } get_family() { @@ -38,12 +38,15 @@ case "$(get_family)" in gcc) print_flags gcc ;; -*clang) +clang) print_flags clang ;; "FreeBSD clang") print_flags clang ;; +"Apple LLVM") + print_flags clang + ;; *) : unknown compiler family ;; Makefile | 15 +++++--------- config.mak.dev | 30 ++++++++++++++++++++++++++++ detect-compiler | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+), 10 deletions(-) create mode 100644 config.mak.dev create mode 100755 detect-compiler diff --git a/Makefile b/Makefile index a1d8775adb..04b2a39bab 100644 --- a/Makefile +++ b/Makefile @@ -434,6 +434,10 @@ all:: # # When cross-compiling, define HOST_CPU as the canonical name of the CPU on # which the built Git will run (for instance "x86_64"). +# +# Define DEVELOPER to enable more compiler warnings. Compiler version +# and faimily are auto detected, but could be overridden by defining +# COMPILER_FEATURES (see config.mak.dev) GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -442,15 +446,6 @@ GIT-VERSION-FILE: FORCE # CFLAGS and LDFLAGS are for the users to override from the command line. CFLAGS = -g -O2 -Wall -DEVELOPER_CFLAGS = -Werror \ - -Wdeclaration-after-statement \ - -Wno-format-zero-length \ - -Wold-style-definition \ - -Woverflow \ - -Wpointer-arith \ - -Wstrict-prototypes \ - -Wunused \ - -Wvla LDFLAGS = ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) ALL_LDFLAGS = $(LDFLAGS) @@ -1051,7 +1046,7 @@ include config.mak.uname -include config.mak ifdef DEVELOPER -CFLAGS += $(DEVELOPER_CFLAGS) +include config.mak.dev endif comma := , diff --git a/config.mak.dev b/config.mak.dev new file mode 100644 index 0000000000..d8beaf9347 --- /dev/null +++ b/config.mak.dev @@ -0,0 +1,30 @@ +CFLAGS += -Werror +CFLAGS += -Wdeclaration-after-statement +CFLAGS += -Wno-format-zero-length +CFLAGS += -Wold-style-definition +CFLAGS += -Woverflow +CFLAGS += -Wpointer-arith +CFLAGS += -Wstrict-prototypes +CFLAGS += -Wunused +CFLAGS += -Wvla + +ifndef COMPILER_FEATURES +COMPILER_FEATURES := $(shell ./detect-compiler $(CC)) +endif + +ifneq ($(filter clang4,$(COMPILER_FEATURES)),) +CFLAGS += -Wtautological-constant-out-of-range-compare +endif + +ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),) +CFLAGS += -Wextra +CFLAGS += -Wmissing-prototypes +CFLAGS += -Wno-empty-body +CFLAGS += -Wno-missing-field-initializers +CFLAGS += -Wno-sign-compare +CFLAGS += -Wno-unused-function +CFLAGS += -Wno-unused-parameter +ifneq ($(filter gcc6,$(COMPILER_FEATURES)),) +CFLAGS += -Wno-maybe-uninitialized +endif +endif diff --git a/detect-compiler b/detect-compiler new file mode 100755 index 0000000000..70b754481c --- /dev/null +++ b/detect-compiler @@ -0,0 +1,53 @@ +#!/bin/sh +# +# Probe the compiler for vintage, version, etc. This is used for setting +# optional make knobs under the DEVELOPER knob. + +CC="$*" + +# we get something like (this is at least true for gcc and clang) +# +# FreeBSD clang version 3.4.1 (tags/RELEASE...) +get_version_line() { + $CC -v 2>&1 | grep ' version ' +} + +get_family() { + get_version_line | sed 's/^\(.*\) version [0-9][^ ]* .*/\1/' +} + +get_version() { + get_version_line | sed 's/^.* version \([0-9][^ ]*\) .*/\1/' +} + +print_flags() { + family=$1 + version=$(get_version | cut -f 1 -d .) + + # Print a feature flag not only for the current version, but also + # for any prior versions we encompass. This avoids needing to do + # numeric comparisons in make, which are awkward. + while test "$version" -gt 0 + do + echo $family$version + version=$((version - 1)) + done +} + +case "$(get_family)" in +gcc) + print_flags gcc + ;; +clang) + print_flags clang + ;; +"FreeBSD clang") + print_flags clang + ;; +"Apple LLVM") + print_flags clang + ;; +*) + : unknown compiler family + ;; +esac -- 2.17.0.rc0.348.gd5a49e0b6f ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v2] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-24 12:53 ` [PATCH v2] " Nguyễn Thái Ngọc Duy @ 2018-03-25 0:40 ` Eric Sunshine 2018-03-26 22:02 ` Junio C Hamano 2018-03-29 15:03 ` [PATCH v3 0/3] Enable more compiler warnings for devs Nguyễn Thái Ngọc Duy 2 siblings, 0 replies; 47+ messages in thread From: Eric Sunshine @ 2018-03-25 0:40 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy Cc: Git List, Junio C Hamano, Jeff King, Ramsay Jones, Lars Schneider On Sat, Mar 24, 2018 at 8:53 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote: > 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 them those warnings are s/them those/those/ > 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> ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-24 12:53 ` [PATCH v2] " Nguyễn Thái Ngọc Duy 2018-03-25 0:40 ` Eric Sunshine @ 2018-03-26 22:02 ` Junio C Hamano 2018-03-27 15:03 ` Duy Nguyen 2018-03-29 15:03 ` [PATCH v3 0/3] Enable more compiler warnings for devs Nguyễn Thái Ngọc Duy 2 siblings, 1 reply; 47+ messages in thread From: Junio C Hamano @ 2018-03-26 22:02 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy Cc: git, Jeff King, Ramsay Jones, larsxschneider Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > 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. This is a good idea in general, but we are not quite ready without some fixups. Here is a quick summary (not exhaustive) from my trial merge to 'pu' (which will be reverted before today's integration is pushed out). - json-writer.c triggers -Werror=old-style-decl - t/helper/test-json-writer.c triggers Werror=missing-prototypes quite a few times. - connect.c -Werror=implicit-fallthough around die_initial_contact(). ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-26 22:02 ` Junio C Hamano @ 2018-03-27 15:03 ` Duy Nguyen 2018-03-27 16:52 ` Junio C Hamano 0 siblings, 1 reply; 47+ messages in thread From: Duy Nguyen @ 2018-03-27 15:03 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List, Jeff King, Ramsay Jones, Lars Schneider On Tue, Mar 27, 2018 at 12:02 AM, Junio C Hamano <gitster@pobox.com> wrote: > Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes: > >> 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. > > This is a good idea in general, but we are not quite ready without > some fixups. > > Here is a quick summary (not exhaustive) from my trial merge to 'pu' > (which will be reverted before today's integration is pushed out). > > - json-writer.c triggers -Werror=old-style-decl > > - t/helper/test-json-writer.c triggers Werror=missing-prototypes > quite a few times. I expected these (and it was a good reason to push this patch forward). I think people also reported style problems with this series. > > - connect.c -Werror=implicit-fallthough around die_initial_contact(). > This I did not expect. But I just looked again and I had this option explicitly turned off in config.mak! gcc-6.4 and gcc-4.9 do not complain about this. gcc-7.3 does. What's your compiler/version? -- Duy ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v2] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-27 15:03 ` Duy Nguyen @ 2018-03-27 16:52 ` Junio C Hamano 0 siblings, 0 replies; 47+ messages in thread From: Junio C Hamano @ 2018-03-27 16:52 UTC (permalink / raw) To: Duy Nguyen; +Cc: Git Mailing List, Jeff King, Ramsay Jones, Lars Schneider Duy Nguyen <pclouds@gmail.com> writes: On Tue, Mar 27, 2018 at 12:02 AM, Junio C Hamano <gitster@pobox.com> wrote: >> >> - connect.c -Werror=implicit-fallthough around die_initial_contact(). > > This I did not expect. But I just looked again and I had this option > explicitly turned off in config.mak! gcc-6.4 and gcc-4.9 do not > complain about this. gcc-7.3 does. What's your compiler/version? gcc (Debian 7.3.0-5) 7.3.0 ^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v3 0/3] Enable more compiler warnings for devs 2018-03-24 12:53 ` [PATCH v2] " Nguyễn Thái Ngọc Duy 2018-03-25 0:40 ` Eric Sunshine 2018-03-26 22:02 ` Junio C Hamano @ 2018-03-29 15:03 ` Nguyễn Thái Ngọc Duy 2018-03-29 15:03 ` [PATCH v3 1/3] connect.c: mark die_initial_contact() NORETURN Nguyễn Thái Ngọc Duy ` (8 more replies) 2 siblings, 9 replies; 47+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2018-03-29 15:03 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jeff King, ramsay, larsxschneider, Eric Sunshine, Nguyễn Thái Ngọc Duy v3 fixes the fallthru warnings in connect.c, and with json-writer series rerolled, 'pu' should build cleanly now. -Wno-maybe-uninitialized is removed, thanks to Ramsay. v3 also adds an experimental patch that adds EAGER_DEVELOPER=1. These developers will have all warnings enabled (nothing is suppressed) but they are not fatal. They could go through them and perhaps clean up the code base a bit more. Interdiff diff --git a/Makefile b/Makefile index e6680a8977..e4f04ce1cb 100644 --- a/Makefile +++ b/Makefile @@ -434,7 +434,9 @@ all:: # # Define DEVELOPER to enable more compiler warnings. Compiler version # and faimily are auto detected, but could be overridden by defining -# COMPILER_FEATURES (see config.mak.dev) +# COMPILER_FEATURES (see config.mak.dev). +# Define EAGER_DEVELOPER keeps compiler warnings non-fatal, but no warning +# class is suppressed anymore. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -1041,6 +1043,9 @@ include config.mak.uname -include config.mak.autogen -include config.mak +ifdef EAGER_DEVELOPER +DEVELOPER = Yes +endif ifdef DEVELOPER include config.mak.dev endif diff --git a/config.mak.dev b/config.mak.dev index d8beaf9347..13883410b3 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -1,4 +1,6 @@ +ifndef EAGER_DEVELOPER CFLAGS += -Werror +endif CFLAGS += -Wdeclaration-after-statement CFLAGS += -Wno-format-zero-length CFLAGS += -Wold-style-definition @@ -18,13 +20,23 @@ endif ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),) 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. CFLAGS += -Wmissing-prototypes +ifndef EAGER_DEVELOPER +# These are disabled because we have these all over the place. CFLAGS += -Wno-empty-body CFLAGS += -Wno-missing-field-initializers CFLAGS += -Wno-sign-compare CFLAGS += -Wno-unused-function CFLAGS += -Wno-unused-parameter -ifneq ($(filter gcc6,$(COMPILER_FEATURES)),) -CFLAGS += -Wno-maybe-uninitialized +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 +ifneq ($(filter gcc4,$(COMPILER_FEATURES)),) +ifeq ($(filter gcc5,$(COMPILER_FEATURES)),) +CFLAGS += -Wno-uninitialized endif endif diff --git a/connect.c b/connect.c index c3a014c5ba..49eca46462 100644 --- a/connect.c +++ b/connect.c @@ -46,7 +46,7 @@ int check_ref_type(const struct ref *ref, int flags) return check_ref(ref->name, flags); } -static void die_initial_contact(int unexpected) +static NORETURN void die_initial_contact(int unexpected) { if (unexpected) die(_("The remote end hung up upon initial contact")); Nguyễn Thái Ngọc Duy (3): connect.c: mark die_initial_contact() NORETURN Makefile: detect compiler and enable more warnings in DEVELOPER=1 Makefile: add EAGER_DEVELOPER mode Makefile | 20 +++++++++---------- config.mak.dev | 42 +++++++++++++++++++++++++++++++++++++++ connect.c | 2 +- detect-compiler | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+), 11 deletions(-) create mode 100644 config.mak.dev create mode 100755 detect-compiler -- 2.17.0.rc1.439.gca064e2955 ^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v3 1/3] connect.c: mark die_initial_contact() NORETURN 2018-03-29 15:03 ` [PATCH v3 0/3] Enable more compiler warnings for devs Nguyễn Thái Ngọc Duy @ 2018-03-29 15:03 ` Nguyễn Thái Ngọc Duy 2018-03-29 15:03 ` [PATCH v3 2/3] Makefile: detect compiler and enable more warnings in DEVELOPER=1 Nguyễn Thái Ngọc Duy ` (7 subsequent siblings) 8 siblings, 0 replies; 47+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2018-03-29 15:03 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jeff King, ramsay, larsxschneider, Eric Sunshine, Nguyễn Thái Ngọc Duy There is a series running in parallel with this one that adds code like this switch (...) { case ...: die_initial_contact(); case ...: There is nothing wrong with this. There is no actual falling through. But since gcc is not that smart and gcc 7.x introduces -Wimplicit-fallthrough, it raises a false alarm in this case. This class of warnings may be useful elsewhere, so instead of suppressing the whole class, let's try to fix just this code. gcc is smart enough to realize that no execution can continue after a NORETURN function call and no longer raises the warning. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- connect.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/connect.c b/connect.c index c3a014c5ba..49eca46462 100644 --- a/connect.c +++ b/connect.c @@ -46,7 +46,7 @@ int check_ref_type(const struct ref *ref, int flags) return check_ref(ref->name, flags); } -static void die_initial_contact(int unexpected) +static NORETURN void die_initial_contact(int unexpected) { if (unexpected) die(_("The remote end hung up upon initial contact")); -- 2.17.0.rc1.439.gca064e2955 ^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v3 2/3] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-29 15:03 ` [PATCH v3 0/3] Enable more compiler warnings for devs Nguyễn Thái Ngọc Duy 2018-03-29 15:03 ` [PATCH v3 1/3] connect.c: mark die_initial_contact() NORETURN Nguyễn Thái Ngọc Duy @ 2018-03-29 15:03 ` Nguyễn Thái Ngọc Duy 2018-03-29 15:03 ` [PATCH v3 3/3] Makefile: add EAGER_DEVELOPER mode Nguyễn Thái Ngọc Duy ` (6 subsequent siblings) 8 siblings, 0 replies; 47+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2018-03-29 15:03 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jeff King, ramsay, larsxschneider, Eric Sunshine, Nguyễn Thái Ngọc Duy 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> --- Makefile | 15 +++++--------- config.mak.dev | 38 +++++++++++++++++++++++++++++++++++ detect-compiler | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 10 deletions(-) create mode 100644 config.mak.dev create mode 100755 detect-compiler diff --git a/Makefile b/Makefile index 7f40f76739..e6680a8977 100644 --- a/Makefile +++ b/Makefile @@ -431,6 +431,10 @@ all:: # # When cross-compiling, define HOST_CPU as the canonical name of the CPU on # which the built Git will run (for instance "x86_64"). +# +# Define DEVELOPER to enable more compiler warnings. Compiler version +# and faimily are auto detected, but could be overridden by defining +# COMPILER_FEATURES (see config.mak.dev) GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -439,15 +443,6 @@ GIT-VERSION-FILE: FORCE # CFLAGS and LDFLAGS are for the users to override from the command line. CFLAGS = -g -O2 -Wall -DEVELOPER_CFLAGS = -Werror \ - -Wdeclaration-after-statement \ - -Wno-format-zero-length \ - -Wold-style-definition \ - -Woverflow \ - -Wpointer-arith \ - -Wstrict-prototypes \ - -Wunused \ - -Wvla LDFLAGS = ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) ALL_LDFLAGS = $(LDFLAGS) @@ -1047,7 +1042,7 @@ include config.mak.uname -include config.mak ifdef DEVELOPER -CFLAGS += $(DEVELOPER_CFLAGS) +include config.mak.dev endif comma := , diff --git a/config.mak.dev b/config.mak.dev new file mode 100644 index 0000000000..716a14ecc7 --- /dev/null +++ b/config.mak.dev @@ -0,0 +1,38 @@ +CFLAGS += -Werror +CFLAGS += -Wdeclaration-after-statement +CFLAGS += -Wno-format-zero-length +CFLAGS += -Wold-style-definition +CFLAGS += -Woverflow +CFLAGS += -Wpointer-arith +CFLAGS += -Wstrict-prototypes +CFLAGS += -Wunused +CFLAGS += -Wvla + +ifndef COMPILER_FEATURES +COMPILER_FEATURES := $(shell ./detect-compiler $(CC)) +endif + +ifneq ($(filter clang4,$(COMPILER_FEATURES)),) +CFLAGS += -Wtautological-constant-out-of-range-compare +endif + +ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),) +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. +CFLAGS += -Wmissing-prototypes +# These are disabled because we have these all over the place. +CFLAGS += -Wno-empty-body +CFLAGS += -Wno-missing-field-initializers +CFLAGS += -Wno-sign-compare +CFLAGS += -Wno-unused-function +CFLAGS += -Wno-unused-parameter +endif + +# uninitialized warnings on gcc 4.9.2 in xdiff/xdiffi.c and config.c +# not worth fixing since newer compilers correctly stop complaining +ifneq ($(filter gcc4,$(COMPILER_FEATURES)),) +ifeq ($(filter gcc5,$(COMPILER_FEATURES)),) +CFLAGS += -Wno-uninitialized +endif +endif diff --git a/detect-compiler b/detect-compiler new file mode 100755 index 0000000000..70b754481c --- /dev/null +++ b/detect-compiler @@ -0,0 +1,53 @@ +#!/bin/sh +# +# Probe the compiler for vintage, version, etc. This is used for setting +# optional make knobs under the DEVELOPER knob. + +CC="$*" + +# we get something like (this is at least true for gcc and clang) +# +# FreeBSD clang version 3.4.1 (tags/RELEASE...) +get_version_line() { + $CC -v 2>&1 | grep ' version ' +} + +get_family() { + get_version_line | sed 's/^\(.*\) version [0-9][^ ]* .*/\1/' +} + +get_version() { + get_version_line | sed 's/^.* version \([0-9][^ ]*\) .*/\1/' +} + +print_flags() { + family=$1 + version=$(get_version | cut -f 1 -d .) + + # Print a feature flag not only for the current version, but also + # for any prior versions we encompass. This avoids needing to do + # numeric comparisons in make, which are awkward. + while test "$version" -gt 0 + do + echo $family$version + version=$((version - 1)) + done +} + +case "$(get_family)" in +gcc) + print_flags gcc + ;; +clang) + print_flags clang + ;; +"FreeBSD clang") + print_flags clang + ;; +"Apple LLVM") + print_flags clang + ;; +*) + : unknown compiler family + ;; +esac -- 2.17.0.rc1.439.gca064e2955 ^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v3 3/3] Makefile: add EAGER_DEVELOPER mode 2018-03-29 15:03 ` [PATCH v3 0/3] Enable more compiler warnings for devs Nguyễn Thái Ngọc Duy 2018-03-29 15:03 ` [PATCH v3 1/3] connect.c: mark die_initial_contact() NORETURN Nguyễn Thái Ngọc Duy 2018-03-29 15:03 ` [PATCH v3 2/3] Makefile: detect compiler and enable more warnings in DEVELOPER=1 Nguyễn Thái Ngọc Duy @ 2018-03-29 15:03 ` Nguyễn Thái Ngọc Duy 2018-03-31 16:40 ` [PATCH 4/3] Makefile: untangle DEVELOPER and -Werror Ævar Arnfjörð Bjarmason ` (5 subsequent siblings) 8 siblings, 0 replies; 47+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2018-03-29 15:03 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jeff King, ramsay, larsxschneider, Eric Sunshine, Nguyễn Thái Ngọc Duy This mode is for developers who want to keep the code base clean. There are warning classes that are currently suppressed because we have too many of them in the code base. This mode will stop the suppression, let the developer see and decide how to fix them. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- Makefile | 7 ++++++- config.mak.dev | 4 ++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e6680a8977..e4f04ce1cb 100644 --- a/Makefile +++ b/Makefile @@ -434,7 +434,9 @@ all:: # # Define DEVELOPER to enable more compiler warnings. Compiler version # and faimily are auto detected, but could be overridden by defining -# COMPILER_FEATURES (see config.mak.dev) +# COMPILER_FEATURES (see config.mak.dev). +# Define EAGER_DEVELOPER keeps compiler warnings non-fatal, but no warning +# class is suppressed anymore. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -1041,6 +1043,9 @@ include config.mak.uname -include config.mak.autogen -include config.mak +ifdef EAGER_DEVELOPER +DEVELOPER = Yes +endif ifdef DEVELOPER include config.mak.dev endif diff --git a/config.mak.dev b/config.mak.dev index 716a14ecc7..13883410b3 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -1,4 +1,6 @@ +ifndef EAGER_DEVELOPER CFLAGS += -Werror +endif CFLAGS += -Wdeclaration-after-statement CFLAGS += -Wno-format-zero-length CFLAGS += -Wold-style-definition @@ -21,6 +23,7 @@ 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. CFLAGS += -Wmissing-prototypes +ifndef EAGER_DEVELOPER # These are disabled because we have these all over the place. CFLAGS += -Wno-empty-body CFLAGS += -Wno-missing-field-initializers @@ -28,6 +31,7 @@ CFLAGS += -Wno-sign-compare CFLAGS += -Wno-unused-function 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 -- 2.17.0.rc1.439.gca064e2955 ^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH 4/3] Makefile: untangle DEVELOPER and -Werror 2018-03-29 15:03 ` [PATCH v3 0/3] Enable more compiler warnings for devs Nguyễn Thái Ngọc Duy ` (2 preceding siblings ...) 2018-03-29 15:03 ` [PATCH v3 3/3] Makefile: add EAGER_DEVELOPER mode Nguyễn Thái Ngọc Duy @ 2018-03-31 16:40 ` Ævar Arnfjörð Bjarmason 2018-03-31 18:36 ` Duy Nguyen 2018-04-14 19:19 ` [PATCH v4 0/4] Make DEVELOPER more more flexible with DEVOPTS Ævar Arnfjörð Bjarmason ` (4 subsequent siblings) 8 siblings, 1 reply; 47+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2018-03-31 16:40 UTC (permalink / raw) To: git Cc: Junio C Hamano, Jeff King, Nguyễn Thái Ngọc Duy, ramsay, larsxschneider, ric Sunshine, Ævar Arnfjörð Bjarmason Change the DEVELOPER flag, and the newly added EAGER_DEVELOPER flag which (approximately) enables -Wextra so that any combination of them and -Werror can be set. 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 DEVELOPER_NONFATAL is set you'll get the same warnings, but without -Werror. I've renamed the newly added EAGER_DEVELOPER flag to DEVELOPER_EXTRA. The reason is that it approximately turns on -Wextra, and it'll be more consistent to add e.g. DEVELOPER_PEDANTIC later than inventing some new name of our own (VERY_EAGER_DEVELOPER?). The DEVELOPER_EXTRA flag implicitly means DEVELOPER_NONFATAL, but just so that this change doesn't introduce yet another arbitrary limitation it's possible to combine it with DEVELOPER_FATAL, which will turn on -Werror for it as well. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- I really like where this is going, but as noted I think this on top would be even better. Makefile | 17 +++++++++++------ config.mak.dev | 10 ++++++++-- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index e4f04ce1cb..641461a569 100644 --- a/Makefile +++ b/Makefile @@ -432,11 +432,16 @@ all:: # When cross-compiling, define HOST_CPU as the canonical name of the CPU on # which the built Git will run (for instance "x86_64"). # -# Define DEVELOPER to enable more compiler warnings. Compiler version -# and faimily are auto detected, but could be overridden by defining -# COMPILER_FEATURES (see config.mak.dev). -# Define EAGER_DEVELOPER keeps compiler warnings non-fatal, but no warning -# class is suppressed anymore. +# Define DEVELOPER to enable more compiler warnings. We'll also enable +# -Werror unless DEVELOPER_NONFATAL is defined. To enable even more +# pedantic warnings that'll flag some potential existing issues in the +# codebase turn on DEVELOPER_EXTRA, which implicitly sets DEVELOPER as +# well, This is -Wextra with a whitelist of disabled warnings. Unless +# DEVELOPER_NONFATAL is set DEVELOPER_EXTRA will turn it on +# implicitly, so if you for some reason want both DEVELOPER and +# DEVELOPER_EXTRA with fatal warnings, you need to set +# DEVELOPER_FATAL=1 to force -Werror. See config.mak.dev for how this +# all works. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -1043,7 +1048,7 @@ include config.mak.uname -include config.mak.autogen -include config.mak -ifdef EAGER_DEVELOPER +ifdef DEVELOPER_EXTRA DEVELOPER = Yes endif ifdef DEVELOPER diff --git a/config.mak.dev b/config.mak.dev index 13883410b3..40f3365729 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -1,6 +1,12 @@ -ifndef EAGER_DEVELOPER +ifndef DEVELOPER_NONFATAL +ifndef DEVELOPER_EXTRA CFLAGS += -Werror endif +endif +ifdef DEVELOPER_FATAL +CFLAGS += -Werror +endif + CFLAGS += -Wdeclaration-after-statement CFLAGS += -Wno-format-zero-length CFLAGS += -Wold-style-definition @@ -23,7 +29,7 @@ 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. CFLAGS += -Wmissing-prototypes -ifndef EAGER_DEVELOPER +ifndef DEVELOPER_EXTRA # These are disabled because we have these all over the place. CFLAGS += -Wno-empty-body CFLAGS += -Wno-missing-field-initializers -- 2.17.0.rc1.321.gba9d0f2565 ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 4/3] Makefile: untangle DEVELOPER and -Werror 2018-03-31 16:40 ` [PATCH 4/3] Makefile: untangle DEVELOPER and -Werror Ævar Arnfjörð Bjarmason @ 2018-03-31 18:36 ` Duy Nguyen 2018-04-03 9:19 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 47+ messages in thread From: Duy Nguyen @ 2018-03-31 18:36 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Git Mailing List, Junio C Hamano, Jeff King, Ramsay Jones, Lars Schneider, ric Sunshine On Sat, Mar 31, 2018 at 6:40 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > Change the DEVELOPER flag, and the newly added EAGER_DEVELOPER flag > which (approximately) enables -Wextra so that any combination of them > and -Werror can be set. > > 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 DEVELOPER_NONFATAL is set you'll get the same warnings, > but without -Werror. > > I've renamed the newly added EAGER_DEVELOPER flag to > DEVELOPER_EXTRA. The reason is that it approximately turns on -Wextra, > and it'll be more consistent to add e.g. DEVELOPER_PEDANTIC later than > inventing some new name of our own (VERY_EAGER_DEVELOPER?). Before we go with zillions of *DEVELOPER* maybe we can have something like DEVOPTS where you can give multiple keywords to a single variable to influence config.mak.dev. This is similar to COMPILER_FEATURES we already have in there, but now it's driven by the dev instead of the compiler. So you can have keywords like "gentle" (no -Werror) "extra" (-Wextra with no suppression) and something else. -- Duy ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 4/3] Makefile: untangle DEVELOPER and -Werror 2018-03-31 18:36 ` Duy Nguyen @ 2018-04-03 9:19 ` Ævar Arnfjörð Bjarmason 2018-04-03 15:17 ` Duy Nguyen 0 siblings, 1 reply; 47+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2018-04-03 9:19 UTC (permalink / raw) To: Duy Nguyen Cc: Git Mailing List, Junio C Hamano, Jeff King, Ramsay Jones, Lars Schneider, ric Sunshine On Sat, Mar 31 2018, Duy Nguyen wrote: > On Sat, Mar 31, 2018 at 6:40 PM, Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: >> Change the DEVELOPER flag, and the newly added EAGER_DEVELOPER flag >> which (approximately) enables -Wextra so that any combination of them >> and -Werror can be set. >> >> 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 DEVELOPER_NONFATAL is set you'll get the same warnings, >> but without -Werror. >> >> I've renamed the newly added EAGER_DEVELOPER flag to >> DEVELOPER_EXTRA. The reason is that it approximately turns on -Wextra, >> and it'll be more consistent to add e.g. DEVELOPER_PEDANTIC later than >> inventing some new name of our own (VERY_EAGER_DEVELOPER?). > > Before we go with zillions of *DEVELOPER* maybe we can have something > like DEVOPTS where you can give multiple keywords to a single variable > to influence config.mak.dev. This is similar to COMPILER_FEATURES we > already have in there, but now it's driven by the dev instead of the > compiler. So you can have keywords like "gentle" (no -Werror) "extra" > (-Wextra with no suppression) and something else. We could do that, but I don't think it's that bad. This patch is one extra option on top of yours, and it's not going to result in some combinatorial explosion of options, i.e. if we add DEVELOPER_PEDANTIC we'll just add one extra flag. But sure, we could make this some string we'd need to parse out similar to COMPILER_FEATURES, it just seems more complex to me for this task. ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 4/3] Makefile: untangle DEVELOPER and -Werror 2018-04-03 9:19 ` Ævar Arnfjörð Bjarmason @ 2018-04-03 15:17 ` Duy Nguyen 2018-04-06 21:42 ` Jeff King 0 siblings, 1 reply; 47+ messages in thread From: Duy Nguyen @ 2018-04-03 15:17 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Git Mailing List, Junio C Hamano, Jeff King, Ramsay Jones, Lars Schneider, ric Sunshine On Tue, Apr 03, 2018 at 11:19:46AM +0200, Ævar Arnfjörð Bjarmason wrote: > > On Sat, Mar 31 2018, Duy Nguyen wrote: > > > On Sat, Mar 31, 2018 at 6:40 PM, Ævar Arnfjörð Bjarmason > > <avarab@gmail.com> wrote: > >> Change the DEVELOPER flag, and the newly added EAGER_DEVELOPER flag > >> which (approximately) enables -Wextra so that any combination of them > >> and -Werror can be set. > >> > >> 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 DEVELOPER_NONFATAL is set you'll get the same warnings, > >> but without -Werror. > >> > >> I've renamed the newly added EAGER_DEVELOPER flag to > >> DEVELOPER_EXTRA. The reason is that it approximately turns on -Wextra, > >> and it'll be more consistent to add e.g. DEVELOPER_PEDANTIC later than > >> inventing some new name of our own (VERY_EAGER_DEVELOPER?). > > > > Before we go with zillions of *DEVELOPER* maybe we can have something > > like DEVOPTS where you can give multiple keywords to a single variable > > to influence config.mak.dev. This is similar to COMPILER_FEATURES we > > already have in there, but now it's driven by the dev instead of the > > compiler. So you can have keywords like "gentle" (no -Werror) "extra" > > (-Wextra with no suppression) and something else. > > We could do that, but I don't think it's that bad. This patch is one > extra option on top of yours, And that eager this was marked experimental because I was not sure if it was the right way :) > and it's not going to result in some combinatorial explosion of > options, i.e. if we add DEVELOPER_PEDANTIC we'll just add one extra > flag. > > But sure, we could make this some string we'd need to parse out similar > to COMPILER_FEATURES, it just seems more complex to me for this task. It's not that complex. With the EAGER_DEVELOPER patch removed, we can have something like this where eager devs just need to put DEVOPTS = gentle no-suppression and you put DEVOPTS = gentle (bad naming, I didn't spend time thinking about names) -- 8< -- diff --git a/Makefile b/Makefile index e6680a8977..7b4e038e1d 100644 --- a/Makefile +++ b/Makefile @@ -435,6 +435,11 @@ all:: # Define DEVELOPER to enable more compiler warnings. Compiler version # and faimily are auto detected, but could be overridden by defining # COMPILER_FEATURES (see config.mak.dev) +# +# When DEVELOPER is set, DEVOPTS can be used to control compiler options. +# This variable contains keywords separated by whitespace. Two keywords +# are recognized: "gentle" removes -Werror and "no-suppression" +# removes all "-Wno-" options. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN diff --git a/config.mak.dev b/config.mak.dev index 716a14ecc7..1d7aba6a6a 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -1,4 +1,6 @@ +ifeq ($(filter gentle,$(DEVOPTS)),) CFLAGS += -Werror +endif CFLAGS += -Wdeclaration-after-statement CFLAGS += -Wno-format-zero-length CFLAGS += -Wold-style-definition @@ -21,6 +23,7 @@ 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. CFLAGS += -Wmissing-prototypes +ifeq ($(filter no-suppression,$(DEVOPTS)),) # These are disabled because we have these all over the place. CFLAGS += -Wno-empty-body CFLAGS += -Wno-missing-field-initializers @@ -28,6 +31,7 @@ CFLAGS += -Wno-sign-compare CFLAGS += -Wno-unused-function 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 -- 8< -- ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH 4/3] Makefile: untangle DEVELOPER and -Werror 2018-04-03 15:17 ` Duy Nguyen @ 2018-04-06 21:42 ` Jeff King 2018-04-07 9:52 ` Duy Nguyen 0 siblings, 1 reply; 47+ messages in thread From: Jeff King @ 2018-04-06 21:42 UTC (permalink / raw) To: Duy Nguyen Cc: Ævar Arnfjörð Bjarmason, Git Mailing List, Junio C Hamano, Ramsay Jones, Lars Schneider, ric Sunshine On Tue, Apr 03, 2018 at 05:17:00PM +0200, Duy Nguyen wrote: > It's not that complex. With the EAGER_DEVELOPER patch removed, we can > have something like this where eager devs just need to put > > DEVOPTS = gentle no-suppression > > and you put > > DEVOPTS = gentle > > (bad naming, I didn't spend time thinking about names) It seems to me like we're losing the point of DEVELOPER here. I thought the idea was to have a turn-key flag you could set to get extra linting on your commits. But now we're tweaking all kinds of individual options. At some point are we better off just letting you put "-Wno-foo" in your CFLAGS yourself? I don't mind the version-based checks because they're automatic, so the feature remains turn-key. But this kind of DEVOPTS seems like a step in the wrong direction. -Peff ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 4/3] Makefile: untangle DEVELOPER and -Werror 2018-04-06 21:42 ` Jeff King @ 2018-04-07 9:52 ` Duy Nguyen 2018-04-07 12:36 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 47+ messages in thread From: Duy Nguyen @ 2018-04-07 9:52 UTC (permalink / raw) To: Jeff King Cc: Ævar Arnfjörð Bjarmason, Git Mailing List, Junio C Hamano, Ramsay Jones, Lars Schneider, ric Sunshine On Fri, Apr 6, 2018 at 11:42 PM, Jeff King <peff@peff.net> wrote: > On Tue, Apr 03, 2018 at 05:17:00PM +0200, Duy Nguyen wrote: > >> It's not that complex. With the EAGER_DEVELOPER patch removed, we can >> have something like this where eager devs just need to put >> >> DEVOPTS = gentle no-suppression >> >> and you put >> >> DEVOPTS = gentle >> >> (bad naming, I didn't spend time thinking about names) > > It seems to me like we're losing the point of DEVELOPER here. I thought > the idea was to have a turn-key flag you could set to get extra linting > on your commits. But now we're tweaking all kinds of individual options. > At some point are we better off just letting you put "-Wno-foo" in your > CFLAGS yourself? It's because what AEvar wanted is no longer a dev thing :) > I don't mind the version-based checks because they're automatic, so the > feature remains turn-key. But this kind of DEVOPTS seems like a step in > the wrong direction. > > -Peff -- Duy ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 4/3] Makefile: untangle DEVELOPER and -Werror 2018-04-07 9:52 ` Duy Nguyen @ 2018-04-07 12:36 ` Ævar Arnfjörð Bjarmason 2018-04-07 17:03 ` Duy Nguyen 0 siblings, 1 reply; 47+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2018-04-07 12:36 UTC (permalink / raw) To: Duy Nguyen Cc: Jeff King, Git Mailing List, Junio C Hamano, Ramsay Jones, Lars Schneider, ric Sunshine On Sat, Apr 07 2018, Duy Nguyen wrote: > On Fri, Apr 6, 2018 at 11:42 PM, Jeff King <peff@peff.net> wrote: >> On Tue, Apr 03, 2018 at 05:17:00PM +0200, Duy Nguyen wrote: >> >>> It's not that complex. With the EAGER_DEVELOPER patch removed, we can >>> have something like this where eager devs just need to put >>> >>> DEVOPTS = gentle no-suppression >>> >>> and you put >>> >>> DEVOPTS = gentle >>> >>> (bad naming, I didn't spend time thinking about names) >> >> It seems to me like we're losing the point of DEVELOPER here. I thought >> the idea was to have a turn-key flag you could set to get extra linting >> on your commits. But now we're tweaking all kinds of individual options. >> At some point are we better off just letting you put "-Wno-foo" in your >> CFLAGS yourself? > > It's because what AEvar wanted is no longer a dev thing :) It's still a dev thing, but just for a slightly different use-case. I hack my patches up with DEVELOPER=1, and then eventually deploy a patched git version with those (and others) on top of the latest release tag. At that point I want the same compiler assertions, but might be building on much older compilers, so I just want the warning output without -Werror. Anyway, I see you've pushed a new version with DEVOPTS. I'll submit mine on top of that once your new version lands (unless you want to try to integrate it yourself). ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 4/3] Makefile: untangle DEVELOPER and -Werror 2018-04-07 12:36 ` Ævar Arnfjörð Bjarmason @ 2018-04-07 17:03 ` Duy Nguyen 2018-04-07 18:38 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 47+ messages in thread From: Duy Nguyen @ 2018-04-07 17:03 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Jeff King, Git Mailing List, Junio C Hamano, Ramsay Jones, Lars Schneider, ric Sunshine On Sat, Apr 7, 2018 at 2:36 PM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > Anyway, I see you've pushed a new version with DEVOPTS. I'll submit mine > on top of that once your new version lands (unless you want to try to > integrate it yourself). Actually I think I'll just drop both EAGER_DEVELOPER and DEVOTPS. -- Duy ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 4/3] Makefile: untangle DEVELOPER and -Werror 2018-04-07 17:03 ` Duy Nguyen @ 2018-04-07 18:38 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 47+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2018-04-07 18:38 UTC (permalink / raw) To: Duy Nguyen Cc: Jeff King, Git Mailing List, Junio C Hamano, Ramsay Jones, Lars Schneider, ric Sunshine On Sat, Apr 07 2018, Duy Nguyen wrote: > On Sat, Apr 7, 2018 at 2:36 PM, Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: >> Anyway, I see you've pushed a new version with DEVOPTS. I'll submit mine >> on top of that once your new version lands (unless you want to try to >> integrate it yourself). > > Actually I think I'll just drop both EAGER_DEVELOPER and DEVOTPS. OK. I'll submit something on top to optionally get rid of -Werror, since that's the only bit I actually care about & have a use-case for. ^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v4 0/4] Make DEVELOPER more more flexible with DEVOPTS 2018-03-29 15:03 ` [PATCH v3 0/3] Enable more compiler warnings for devs Nguyễn Thái Ngọc Duy ` (3 preceding siblings ...) 2018-03-31 16:40 ` [PATCH 4/3] Makefile: untangle DEVELOPER and -Werror Ævar Arnfjörð Bjarmason @ 2018-04-14 19:19 ` Ævar Arnfjörð Bjarmason 2018-04-16 4:57 ` Junio C Hamano 2018-04-14 19:19 ` [PATCH v4 1/4] connect.c: mark die_initial_contact() NORETURN Ævar Arnfjörð Bjarmason ` (3 subsequent siblings) 8 siblings, 1 reply; 47+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2018-04-14 19:19 UTC (permalink / raw) To: git Cc: Junio C Hamano, Ramsay Jones, Jeff King, larsxschneider, Eric Sunshine, Nguyễn Thái Ngọc Duy, Ævar Arnfjörð Bjarmason This is a v4 and replacement of gitster/nd/warn-more-for-devs. I'm sending this with Duy's blessing. The first two patches are the same, except for one trivial s/faimily/family/ typo fix. The third patch in gitster/nd/warn-more-for-devs ("Makefile: add EAGER_DEVELOPER mode") is gone, instead there's now a DEVOPTS option. The 3/4 and 4/4 add a way to turn off -Werror & the -Wextra suppression, respectively. Duy was right in [1] that this is a much better and extensible way of doing this than my "Makefile: untangle DEVELOPER and -Werror" patch. Most of 3/4 & 4/4 are just tweaked from git@github.com:pclouds/git.git pclouds/more-warnings and combined with my previous 4/3 patch[2]. I changed the "no-suppression" name in Duy's WIP patch to "extra-all", and "gentle" to "no-error". I think those are cleare,r and leave things more open to future expansion, e.g. if we'd like pedantic-all. 1. https://public-inbox.org/git/CACsJy8CyB0igY365NMkswSgAi9_rf+XBOMQyJ7XW6iQxQiCEyQ@mail.gmail.com/ 2. https://public-inbox.org/git/20180331164009.2264-1-avarab@gmail.com/ Nguyễn Thái Ngọc Duy (2): connect.c: mark die_initial_contact() NORETURN Makefile: detect compiler and enable more warnings in DEVELOPER=1 Ævar Arnfjörð Bjarmason (2): Makefile: add a DEVOPTS to suppress -Werror under DEVELOPER Makefile: add a DEVOPTS to get all of -Wextra Makefile | 31 +++++++++++++++++++---------- config.mak.dev | 42 +++++++++++++++++++++++++++++++++++++++ connect.c | 2 +- detect-compiler | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 11 deletions(-) create mode 100644 config.mak.dev create mode 100755 detect-compiler -- 2.17.0.290.gded63e768a ^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH v4 0/4] Make DEVELOPER more more flexible with DEVOPTS 2018-04-14 19:19 ` [PATCH v4 0/4] Make DEVELOPER more more flexible with DEVOPTS Ævar Arnfjörð Bjarmason @ 2018-04-16 4:57 ` Junio C Hamano 0 siblings, 0 replies; 47+ messages in thread From: Junio C Hamano @ 2018-04-16 4:57 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: git, Ramsay Jones, Jeff King, larsxschneider, Eric Sunshine, Nguyễn Thái Ngọc Duy Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > This is a v4 and replacement of gitster/nd/warn-more-for-devs. I'm > sending this with Duy's blessing. > > The first two patches are the same, except for one trivial > s/faimily/family/ typo fix. > > The third patch in gitster/nd/warn-more-for-devs ("Makefile: add > EAGER_DEVELOPER mode") is gone, instead there's now a DEVOPTS > option. The 3/4 and 4/4 add a way to turn off -Werror & the -Wextra > suppression, respectively. Looks sensible, mostly. Will queue. Thanks. ^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH v4 1/4] connect.c: mark die_initial_contact() NORETURN 2018-03-29 15:03 ` [PATCH v3 0/3] Enable more compiler warnings for devs Nguyễn Thái Ngọc Duy ` (4 preceding siblings ...) 2018-04-14 19:19 ` [PATCH v4 0/4] Make DEVELOPER more more flexible with DEVOPTS Ævar Arnfjörð Bjarmason @ 2018-04-14 19:19 ` Ævar Arnfjörð Bjarmason 2018-04-14 19:19 ` [PATCH v4 2/4] Makefile: detect compiler and enable more warnings in DEVELOPER=1 Ævar Arnfjörð Bjarmason ` (2 subsequent siblings) 8 siblings, 0 replies; 47+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2018-04-14 19:19 UTC (permalink / raw) To: git Cc: Junio C Hamano, Ramsay Jones, Jeff King, larsxschneider, Eric Sunshine, Nguyễn Thái Ngọc Duy From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> There is a series running in parallel with this one that adds code like this switch (...) { case ...: die_initial_contact(); case ...: There is nothing wrong with this. There is no actual falling through. But since gcc is not that smart and gcc 7.x introduces -Wimplicit-fallthrough, it raises a false alarm in this case. This class of warnings may be useful elsewhere, so instead of suppressing the whole class, let's try to fix just this code. gcc is smart enough to realize that no execution can continue after a NORETURN function call and no longer raises the warning. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- connect.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/connect.c b/connect.c index c3a014c5ba..49eca46462 100644 --- a/connect.c +++ b/connect.c @@ -46,7 +46,7 @@ int check_ref_type(const struct ref *ref, int flags) return check_ref(ref->name, flags); } -static void die_initial_contact(int unexpected) +static NORETURN void die_initial_contact(int unexpected) { if (unexpected) die(_("The remote end hung up upon initial contact")); -- 2.17.0.290.gded63e768a ^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v4 2/4] Makefile: detect compiler and enable more warnings in DEVELOPER=1 2018-03-29 15:03 ` [PATCH v3 0/3] Enable more compiler warnings for devs Nguyễn Thái Ngọc Duy ` (5 preceding siblings ...) 2018-04-14 19:19 ` [PATCH v4 1/4] connect.c: mark die_initial_contact() NORETURN Ævar Arnfjörð Bjarmason @ 2018-04-14 19:19 ` Ævar Arnfjörð Bjarmason 2018-04-14 19:19 ` [PATCH v4 3/4] Makefile: add a DEVOPTS to suppress -Werror under DEVELOPER Ævar Arnfjörð Bjarmason 2018-04-14 19:19 ` [PATCH v4 4/4] Makefile: add a DEVOPTS to get all of -Wextra Ævar Arnfjörð Bjarmason 8 siblings, 0 replies; 47+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2018-04-14 19:19 UTC (permalink / raw) To: git Cc: Junio C Hamano, Ramsay Jones, Jeff King, larsxschneider, Eric Sunshine, Nguyễn Thái Ngọc Duy From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> 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> --- Makefile | 15 +++++--------- config.mak.dev | 38 +++++++++++++++++++++++++++++++++++ detect-compiler | 53 +++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 96 insertions(+), 10 deletions(-) create mode 100644 config.mak.dev create mode 100755 detect-compiler diff --git a/Makefile b/Makefile index f181687250..3038c78325 100644 --- a/Makefile +++ b/Makefile @@ -441,6 +441,10 @@ all:: # # When cross-compiling, define HOST_CPU as the canonical name of the CPU on # which the built Git will run (for instance "x86_64"). +# +# Define DEVELOPER to enable more compiler warnings. Compiler version +# and family are auto detected, but could be overridden by defining +# COMPILER_FEATURES (see config.mak.dev) GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -449,15 +453,6 @@ GIT-VERSION-FILE: FORCE # CFLAGS and LDFLAGS are for the users to override from the command line. CFLAGS = -g -O2 -Wall -DEVELOPER_CFLAGS = -Werror \ - -Wdeclaration-after-statement \ - -Wno-format-zero-length \ - -Wold-style-definition \ - -Woverflow \ - -Wpointer-arith \ - -Wstrict-prototypes \ - -Wunused \ - -Wvla LDFLAGS = ALL_CFLAGS = $(CPPFLAGS) $(CFLAGS) ALL_LDFLAGS = $(LDFLAGS) @@ -1062,7 +1057,7 @@ include config.mak.uname -include config.mak ifdef DEVELOPER -CFLAGS += $(DEVELOPER_CFLAGS) +include config.mak.dev endif comma := , diff --git a/config.mak.dev b/config.mak.dev new file mode 100644 index 0000000000..716a14ecc7 --- /dev/null +++ b/config.mak.dev @@ -0,0 +1,38 @@ +CFLAGS += -Werror +CFLAGS += -Wdeclaration-after-statement +CFLAGS += -Wno-format-zero-length +CFLAGS += -Wold-style-definition +CFLAGS += -Woverflow +CFLAGS += -Wpointer-arith +CFLAGS += -Wstrict-prototypes +CFLAGS += -Wunused +CFLAGS += -Wvla + +ifndef COMPILER_FEATURES +COMPILER_FEATURES := $(shell ./detect-compiler $(CC)) +endif + +ifneq ($(filter clang4,$(COMPILER_FEATURES)),) +CFLAGS += -Wtautological-constant-out-of-range-compare +endif + +ifneq ($(or $(filter gcc6,$(COMPILER_FEATURES)),$(filter clang4,$(COMPILER_FEATURES))),) +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. +CFLAGS += -Wmissing-prototypes +# These are disabled because we have these all over the place. +CFLAGS += -Wno-empty-body +CFLAGS += -Wno-missing-field-initializers +CFLAGS += -Wno-sign-compare +CFLAGS += -Wno-unused-function +CFLAGS += -Wno-unused-parameter +endif + +# uninitialized warnings on gcc 4.9.2 in xdiff/xdiffi.c and config.c +# not worth fixing since newer compilers correctly stop complaining +ifneq ($(filter gcc4,$(COMPILER_FEATURES)),) +ifeq ($(filter gcc5,$(COMPILER_FEATURES)),) +CFLAGS += -Wno-uninitialized +endif +endif diff --git a/detect-compiler b/detect-compiler new file mode 100755 index 0000000000..70b754481c --- /dev/null +++ b/detect-compiler @@ -0,0 +1,53 @@ +#!/bin/sh +# +# Probe the compiler for vintage, version, etc. This is used for setting +# optional make knobs under the DEVELOPER knob. + +CC="$*" + +# we get something like (this is at least true for gcc and clang) +# +# FreeBSD clang version 3.4.1 (tags/RELEASE...) +get_version_line() { + $CC -v 2>&1 | grep ' version ' +} + +get_family() { + get_version_line | sed 's/^\(.*\) version [0-9][^ ]* .*/\1/' +} + +get_version() { + get_version_line | sed 's/^.* version \([0-9][^ ]*\) .*/\1/' +} + +print_flags() { + family=$1 + version=$(get_version | cut -f 1 -d .) + + # Print a feature flag not only for the current version, but also + # for any prior versions we encompass. This avoids needing to do + # numeric comparisons in make, which are awkward. + while test "$version" -gt 0 + do + echo $family$version + version=$((version - 1)) + done +} + +case "$(get_family)" in +gcc) + print_flags gcc + ;; +clang) + print_flags clang + ;; +"FreeBSD clang") + print_flags clang + ;; +"Apple LLVM") + print_flags clang + ;; +*) + : unknown compiler family + ;; +esac -- 2.17.0.290.gded63e768a ^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v4 3/4] Makefile: add a DEVOPTS to suppress -Werror under DEVELOPER 2018-03-29 15:03 ` [PATCH v3 0/3] Enable more compiler warnings for devs Nguyễn Thái Ngọc Duy ` (6 preceding siblings ...) 2018-04-14 19:19 ` [PATCH v4 2/4] Makefile: detect compiler and enable more warnings in DEVELOPER=1 Ævar Arnfjörð Bjarmason @ 2018-04-14 19:19 ` Ævar Arnfjörð Bjarmason 2018-04-14 19:19 ` [PATCH v4 4/4] Makefile: add a DEVOPTS to get all of -Wextra Ævar Arnfjörð Bjarmason 8 siblings, 0 replies; 47+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2018-04-14 19:19 UTC (permalink / raw) To: git Cc: Junio C Hamano, Ramsay Jones, Jeff King, larsxschneider, Eric Sunshine, Nguyễn Thái Ngọc Duy, Ævar Arnfjörð Bjarmason 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> --- Makefile | 10 ++++++++++ config.mak.dev | 2 ++ 2 files changed, 12 insertions(+) diff --git a/Makefile b/Makefile index 3038c78325..9a57495fae 100644 --- a/Makefile +++ b/Makefile @@ -445,6 +445,16 @@ all:: # Define DEVELOPER to enable more compiler warnings. Compiler version # and family are auto detected, but could be overridden by defining # COMPILER_FEATURES (see config.mak.dev) +# +# When DEVELOPER is set, DEVOPTS can be used to control compiler +# options. This variable contains keywords separated by +# whitespace. The following keywords are are recognized: +# +# no-error: +# +# suppresses the -Werror that implicitly comes with +# DEVELOPER=1. Useful for getting the full set of errors +# without immediately dying, or for logging them. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN diff --git a/config.mak.dev b/config.mak.dev index 716a14ecc7..7a54426d78 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -1,4 +1,6 @@ +ifeq ($(filter no-error,$(DEVOPTS)),) CFLAGS += -Werror +endif CFLAGS += -Wdeclaration-after-statement CFLAGS += -Wno-format-zero-length CFLAGS += -Wold-style-definition -- 2.17.0.290.gded63e768a ^ permalink raw reply related [flat|nested] 47+ messages in thread
* [PATCH v4 4/4] Makefile: add a DEVOPTS to get all of -Wextra 2018-03-29 15:03 ` [PATCH v3 0/3] Enable more compiler warnings for devs Nguyễn Thái Ngọc Duy ` (7 preceding siblings ...) 2018-04-14 19:19 ` [PATCH v4 3/4] Makefile: add a DEVOPTS to suppress -Werror under DEVELOPER Ævar Arnfjörð Bjarmason @ 2018-04-14 19:19 ` Ævar Arnfjörð Bjarmason 8 siblings, 0 replies; 47+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2018-04-14 19:19 UTC (permalink / raw) To: git Cc: Junio C Hamano, Ramsay Jones, Jeff King, larsxschneider, Eric Sunshine, Nguyễn Thái Ngọc Duy, Ævar Arnfjörð Bjarmason 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> --- Makefile | 6 ++++++ config.mak.dev | 2 ++ 2 files changed, 8 insertions(+) diff --git a/Makefile b/Makefile index 9a57495fae..47ddc85aae 100644 --- a/Makefile +++ b/Makefile @@ -455,6 +455,12 @@ all:: # suppresses the -Werror that implicitly comes with # DEVELOPER=1. Useful for getting the full set of errors # without immediately dying, or for logging them. +# +# extra-all: +# +# The DEVELOPER mode enables -Wextra with a few exceptions. By +# setting this flag the exceptions are removed, and all of +# -Wextra is used. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN diff --git a/config.mak.dev b/config.mak.dev index 7a54426d78..2d244ca470 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -23,6 +23,7 @@ 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. CFLAGS += -Wmissing-prototypes +ifeq ($(filter extra-all,$(DEVOPTS)),) # These are disabled because we have these all over the place. CFLAGS += -Wno-empty-body CFLAGS += -Wno-missing-field-initializers @@ -30,6 +31,7 @@ CFLAGS += -Wno-sign-compare CFLAGS += -Wno-unused-function 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 -- 2.17.0.290.gded63e768a ^ permalink raw reply related [flat|nested] 47+ messages in thread
* Re: [PATCH v2] travis-ci: enable more warnings on travis linux-gcc job 2018-03-17 14:29 ` Lars Schneider 2018-03-17 14:59 ` Duy Nguyen @ 2018-03-17 15:16 ` Jeff King 1 sibling, 0 replies; 47+ messages in thread From: Jeff King @ 2018-03-17 15:16 UTC (permalink / raw) To: Lars Schneider; +Cc: Duy Nguyen, Git Mailing List On Sat, Mar 17, 2018 at 03:29:31PM +0100, Lars Schneider wrote: > >> Why isn't this just turning on DEVELOPER=1 if we know we have a capable > >> compiler? > > > > DEVELOPER=1 is always set even before this patch. It's set and > > exported in lib-travisci.sh. > > I interpreted Peff's comment like this: > > If DEVELOPER=1 is set and we detect a gcc-6 in the makefile, > then we could set your additional flags in the makefile. > > This way every developer with a new compiler would run these > flags locally (if DEVELOPER=1 is set). Actually, I was mostly just confused, and didn't realize that these were going above and beyond what's in DEVELOPER. But that said, now that I understand, I agree completely with your suggestion. :) -Peff ^ permalink raw reply [flat|nested] 47+ messages in thread
end of thread, other threads:[~2018-04-16 4:57 UTC | newest] Thread overview: 47+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-03 1:46 [PATCH] travis-ci: enable more warnings on travis linux-gcc job Nguyễn Thái Ngọc Duy 2018-03-16 19:33 ` [PATCH v2] " Nguyễn Thái Ngọc Duy 2018-03-16 21:22 ` Jeff King 2018-03-17 8:01 ` Duy Nguyen 2018-03-17 14:29 ` Lars Schneider 2018-03-17 14:59 ` Duy Nguyen 2018-03-17 16:08 ` Jeff King 2018-03-17 16:12 ` Jeff King 2018-03-17 23:50 ` Junio C Hamano 2018-03-18 8:18 ` [PATCH] Makefile: detect compiler and enable more warnings in DEVELOPER=1 Nguyễn Thái Ngọc Duy 2018-03-18 9:06 ` Eric Sunshine 2018-03-18 9:17 ` Duy Nguyen 2018-03-18 9:20 ` Jeff King 2018-03-18 9:24 ` Eric Sunshine 2018-03-18 9:28 ` Jeff King 2018-03-18 9:37 ` Eric Sunshine 2018-03-18 9:26 ` Jeff King 2018-03-18 9:45 ` Duy Nguyen 2018-03-18 15:55 ` Duy Nguyen 2018-03-18 18:56 ` Ramsay Jones 2018-03-19 16:30 ` Duy Nguyen 2018-03-20 5:32 ` Jeff King 2018-03-24 12:53 ` [PATCH v2] " Nguyễn Thái Ngọc Duy 2018-03-25 0:40 ` Eric Sunshine 2018-03-26 22:02 ` Junio C Hamano 2018-03-27 15:03 ` Duy Nguyen 2018-03-27 16:52 ` Junio C Hamano 2018-03-29 15:03 ` [PATCH v3 0/3] Enable more compiler warnings for devs Nguyễn Thái Ngọc Duy 2018-03-29 15:03 ` [PATCH v3 1/3] connect.c: mark die_initial_contact() NORETURN Nguyễn Thái Ngọc Duy 2018-03-29 15:03 ` [PATCH v3 2/3] Makefile: detect compiler and enable more warnings in DEVELOPER=1 Nguyễn Thái Ngọc Duy 2018-03-29 15:03 ` [PATCH v3 3/3] Makefile: add EAGER_DEVELOPER mode Nguyễn Thái Ngọc Duy 2018-03-31 16:40 ` [PATCH 4/3] Makefile: untangle DEVELOPER and -Werror Ævar Arnfjörð Bjarmason 2018-03-31 18:36 ` Duy Nguyen 2018-04-03 9:19 ` Ævar Arnfjörð Bjarmason 2018-04-03 15:17 ` Duy Nguyen 2018-04-06 21:42 ` Jeff King 2018-04-07 9:52 ` Duy Nguyen 2018-04-07 12:36 ` Ævar Arnfjörð Bjarmason 2018-04-07 17:03 ` Duy Nguyen 2018-04-07 18:38 ` Ævar Arnfjörð Bjarmason 2018-04-14 19:19 ` [PATCH v4 0/4] Make DEVELOPER more more flexible with DEVOPTS Ævar Arnfjörð Bjarmason 2018-04-16 4:57 ` Junio C Hamano 2018-04-14 19:19 ` [PATCH v4 1/4] connect.c: mark die_initial_contact() NORETURN Ævar Arnfjörð Bjarmason 2018-04-14 19:19 ` [PATCH v4 2/4] Makefile: detect compiler and enable more warnings in DEVELOPER=1 Ævar Arnfjörð Bjarmason 2018-04-14 19:19 ` [PATCH v4 3/4] Makefile: add a DEVOPTS to suppress -Werror under DEVELOPER Ævar Arnfjörð Bjarmason 2018-04-14 19:19 ` [PATCH v4 4/4] Makefile: add a DEVOPTS to get all of -Wextra Ævar Arnfjörð Bjarmason 2018-03-17 15:16 ` [PATCH v2] travis-ci: enable more warnings on travis linux-gcc job Jeff King
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).