GNOME Bugzilla – Bug 746372
Compilation error when any of ENABLE_XXX constants is undefined
Last modified: 2015-03-27 13:59:08 UTC
-Wswitch-enum + -Werror = errors in src/application_window.c:{2086, 2254}. Enum values not handled despite default label. I think -Wswitch would be better in this case.
Thanks for the report. I do not often build without all tag support enabled, but this should be fixed on master with 2f8ce5f47892b75bd610647de22b826f9375b8a2.
Same error also appears in line 2272. Anyway, the proposed patch with all those ifndef hinders readability in a dramatic way and in the long run will become quite a maintenance nightmare. Wouldn't it be a better fix to use -Wswitch instead of -Wswitch-enum? If I have time I'll propose a patch.
Created attachment 300434 [details] [review] Proposed patch There were far more issues than I expected, but with these changes it does compile
Review of attachment 300434 [details] [review]: Without a build log demonstrating the warnings, this patch is not that helpful. ::: Makefile.am @@ +97,3 @@ +easytag_SOURCES += \ + src/tags/wavpack_header.c \ + src/tags/wavpack_private.c \ Just use ifdefs in the files, rather than conditionally compiling them, and be careful of indentation. ::: configure.ac @@ +316,3 @@ [AC_MSG_ERROR([Wavpack support requested but required dependencies ($WAVPACK_DEPS) not found])])]) +AM_CONDITIONAL([ENABLE_WAVPACK], [test x"$have_wavpack" != x"no"]) Unnecessary. ::: m4/ax_compiler_flags_cflags.m4 @@ +111,3 @@ # "maximum" flags AX_APPEND_COMPILE_FLAGS([ dnl + -Wswitch dnl This is from the upstream AX_COMPILER_FLAGS_CFLAGS macro and should not be changed. ::: src/preferences_dialog.c @@ +126,3 @@ } +#if FALSE Unnecessary.
I cannot provide a log of all make errors since each error stops the compilation (the first one is attached). As you can see in the patch (if you don't want to try to reproduce them by building with all warnings enabled and disabling support for ANY of the dependencies), there were a couple of errors about unused functions in src/preferences_dialog.c and the others had to do with using -Wswitch-enum in CFLAGS all over the source tree. If I make a patch with special cure not to mess indentation and all that, addressing these problems and taking into account all things you said above, will you please consider to review/merge it without the build logs?
I would rather you just expanded on the existing approach of using ifdef and ifndef, and did not add any automake conditionals or conditional compilation.
The existing approach is not maintainable in any way, although you have a point that disabling warnings is also not the right way to do it. After reading https://github.com/peti/autoconf-archive/commit/0712d7feca6f35bc9936004a5c3408214a9acdef, would you mind using gcc diagnostic pragmas in specific code snippets?
Created attachment 300444 [details] [review] Corrected proposed patch This is the patch so that you that can review it
No, I do not want to use pragmas. I pushed some patches which allow compilation to succeed with all file support disabled (for me), so please test and report back.
It now builds cleanly. I only hope we don't have to add support for any other file format...