mbox series

[v2,0/2] ci: detect more warnings via `-Og`

Message ID cover.1717662814.git.ps@pks.im (mailing list archive)
Headers show
Series ci: detect more warnings via `-Og` | expand

Message

Patrick Steinhardt June 6, 2024, 9:31 a.m. UTC
Hi,

this is the second version of my patch series that modifies one of our
CI jobs to compile with `-Og`. We have noticed that it may surface more
warnings that we do not see with `-O2`, so this should help to find more
bugs up front.

Changes compared to v1:

  - Instead of adapting the "pedantic" job, we now adapt
    "linux-gcc-default" to compile with `-Og`. This is because that job
    uses ubuntu:latest and thus a recent compiler, and there are other
    jobs with ubuntu:latest that continue to compile with `-O2`. So this
    is a strict improvement of coverage for diagnostics.

  - Add a way to override the optimization level to our Makefile, as
    suggested by Peff.

Patrick

Patrick Steinhardt (2):
  ci: fix check for Ubuntu 20.04
  ci: compile "linux-gcc-default" job with -Og

 Makefile                  | 3 ++-
 ci/lib.sh                 | 2 +-
 ci/run-build-and-tests.sh | 9 +++++++++
 3 files changed, 12 insertions(+), 2 deletions(-)

Range-diff against v1:
1:  f91004a438 = 1:  f91004a438 ci: fix check for Ubuntu 20.04
2:  351dec4a4d ! 2:  bdf0e40a77 ci: let pedantic job compile with -Og
    @@ Metadata
     Author: Patrick Steinhardt <ps@pks.im>
     
      ## Commit message ##
    -    ci: let pedantic job compile with -Og
    +    ci: compile "linux-gcc-default" job with -Og
     
         We have recently noticed that our CI does not always notice variables
         that may be used uninitialized. While it is expected that compiler
    @@ Commit message
     
         While we could adapt all jobs to compile with `-Og` now, that would
         potentially mask other warnings that only get diagnosed with `-O2`.
    -    Instead, only adapt the "pedantic" job to compile with `-Og`.
    +    Instead, adapt the "linux-gcc-default" job to compile with `-Og`. This
    +    job is chosen because it uses the "ubuntu:latest" image and should thus
    +    have a comparatively recent compiler toolchain, and because we have
    +    other jobs that use "ubuntu:latest" so that we do not loose coverage for
    +    warnings diagnosed only on `-O2` level.
    +
    +    To make it easier to set up the optimization level in our CI, add
    +    support in our Makefile to specify the level via an environment
    +    variable.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
    + ## Makefile ##
    +@@ Makefile: endif
    + # tweaked by config.* below as well as the command-line, both of
    + # which'll override these defaults.
    + # Older versions of GCC may require adding "-std=gnu99" at the end.
    +-CFLAGS = -g -O2 -Wall
    ++O ?= 2
    ++CFLAGS = -g -O$(O) -Wall
    + LDFLAGS =
    + CC_LD_DYNPATH = -Wl,-rpath,
    + BASIC_CFLAGS = -I.
    +
      ## ci/run-build-and-tests.sh ##
    -@@ ci/run-build-and-tests.sh: pedantic)
    - 	# Don't run the tests; we only care about whether Git can be
    - 	# built.
    - 	export DEVOPTS=pedantic
    +@@ ci/run-build-and-tests.sh: esac
    + run_tests=t
    + 
    + case "$jobname" in
    ++linux-gcc-default)
     +	# Warnings generated by compilers are unfortunately specific to the
     +	# optimization level. With `-O0`, many warnings won't be shown at all,
     +	# whereas the optimizations performed by our default optimization level
     +	# `-O2` will mask others. We thus use `-Og` here just so that we have
     +	# at least one job with a different optimization level so that we can
     +	# overall surface more warnings.
    -+	cat >config.mak <<-EOF
    -+	export CFLAGS=-Og
    -+	EOF
    - 	run_tests=
    ++	export O=g
    ++	;;
    + linux-gcc)
    + 	export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
      	;;
    - esac