| Message ID | 20210809013833.58110-4-carenas@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pedantic errors in next | expand |
On 09/08/21 08.38, Carlo Marcelo Arenas Belón wrote: > similar to the recently added sparse task, it is nice to know as early > as possible. > > add a dockerized build using fedora (that usually has the latest gcc) > to be ahead of the curve and avoid older ISO C issues at the same time. > But from GCC manual [1], the default C dialect used is `-std=gnu17`, while `-pedantic` is only relevant for ISO C (such as `-std=c17`). And why not using `-pedantic-errors`, so that non-ISO features are treated as errors? Newcomers contributing to Git may think that based on what our CI do, they can submit patches with C17 features (perhaps with GNU extensions). Then at some time there is casual users that complain that Git doesn't compile with their default older compiler (maybe they run LTS distributions or pre-C17 compiler). Thus we want Git to be compiled successfully using wide variety of compilers (maybe as old as GCC 4.8). [1]: https://gcc.gnu.org/onlinedocs/gcc-11.2.0/gcc/Standards.html#Standards
Hi Carlo On 09/08/2021 02:38, Carlo Marcelo Arenas Belón wrote: > similar to the recently added sparse task, it is nice to know as early > as possible. > > add a dockerized build using fedora (that usually has the latest gcc) > to be ahead of the curve and avoid older ISO C issues at the same time. If we want to be able to compile with -Wpedantic then it might be better just to turn it on unconditionally in config.mak.dev. Then developers will see any errors before they push and the ci builds will all use it rather than having to run an extra job. I had a quick scan of the mail archive threads starting at [1,2] and it's not clear to me why -Wpedaintic was added as an optional extra. Totally unrelated to this patch but while looking at the ci scripts I noticed that we only run the linux-gcc-4.8 job on travis, not on github. Best Wishes Phillip [1] https://lore.kernel.org/git/20180721185933.32377-1-dev+git@drbeat.li/ [2] https://lore.kernel.org/git/20180721203647.2619-1-dev+git@drbeat.li/ > Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> > --- > .github/workflows/main.yml | 2 ++ > ci/install-docker-dependencies.sh | 4 ++++ > ci/run-build-and-tests.sh | 10 +++++++--- > 3 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml > index 47876a4f02..6b9427eff1 100644 > --- a/.github/workflows/main.yml > +++ b/.github/workflows/main.yml > @@ -259,6 +259,8 @@ jobs: > image: alpine > - jobname: Linux32 > image: daald/ubuntu32:xenial > + - jobname: pedantic > + image: fedora > env: > jobname: ${{matrix.vector.jobname}} > runs-on: ubuntu-latest > diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh > index 26a6689766..07a8c6b199 100755 > --- a/ci/install-docker-dependencies.sh > +++ b/ci/install-docker-dependencies.sh > @@ -15,4 +15,8 @@ linux-musl) > apk add --update build-base curl-dev openssl-dev expat-dev gettext \ > pcre2-dev python3 musl-libintl perl-utils ncurses >/dev/null > ;; > +pedantic) > + dnf -yq update >/dev/null && > + dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null > + ;; > esac > diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh > index 3ce81ffee9..f3aba5d6cb 100755 > --- a/ci/run-build-and-tests.sh > +++ b/ci/run-build-and-tests.sh > @@ -10,6 +10,11 @@ windows*) cmd //c mklink //j t\\.prove "$(cygpath -aw "$cache_dir/.prove")";; > *) ln -s "$cache_dir/.prove" t/.prove;; > esac > > +if test "$jobname" = "pedantic" > +then > + export DEVOPTS=pedantic > +fi > + > make > case "$jobname" in > linux-gcc) > @@ -35,10 +40,9 @@ linux-clang) > export GIT_TEST_DEFAULT_HASH=sha256 > make test > ;; > -linux-gcc-4.8) > +linux-gcc-4.8|pedantic) > # Don't run the tests; we only care about whether Git can be > - # built with GCC 4.8, as it errors out on some undesired (C99) > - # constructs that newer compilers seem to quietly accept. > + # built with GCC 4.8 or with pedantic > ;; > *) > make test >
On Mon, Aug 9, 2021 at 3:50 AM Bagas Sanjaya <bagasdotme@gmail.com> wrote: > > On 09/08/21 08.38, Carlo Marcelo Arenas Belón wrote: > > add a dockerized build using fedora (that usually has the latest gcc) > > to be ahead of the curve and avoid older ISO C issues at the same time. > > But from GCC manual [1], the default C dialect used is `-std=gnu17`, > while `-pedantic` is only relevant for ISO C (such as `-std=c17`). sorry about that, my comment was confusing I only meant to imply that newer compilers were not throwing any more warnings than the ones that were fixed unlike what you would get if using older compilers or targeting an older standard. This implies that it will likely not have many false positives and the few breaks that would come with newer compiled might be worth investigating or adding to the ignore list. a strict C89 compiler won't even build (ex: inline is a gnu extension and the codebase has been adding those officially since fe9dc6b08c (Merge branch 'jc/post-c89-rules-doc', 2019-07-25)) and so the pedantic check implied you would target at least gnu89 and generate lots of warnings (so don't expect to build with DEVELOPER=1 that adds -Werror) are you suggesting we need a more aggresive target like strict C99? at least gcc 11.2.0 seems to be able to still build next without warnings. > And why not using `-pedantic-errors`, so that non-ISO features are > treated as errors? warnings are already treated as errors, if you want to see all warnings need DEVOPTS="no-error pedantic" > Newcomers contributing to Git may think that based on what our CI do, > they can submit patches with C17 features (perhaps with GNU extensions). > Then at some time there is casual users that complain that Git doesn't > compile with their default older compiler (maybe they run LTS > distributions or pre-C17 compiler). Thus we want Git to be compiled > successfully using wide variety of compilers (maybe as old as GCC 4.8). the codebase was meant to be C89 compatible (as described in Documentation/CodingGuidelines). gcc-4 is a good target because AFAIK was the last one that defaulted to gnu89 mode and was also used as the system compiler for several really old systems that still have support. I tested with 4.9.4, which was the oldest I could get a hold off from gcc's docker hub, but I suspect will work the same in that old gcc from centos or debian as well. Carlo
On Mon, Aug 9, 2021 at 7:56 AM Phillip Wood <phillip.wood123@gmail.com> wrote: > > Totally unrelated to this patch but while looking at the ci scripts I > noticed that we only run the linux-gcc-4.8 job on travis, not on github. it is actually related and part of the reason why I sent this as an RFC. travis[1] itself is not running, probably because it broke when travis-ci.org was shutdown some time ago. maybe wasn't as useful as a CI job using valuable CPU minutes when it could run in the development environment before the code was submitted? the same could apply to this request if you consider that unlike the other similar jobs (ex: sparse or "Static Analysis") there is no need to install an additional (probably tricky to get tool) Carlo [1] https://travis-ci.com/github/git
On 09/08/2021 23:48, Carlo Arenas wrote: > On Mon, Aug 9, 2021 at 7:56 AM Phillip Wood <phillip.wood123@gmail.com> wrote: >> >> Totally unrelated to this patch but while looking at the ci scripts I >> noticed that we only run the linux-gcc-4.8 job on travis, not on github. > > it is actually related and part of the reason why I sent this as an RFC. > travis[1] itself is not running, probably because it broke when > travis-ci.org was > shutdown some time ago. > > maybe wasn't as useful as a CI job using valuable CPU minutes when it could > run in the development environment before the code was submitted? the same > could apply to this request if you consider that unlike the other > similar jobs (ex: sparse or "Static Analysis") > there is no need to install an additional (probably tricky to get tool) I think there is value in running the CI jobs with -Wpedantic otherwise we'll continually fixing patches up after they've been merged, I just wonder if we need a separate job to do it. We could export DEVOPTS=pedantic in ci/build-and-run-tests.sh or change config.mak.dev to turn on -Wpedantic with DEVELOPER=1. Having said all that your commit message also mentioned using a recent compiler to pick up any problem early, I'm not sure how common that is but perhaps that makes a new job worth it. If so there is a gcc docker image[1] which always has the latest compiler. Best Wishes Phillip [1] https://hub.docker.com/_/gcc > Carlo > > [1] https://travis-ci.com/github/git >
Phillip Wood <phillip.wood123@gmail.com> writes: > I think there is value in running the CI jobs with -Wpedantic > otherwise we'll continually fixing patches up after they've been > merged, I just wonder if we need a separate job to do it. Seeing https://github.com/git/git/actions/runs/1114402527 for example, I notice that we run three dockerized jobs and this one takes about 3 minutes to compile everything. The other two take about 9 minutes and compile and run tests. Shouldn't we be running tests in this job, too? I know the new comment in ci/run-build-and-tests.sh says "Don't run the tests; we only care about ...", but I do not see a reason to declare that we do not care if the resulting binary passes the tests or not. It also seems that the environment does not have an access to any working "git" binary and "save_good_tree" helper seems to fail because of it. https://github.com/git/git/runs/3284998605?check_suite_focus=true#step:5:692 I wonder if this untested patch on top of the patch under discussion is sufficient, if we wanted to also run tests on the fedora image? diff --git c/ci/install-docker-dependencies.sh w/ci/install-docker-dependencies.sh index 07a8c6b199..f139c0632b 100755 --- c/ci/install-docker-dependencies.sh +++ w/ci/install-docker-dependencies.sh @@ -17,6 +17,8 @@ linux-musl) ;; pedantic) dnf -yq update >/dev/null && - dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null + dnf -yq install git make gcc findutils diffutils perl python3 \ + gettext zlib-devel expat-devel openssl-devel \ + curl-devel pcre2-devel >/dev/null ;; esac diff --git c/ci/run-build-and-tests.sh w/ci/run-build-and-tests.sh index f3aba5d6cb..5b2fcf3428 100755 --- c/ci/run-build-and-tests.sh +++ w/ci/run-build-and-tests.sh @@ -40,9 +40,10 @@ linux-clang) export GIT_TEST_DEFAULT_HASH=sha256 make test ;; -linux-gcc-4.8|pedantic) +linux-gcc-4.8) # Don't run the tests; we only care about whether Git can be - # built with GCC 4.8 or with pedantic + # built with GCC 4.8, as it errors out on some undesired (C99) + # constructs that newer compilers seem to quietly accept. ;; *) make test
On Mon, Aug 09 2021, Phillip Wood wrote: > Hi Carlo > > On 09/08/2021 02:38, Carlo Marcelo Arenas Belón wrote: >> similar to the recently added sparse task, it is nice to know as early >> as possible. >> add a dockerized build using fedora (that usually has the latest >> gcc) >> to be ahead of the curve and avoid older ISO C issues at the same time. > > If we want to be able to compile with -Wpedantic then it might be > better just to turn it on unconditionally in config.mak.dev. Then > developers will see any errors before they push and the ci builds will > all use it rather than having to run an extra job. I had a quick scan > of the mail archive threads starting at [1,2] and it's not clear to me > why -Wpedaintic was added as an optional extra. This is from wetware memory, so maybe it's wrong: But I recall that with DEVOPTS=pedantic we used to have a giant wall of warnings not too long ago (i.e. 1-3 years), and not just that referenced USE_PARENS_AROUND_GETTEXT_N issue. So yeah, I take and agree with your point that perhaps we should turn this on by default for DEVELOPER if that's not the case. On the other hand we can't combine that with USE_PARENS_AROUND_GETTEXT_N, and to the extent that we think DEVELOPER is useful, the entire point of having USE_PARENS_AROUND_GETTEXT_N seems to be to catch exactly that sort of in-development issue. So if we turn pedantic on in DEVOPTS by default, wouldn't it make sense to at least have a CI job where we test that we compile with USE_PARENS_AROUND_GETTEXT_N (which at that point would no be the default anymore). Or maybe the existing CI config matrix would already cover that, i.e. we've got some entry point to it that doesn't go through ci/lib.sh's DEVELOPER=1 that I've missed, if so nevermind the last two paragraphs (three, including this one).
On Sun, Aug 08 2021, Carlo Marcelo Arenas Belón wrote: > -linux-gcc-4.8) > +linux-gcc-4.8|pedantic) > # Don't run the tests; we only care about whether Git can be > - # built with GCC 4.8, as it errors out on some undesired (C99) > - # constructs that newer compilers seem to quietly accept. > + # built with GCC 4.8 or with pedantic > ;; > *) > make test Aside from Junio's suggested squash in <xmqqeeb1dumx.fsf@gitster.g> downthread, which would obsolete this comment: I think this would be clearer by not combining these two, i.e. just: linux-gcc-4.8) # <existing comment about that setup> ;; pedantic) # <A new comment, or not> ;; We'll surely eventually end up with not just one, but N setups that want to compile-only, so not having to reword one big comment referring to them all when we do so leads to less churn...
On Mon, Aug 30, 2021 at 4:40 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > On Mon, Aug 09 2021, Phillip Wood wrote: > > On 09/08/2021 02:38, Carlo Marcelo Arenas Belón wrote: > >> similar to the recently added sparse task, it is nice to know as early > >> as possible. > >> add a dockerized build using fedora (that usually has the latest > >> gcc) > >> to be ahead of the curve and avoid older ISO C issues at the same time. > > > > If we want to be able to compile with -Wpedantic then it might be > > better just to turn it on unconditionally in config.mak.dev. Then > > developers will see any errors before they push and the ci builds will > > all use it rather than having to run an extra job. I had a quick scan > > of the mail archive threads starting at [1,2] and it's not clear to me > > why -Wpedaintic was added as an optional extra. > > This is from wetware memory, so maybe it's wrong: But I recall that with > DEVOPTS=pedantic we used to have a giant wall of warnings not too long > ago (i.e. 1-3 years), and not just that referenced > USE_PARENS_AROUND_GETTEXT_N issue. when gcc (and clang) moved to target C99 by default (after version 5) then that wall of errors went away. Indeed git can build cleanly in a strict C99 compiler and until reftable was able to build even with gcc 2.95.3 the nostalgic can get it back with `CC=gcc -std=gnu89`, and indeed I was considering this might be a good alternative to the defunct gcc-4.8 job, where the weather balloons breaking with strict C89 compatibility could be explicitly coded. > So if we turn pedantic on in DEVOPTS by default, wouldn't it make sense > to at least have a CI job where we test that we compile with > USE_PARENS_AROUND_GETTEXT_N (which at that point would not be the default > anymore). agree, and indeed was thinking it might be worth combining this job with the SANITIZE one for efficiency. Carlo
On Tue, Aug 31 2021, Carlo Arenas wrote: > On Mon, Aug 30, 2021 at 4:40 AM Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: >> On Mon, Aug 09 2021, Phillip Wood wrote: >> > On 09/08/2021 02:38, Carlo Marcelo Arenas Belón wrote: >> >> similar to the recently added sparse task, it is nice to know as early >> >> as possible. >> >> add a dockerized build using fedora (that usually has the latest >> >> gcc) >> >> to be ahead of the curve and avoid older ISO C issues at the same time. >> > >> > If we want to be able to compile with -Wpedantic then it might be >> > better just to turn it on unconditionally in config.mak.dev. Then >> > developers will see any errors before they push and the ci builds will >> > all use it rather than having to run an extra job. I had a quick scan >> > of the mail archive threads starting at [1,2] and it's not clear to me >> > why -Wpedaintic was added as an optional extra. >> >> This is from wetware memory, so maybe it's wrong: But I recall that with >> DEVOPTS=pedantic we used to have a giant wall of warnings not too long >> ago (i.e. 1-3 years), and not just that referenced >> USE_PARENS_AROUND_GETTEXT_N issue. > > when gcc (and clang) moved to target C99 by default (after version 5) > then that wall of errors went away. Indeed git can build cleanly in a > strict C99 compiler and until reftable was able to build even with gcc > 2.95.3 > > the nostalgic can get it back with `CC=gcc -std=gnu89`, and indeed I > was considering this might be a good alternative to the defunct > gcc-4.8 job, where the weather balloons breaking with strict C89 > compatibility could be explicitly coded. > >> So if we turn pedantic on in DEVOPTS by default, wouldn't it make sense >> to at least have a CI job where we test that we compile with >> USE_PARENS_AROUND_GETTEXT_N (which at that point would not be the default >> anymore). > > agree, and indeed was thinking it might be worth combining this job > with the SANITIZE one for efficiency. On the other hand maybe we should just remove USE_PARENS_AROUND_GETTEXT_N entirely, i.e. always use the parens. That facility seems to have been added in response to a one-off mistake in 9c9b4f2f8b7 (standardize usage info string format, 2015-01-13). See https://lore.kernel.org/git/ecb18f9d6ac56da0a61c3b98f8f2236@74d39fa044aa309eaea14b9f57fe79c/. That later landed as 290c8e7a3fe (gettext.h: add parentheses around N_ expansion if supported, 2015-01-11). It doesn't seem worth the effort to forever maintain this special case and use CI resources etc. to catch what was effectively a one-off typo.
On Tue, Aug 31, 2021 at 1:57 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > On the other hand maybe we should just remove > USE_PARENS_AROUND_GETTEXT_N entirely, i.e. always use the parens. that would break pedantic in all versions of gcc since it is a GNU extension and is not valid in any C standard. (unlike the ones we are using with weather balloons and that are valid C99) the C standard says arrays can be initialized by a string literal (obviously with quotes) and allows only optional {} which would avoid the accidental concatenation that triggered this, but can't be used as an alternative. > It doesn't seem worth the effort to forever maintain this special case > and use CI resources etc. to catch what was effectively a one-off typo. under that argument, removing this safeguard might be also possible. Carlo
On Tue, Aug 31, 2021 at 04:54:52PM -0700, Carlo Arenas wrote: > On Tue, Aug 31, 2021 at 1:57 PM Ævar Arnfjörð Bjarmason > <avarab@gmail.com> wrote: > > > > On the other hand maybe we should just remove > > USE_PARENS_AROUND_GETTEXT_N entirely, i.e. always use the parens. > > that would break pedantic in all versions of gcc since it is a GNU > extension and is not valid in any C standard. > (unlike the ones we are using with weather balloons and that are valid C99) I think Ævar might have mis-spoke there. It would make sense to get rid of the feature and _never_ use parens, which is always valid C (and does not tickle pedantic, but also does not catch any accidental string concatenation). That actually seems quite reasonable to me. Something like this, I guess? diff --git a/Makefile b/Makefile index d1feab008f..4936e234bc 100644 --- a/Makefile +++ b/Makefile @@ -409,15 +409,6 @@ all:: # Define NEEDS_LIBRT if your platform requires linking with librt (glibc version # before 2.17) for clock_gettime and CLOCK_MONOTONIC. # -# Define USE_PARENS_AROUND_GETTEXT_N to "yes" if your compiler happily -# compiles the following initialization: -# -# static const char s[] = ("FOO"); -# -# and define it to "no" if you need to remove the parentheses () around the -# constant. The default is "auto", which means to use parentheses if your -# compiler is detected to support it. -# # Define HAVE_BSD_SYSCTL if your platform has a BSD-compatible sysctl function. # # Define HAVE_GETDELIM if your system has the getdelim() function. @@ -497,8 +488,7 @@ all:: # # pedantic: # -# Enable -pedantic compilation. This also disables -# USE_PARENS_AROUND_GETTEXT_N to produce only relevant warnings. +# Enable -pedantic compilation. GIT-VERSION-FILE: FORCE @$(SHELL_PATH) ./GIT-VERSION-GEN @@ -1347,14 +1337,6 @@ ifneq (,$(SOCKLEN_T)) BASIC_CFLAGS += -Dsocklen_t=$(SOCKLEN_T) endif -ifeq (yes,$(USE_PARENS_AROUND_GETTEXT_N)) - BASIC_CFLAGS += -DUSE_PARENS_AROUND_GETTEXT_N=1 -else -ifeq (no,$(USE_PARENS_AROUND_GETTEXT_N)) - BASIC_CFLAGS += -DUSE_PARENS_AROUND_GETTEXT_N=0 -endif -endif - ifeq ($(uname_S),Darwin) ifndef NO_FINK ifeq ($(shell test -d /sw/lib && echo y),y) diff --git a/config.mak.dev b/config.mak.dev index 022fb58218..41d6345bc0 100644 --- a/config.mak.dev +++ b/config.mak.dev @@ -4,8 +4,6 @@ SPARSE_FLAGS += -Wsparse-error endif ifneq ($(filter pedantic,$(DEVOPTS)),) DEVELOPER_CFLAGS += -pedantic -# don't warn for each N_ use -DEVELOPER_CFLAGS += -DUSE_PARENS_AROUND_GETTEXT_N=0 endif DEVELOPER_CFLAGS += -Wall DEVELOPER_CFLAGS += -Wdeclaration-after-statement diff --git a/gettext.h b/gettext.h index c8b34fd612..d209911ebb 100644 --- a/gettext.h +++ b/gettext.h @@ -55,31 +55,7 @@ const char *Q_(const char *msgid, const char *plu, unsigned long n) } /* Mark msgid for translation but do not translate it. */ -#if !USE_PARENS_AROUND_GETTEXT_N #define N_(msgid) msgid -#else -/* - * Strictly speaking, this will lead to invalid C when - * used this way: - * static const char s[] = N_("FOO"); - * which will expand to - * static const char s[] = ("FOO"); - * and in valid C, the initializer on the right hand side must - * be without the parentheses. But many compilers do accept it - * as a language extension and it will allow us to catch mistakes - * like: - * static const char *msgs[] = { - * N_("one") - * N_("two"), - * N_("three"), - * NULL - * }; - * (notice the missing comma on one of the lines) by forcing - * a compilation error, because parenthesised ("one") ("two") - * will not get silently turned into ("onetwo"). - */ -#define N_(msgid) (msgid) -#endif const char *get_preferred_languages(void); int is_utf8_locale(void); diff --git a/git-compat-util.h b/git-compat-util.h index b46605300a..ddc65ff61d 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1253,10 +1253,6 @@ int warn_on_fopen_errors(const char *path); */ int open_nofollow(const char *path, int flags); -#if !defined(USE_PARENS_AROUND_GETTEXT_N) && defined(__GNUC__) -#define USE_PARENS_AROUND_GETTEXT_N 1 -#endif - #ifndef SHELL_PATH # define SHELL_PATH "/bin/sh" #endif
Jeff King <peff@peff.net> writes: > On Tue, Aug 31, 2021 at 04:54:52PM -0700, Carlo Arenas wrote: > >> On Tue, Aug 31, 2021 at 1:57 PM Ævar Arnfjörð Bjarmason >> <avarab@gmail.com> wrote: >> > >> > On the other hand maybe we should just remove >> > USE_PARENS_AROUND_GETTEXT_N entirely, i.e. always use the parens. >> >> that would break pedantic in all versions of gcc since it is a GNU >> extension and is not valid in any C standard. >> (unlike the ones we are using with weather balloons and that are valid C99) > > I think Ævar might have mis-spoke there. It would make sense to get rid > of the feature and _never_ use parens, which is always valid C (and does > not tickle pedantic, but also does not catch any accidental string > concatenation). > > That actually seems quite reasonable to me. That does sound sensible. > Something like this, I guess? Looks good. We could give a warning when the now defunct knob is used, but I don't think there is anything gained by doing so (over just silently ignoring it).
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 47876a4f02..6b9427eff1 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -259,6 +259,8 @@ jobs: image: alpine - jobname: Linux32 image: daald/ubuntu32:xenial + - jobname: pedantic + image: fedora env: jobname: ${{matrix.vector.jobname}} runs-on: ubuntu-latest diff --git a/ci/install-docker-dependencies.sh b/ci/install-docker-dependencies.sh index 26a6689766..07a8c6b199 100755 --- a/ci/install-docker-dependencies.sh +++ b/ci/install-docker-dependencies.sh @@ -15,4 +15,8 @@ linux-musl) apk add --update build-base curl-dev openssl-dev expat-dev gettext \ pcre2-dev python3 musl-libintl perl-utils ncurses >/dev/null ;; +pedantic) + dnf -yq update >/dev/null && + dnf -yq install make gcc findutils diffutils perl python3 gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null + ;; esac diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh index 3ce81ffee9..f3aba5d6cb 100755 --- a/ci/run-build-and-tests.sh +++ b/ci/run-build-and-tests.sh @@ -10,6 +10,11 @@ windows*) cmd //c mklink //j t\\.prove "$(cygpath -aw "$cache_dir/.prove")";; *) ln -s "$cache_dir/.prove" t/.prove;; esac +if test "$jobname" = "pedantic" +then + export DEVOPTS=pedantic +fi + make case "$jobname" in linux-gcc) @@ -35,10 +40,9 @@ linux-clang) export GIT_TEST_DEFAULT_HASH=sha256 make test ;; -linux-gcc-4.8) +linux-gcc-4.8|pedantic) # Don't run the tests; we only care about whether Git can be - # built with GCC 4.8, as it errors out on some undesired (C99) - # constructs that newer compilers seem to quietly accept. + # built with GCC 4.8 or with pedantic ;; *) make test
similar to the recently added sparse task, it is nice to know as early as possible. add a dockerized build using fedora (that usually has the latest gcc) to be ahead of the curve and avoid older ISO C issues at the same time. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> --- .github/workflows/main.yml | 2 ++ ci/install-docker-dependencies.sh | 4 ++++ ci/run-build-and-tests.sh | 10 +++++++--- 3 files changed, 13 insertions(+), 3 deletions(-)