Add the avif_enable_warnings INTERFACE library

[wantehchang]

It collects all warning-related compiler options and macro definitions. Re-enable warnings on the apps (avifdec, avifenc, and avifgainmaputil).

Warnings on the tests will be re-enabled separately.

A partial fix for #2340.

Note: We cannot use add_compile_options() to enable warnings, because add_compile_options() affects not only our own targets but also the external dependencies we bring in by FetchContent.

[jzern]

This looks to encapsulate the warnings in one target rather than relying on the generators in individual ones. I don't know if this matches the behavior entirely given the earlier comments about exporting the warnings to users of the targets (via add_subdirectory?)

[wantehchang]

This pull request is modeled after the enable_rtti example at https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#interface-libraries.

[vrabaud]

The goal is to not repeat the warning we set through the generator expression and target_compile_options ? That is the standard way, I'd rather see a macro calling what we have for all the targets that we want (especially because some are C++, like avifgainmaputil)

[wantehchang]

Vincent: I can change -Wall -Wextra back to$<BUILD_INTERFACE:-Wall -Wextra>. But avif_enable_warnings doesn't need to use BUILD_INTERFACE because we will only link a library target with avif_enable_warnings as a PRIVATE dependency, so the warning options won't be added to the interface properties of the library target.

As for using a macro or an interface library, it doesn't really matter. Are you suggesting a macro because it is a more elementary feature than an interface library?

The tutorial you mentioned in #2323 (comment) also uses an interface library (named tutorial_compiler_flags) for warning flags.

So far all the compile options and macro definitions in avif_enable_warnings apply to both C and C++, so it's not necessary to deal with language-specific options yet.

[fdintino]

I think this may have been a regression introduced by #1819. I'm having a hard time finding documentation to confirm this, but it seems as though object libraries don't transitively propagate compiler flags to targets. When the target for the warning flags was changed from avif to avif_obj, the warning flags no longer applied to source files on targets that don't directly link to avif_obj. But I think that a simpler fix would be to switch these back to being compile options for the avif target, e.g.

target_compile_options(avif_obj PUBLIC $<BUILD_INTERFACE:-Wall -Wextra -Wshorten-64-to-32>)

would become

target_compile_options(avif PUBLIC $<BUILD_INTERFACE:-Wall -Wextra -Wshorten-64-to-32>)

One virtue of this approach, if it does in fact work, is that the warning flags are automatically applied to all targets, including tests, without any additional work.

[wantehchang]

@fdintino Frankie: Thank you for the suggestion. The only compiler option that the avif library should propagate is -DAVIF_DLL. Also, if another project calls add_subdirectory(libavif), the approach you suggested will propagate the avif library's warning options to the targets in that project that are linked with the avif library. I verified this with the following experiment:

$ cd libavif
$ git diff
diff --git a/CMakeLists.txt b/CMakeLists.txt
index baf59643..398ea149 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -610,6 +610,7 @@ endif()
 # Main avif library.
 set_target_properties(avif PROPERTIES VERSION ${LIBRARY_VERSION} SOVERSION ${LIBRARY_SOVERSION})
 target_link_libraries(avif PRIVATE avif_obj)
+target_compile_options(avif PUBLIC $<BUILD_INTERFACE:-Wfatal-errors>)
 target_include_directories(avif PUBLIC $<BUILD_INTERFACE:${libavif_SOURCE_DIR}/include> $<INSTALL_INTERFACE:include>)
 if(BUILD_SHARED_LIBS)
     target_compile_definitions(avif INTERFACE AVIF_DLL)
$ cd ..
$ cat foo.c
#include <avif/avif.h>
#include <stdio.h>

int main(void) {
  char outBuffer[256];
  avifCodecVersions(outBuffer);
  printf("libavif version: %s\n", avifVersion());
  printf("Codec versions: %s\n", outBuffer);
  return 0;
}
$ cat CMakeLists.txt 
cmake_minimum_required(VERSION 3.28)
project(foo C)
add_executable(foo foo.c)
add_subdirectory(libavif)
target_link_libraries(foo avif)
$ mkdir build
$ cd build
$ cmake .. -G Ninja
-- The C compiler identification is GNU 13.2.0
-- Detecting C compiler ABI info
***snipped***
-- Configuring done (1.4s)
-- Generating done (0.0s)
-- Build files have been written to: /home/wtc/libavif.2/build
$ ninja
[23/23] Linking C static library libavif/libavif_internal.a
$ rm CMakeFiles/foo.dir/foo.c.o
$ ninja -v
[1/2] /usr/bin/cc -DAVIF_DLL -I/home/wtc/libavif.2/libavif/include -Wfatal-errors -MD -MT CMakeFiles/foo.dir/foo.c.o -MF CMakeFiles/foo.dir/foo.c.o.d -o CMakeFiles/foo.dir/foo.c.o -c /home/wtc/libavif.2/foo.c
[2/2] : && /usr/bin/cc   CMakeFiles/foo.dir/foo.c.o -o foo  -Wl,-rpath,/home/wtc/libavif.2/build/libavif  libavif/libavif.so.16.1.1 && :

Note that foo.c is compiled with the -Wfatal-errors option propagated from the avif library, in addition to the -DAVIF_DLL option.

Since multiple sources (including official CMake documentation) recommend that an interface library be used to enable warning options, I am going to switch to this approach.

[wantehchang]

Also, object libraries do transitively propagate compiler flags to targets. That's why the source files in the avif_apps library, which is based on the avif_apps_obj object library, are also compiled with warnings enabled.

    target_link_libraries(avif_apps_obj PUBLIC avif_obj PNG::PNG ZLIB::ZLIB JPEG::JPEG)
    ...
    # Main avif_apps library.
    add_library(avif_apps STATIC)
    target_link_libraries(avif_apps PUBLIC avif PRIVATE avif_apps_obj)