| Message ID | 1342620628-12751-12-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Commit | 8dc38a78d0f74e1561c1fe4d276150a1a8a12c4c |
| Headers | show |
On Wed, Jul 18, 2012 at 2:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > Add support for checking whether test case code can compile without > warnings, by recompiling each successful test with -Werror. If the > -Werror version doesn't pass, we bail out. This gives us the same > level of visibility of warnings in test code as --enable-werror > provides for the main compile. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > configure | 32 ++++++++++++++++++++++++++++---- > 1 files changed, 28 insertions(+), 4 deletions(-) > > diff --git a/configure b/configure > index 8140464..1939bdb 100755 > --- a/configure > +++ b/configure > @@ -27,16 +27,40 @@ printf " '%s'" "$0" "$@" >> config.log > echo >> config.log > echo "#" >> config.log > > +do_cc() { > + # Run the compiler, capturing its output to the log. > + echo $cc "$@" >> config.log > + $cc "$@" >> config.log 2>&1 || return $? > + # Test passed. If this is an --enable-werror build, rerun > + # the test with -Werror and bail out if it fails. This > + # makes warning-generating-errors in configure test code > + # obvious to developers. > + if test "$werror" != "yes"; then > + return 0 > + fi > + # Don't bother rerunning the compile if we were already using -Werror > + case "$*" in > + *-Werror*) > + return 0 > + ;; > + esac > + echo $cc -Werror "$@" >> config.log > + $cc -Werror "$@" >> config.log 2>&1 && return $? > + echo "ERROR: configure test passed without -Werror but failed with -Werror." > + echo "This is probably a bug in the configure script. The failing command" > + echo "will be at the bottom of config.log." > + echo "You can run configure with --disable-werror to bypass this check." > + exit 1 This should be degraded to a warning, I'm getting configure breakage in some cases: config-host.mak is out-of-date, running configure ERROR: configure test passed without -Werror but failed with -Werror. This is probably a bug in the configure script. The failing command will be at the bottom of config.log. You can run configure with --disable-werror to bypass this check. gcc -Werror -m32 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fstack-protector-all -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -I/usr/include/libpng12 -o /tmp/qemu-conf--24538-.exe /tmp/qemu-conf--24538-.c -m32 -g cc1: warnings being treated as errors /tmp/qemu-conf--24538-.c:2: error: unknown option after '#pragma GCC diagnostic' kind > +} > + > compile_object() { > - echo $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log > - $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log 2>&1 > + do_cc $QEMU_CFLAGS -c -o $TMPO $TMPC > } > > compile_prog() { > local_cflags="$1" > local_ldflags="$2" > - echo $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log > - $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log 2>&1 > + do_cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > } > > # symbolically link $1 to $2. Portable version of "ln -sf". > -- > 1.7.5.4 > >
On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote: > On Wed, Jul 18, 2012 at 2:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> + echo "ERROR: configure test passed without -Werror but failed with -Werror." >> + echo "This is probably a bug in the configure script. The failing command" >> + echo "will be at the bottom of config.log." >> + echo "You can run configure with --disable-werror to bypass this check." >> + exit 1 > > This should be degraded to a warning The idea is that by running configure with --enable-werror (either explicitly or implicitly for a git build) you've asked for compile warnings to cause build failure. We're just doing that in configure as well as for the main build. >, I'm getting configure breakage > in some cases: The thesis of the patch series is that these represent bugs to be fixed. -- PMM
On Sat, Jul 28, 2012 at 10:57 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote: >> On Wed, Jul 18, 2012 at 2:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> + echo "ERROR: configure test passed without -Werror but failed with -Werror." >>> + echo "This is probably a bug in the configure script. The failing command" >>> + echo "will be at the bottom of config.log." >>> + echo "You can run configure with --disable-werror to bypass this check." >>> + exit 1 >> >> This should be degraded to a warning > > The idea is that by running configure with --enable-werror (either > explicitly or implicitly for a git build) you've asked for compile > warnings to cause build failure. We're just doing that in configure > as well as for the main build. > >>, I'm getting configure breakage >> in some cases: > > The thesis of the patch series is that these represent bugs to be fixed. Yes, but then the errors should be fixed by the series. When adding new GCC warning flags, I also fixed the causes for the warning first. Committing known build breakages is not OK. > > -- PMM
On 28 July 2012 13:09, Blue Swirl <blauwirbel@gmail.com> wrote: > On Sat, Jul 28, 2012 at 10:57 AM, Peter Maydell > <peter.maydell@linaro.org> wrote: >> On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote: >>>, I'm getting configure breakage in some cases: >> >> The thesis of the patch series is that these represent bugs to be fixed. > > Yes, but then the errors should be fixed by the series. When adding > new GCC warning flags, I also fixed the causes for the warning first. > Committing known build breakages is not OK. I fixed all the ones I saw on my system, obviously. I'll have a look at this one. I think it would be worth committing patches 1-10 now anyway : those fix configure issues (patch 1 in particular fixes the problem of tests silently failing when they should pass). -- PMM
On Sat, Jul 28, 2012 at 12:14 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 28 July 2012 13:09, Blue Swirl <blauwirbel@gmail.com> wrote: >> On Sat, Jul 28, 2012 at 10:57 AM, Peter Maydell >> <peter.maydell@linaro.org> wrote: >>> On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote: >>>>, I'm getting configure breakage in some cases: >>> >>> The thesis of the patch series is that these represent bugs to be fixed. >> >> Yes, but then the errors should be fixed by the series. When adding >> new GCC warning flags, I also fixed the causes for the warning first. >> Committing known build breakages is not OK. > > I fixed all the ones I saw on my system, obviously. I'll have a look > at this one. > > I think it would be worth committing patches 1-10 now anyway : those fix > configure issues (patch 1 in particular fixes the problem of tests > silently failing when they should pass). OK, I'll test those. > > -- PMM
On Sat, Jul 28, 2012 at 12:14 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 28 July 2012 13:09, Blue Swirl <blauwirbel@gmail.com> wrote: >> On Sat, Jul 28, 2012 at 10:57 AM, Peter Maydell >> <peter.maydell@linaro.org> wrote: >>> On 28 July 2012 10:04, Blue Swirl <blauwirbel@gmail.com> wrote: >>>>, I'm getting configure breakage in some cases: >>> >>> The thesis of the patch series is that these represent bugs to be fixed. >> >> Yes, but then the errors should be fixed by the series. When adding >> new GCC warning flags, I also fixed the causes for the warning first. >> Committing known build breakages is not OK. > > I fixed all the ones I saw on my system, obviously. I'll have a look > at this one. > > I think it would be worth committing patches 1-10 now anyway : those fix > configure issues (patch 1 in particular fixes the problem of tests > silently failing when they should pass). I'm getting this error, probably because now Valgrind support is enabled: CC coroutine-ucontext.o cc1: warnings being treated as errors /src/qemu/coroutine-ucontext.c:204: error: unknown option after '#pragma GCC diagnostic' kind /src/qemu/coroutine-ucontext.c:209: error: unknown option after '#pragma GCC diagnostic' kind Maybe the compiler does not understand this pragma and Valgrind check should also fail in that case. $ gcc -v Using built-in specs. Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Debian 4.4.5-8' --with-bugurl=file:///usr/share/doc/gcc-4.4/README.Bugs --enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-4.4 --enable-shared --enable-multiarch --enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.4 --libdir=/usr/lib --enable-nls --enable-clocale=gnu --enable-libstdcxx-debug --enable-objc-gc --with-arch-32=i586 --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 4.4.5 (Debian 4.4.5-8) > > -- PMM
On 28 July 2012 13:31, Blue Swirl <blauwirbel@gmail.com> wrote: > I'm getting this error, probably because now Valgrind support is enabled: > CC coroutine-ucontext.o > cc1: warnings being treated as errors > /src/qemu/coroutine-ucontext.c:204: error: unknown option after > '#pragma GCC diagnostic' kind > /src/qemu/coroutine-ucontext.c:209: error: unknown option after > '#pragma GCC diagnostic' kind > > Maybe the compiler does not understand this pragma and Valgrind check > should also fail in that case. Yeah, I think this is one of the few tests which want to explicitly check "is this construct going to provoke a compiler warning" -- fix is for that test to explictly put -Werror in the cflags in the compile_prog line. -- PMM
Thanks, applied. On Wed, Jul 18, 2012 at 2:10 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > Add support for checking whether test case code can compile without > warnings, by recompiling each successful test with -Werror. If the > -Werror version doesn't pass, we bail out. This gives us the same > level of visibility of warnings in test code as --enable-werror > provides for the main compile. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > configure | 32 ++++++++++++++++++++++++++++---- > 1 files changed, 28 insertions(+), 4 deletions(-) > > diff --git a/configure b/configure > index 8140464..1939bdb 100755 > --- a/configure > +++ b/configure > @@ -27,16 +27,40 @@ printf " '%s'" "$0" "$@" >> config.log > echo >> config.log > echo "#" >> config.log > > +do_cc() { > + # Run the compiler, capturing its output to the log. > + echo $cc "$@" >> config.log > + $cc "$@" >> config.log 2>&1 || return $? > + # Test passed. If this is an --enable-werror build, rerun > + # the test with -Werror and bail out if it fails. This > + # makes warning-generating-errors in configure test code > + # obvious to developers. > + if test "$werror" != "yes"; then > + return 0 > + fi > + # Don't bother rerunning the compile if we were already using -Werror > + case "$*" in > + *-Werror*) > + return 0 > + ;; > + esac > + echo $cc -Werror "$@" >> config.log > + $cc -Werror "$@" >> config.log 2>&1 && return $? > + echo "ERROR: configure test passed without -Werror but failed with -Werror." > + echo "This is probably a bug in the configure script. The failing command" > + echo "will be at the bottom of config.log." > + echo "You can run configure with --disable-werror to bypass this check." > + exit 1 > +} > + > compile_object() { > - echo $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log > - $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log 2>&1 > + do_cc $QEMU_CFLAGS -c -o $TMPO $TMPC > } > > compile_prog() { > local_cflags="$1" > local_ldflags="$2" > - echo $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log > - $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log 2>&1 > + do_cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags > } > > # symbolically link $1 to $2. Portable version of "ln -sf". > -- > 1.7.5.4 > >
diff --git a/configure b/configure index 8140464..1939bdb 100755 --- a/configure +++ b/configure @@ -27,16 +27,40 @@ printf " '%s'" "$0" "$@" >> config.log echo >> config.log echo "#" >> config.log +do_cc() { + # Run the compiler, capturing its output to the log. + echo $cc "$@" >> config.log + $cc "$@" >> config.log 2>&1 || return $? + # Test passed. If this is an --enable-werror build, rerun + # the test with -Werror and bail out if it fails. This + # makes warning-generating-errors in configure test code + # obvious to developers. + if test "$werror" != "yes"; then + return 0 + fi + # Don't bother rerunning the compile if we were already using -Werror + case "$*" in + *-Werror*) + return 0 + ;; + esac + echo $cc -Werror "$@" >> config.log + $cc -Werror "$@" >> config.log 2>&1 && return $? + echo "ERROR: configure test passed without -Werror but failed with -Werror." + echo "This is probably a bug in the configure script. The failing command" + echo "will be at the bottom of config.log." + echo "You can run configure with --disable-werror to bypass this check." + exit 1 +} + compile_object() { - echo $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log - $cc $QEMU_CFLAGS -c -o $TMPO $TMPC >> config.log 2>&1 + do_cc $QEMU_CFLAGS -c -o $TMPO $TMPC } compile_prog() { local_cflags="$1" local_ldflags="$2" - echo $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log - $cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags >> config.log 2>&1 + do_cc $QEMU_CFLAGS $local_cflags -o $TMPE $TMPC $LDFLAGS $local_ldflags } # symbolically link $1 to $2. Portable version of "ln -sf".
Add support for checking whether test case code can compile without warnings, by recompiling each successful test with -Werror. If the -Werror version doesn't pass, we bail out. This gives us the same level of visibility of warnings in test code as --enable-werror provides for the main compile. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- configure | 32 ++++++++++++++++++++++++++++---- 1 files changed, 28 insertions(+), 4 deletions(-)