To take this discussion all the way back to APR project's potential concerns...
On Wed, Mar 13, 2019 at 8:03 AM Jim Jagielski <j...@jagunet.com> wrote: > > > On Mar 13, 2019, at 8:56 AM, Nick Kew <n...@apache.org> wrote: > > > >> On 13 Mar 2019, at 12:43, Jim Jagielski <j...@jagunet.com> wrote: > >> > >> Is there anyone else building 2.4 on macOS under maintainer-mode > >> who is also being affected by the above? The fact that I seem to > >> be the anyone "complaining" :) seems weird... > > > > Just out of interest, is that with a libxml2-enabled APR version? > > Guess I need to test-drive that on Mac/latest, which has bitten me > > on similar platform issues before now! > > It's libxml2 from MacPorts... which is basically vanilla libxml2. > > APR is 1.6.x Here's the concern Nick raises... apr 2.0 (and now apr-util 1.7) allow us to substitute --with-libxml2 in place of expat. This works for most scenarios (subversion itself still needs expat specifically, since it uses more than simple apr_xml.) It would be good to also fix maintainer mode in apr 1.7.0, along with the OSX quirk in APR_OFF_T_FMT, so we are prepared for the apr-util 1.7.0 libxml2 Nick has just backported. This same compiler emit on OSX should rise again in this scenario, *except* that in apr we don't toggle -std=c89. So perhaps we will observe no warnings, if clang defaults to a later -std= option? I'd like (speaking from APR's perspective) to find the right solution to this bogus combination of OSX clang 5 comment rules and their icu library includes. And we presume that is going to happen again with some other system headers down the road. Here's what happens today using libxml2 on httpd; On Wed, Mar 13, 2019 at 8:04 AM Jim Jagielski <j...@jagunet.com> wrote: > > > On Wed, Mar 13, 2019 at 1:45 PM Eric Covener <cove...@gmail.com> wrote: > >> > >> On Wed, Mar 13, 2019 at 8:43 AM Jim Jagielski <j...@jagunet.com> wrote: > >>> > >>> Is there anyone else building 2.4 on macOS under maintainer-mode > >>> who is also being affected by the above? The fact that I seem to > >>> be the anyone "complaining" :) seems weird... > >> > >> FWIW Looks like I have maintainer mode but not libxml2 > > Yeah, I also think it depends on the version of clang... previous versions did not flag > > /usr/local/include/unicode/uenum.h:1:1: error: // comments are not allowed in this language [-Werror,-Wcomment] > > as fatal errors. To solve for the case of --with-maintainer-mode for APR + APR-UTIL 1.7 releases, I see three possibilities; Only for maintainer mode, where we are strictly handling all errors, always check all -std=c99 behaviors (fix any legacy pre-c99 issues that may arise.) All the system headers using c99 (or earlier) semantics should behave well. I'm expecting we still adhere to c89, especially in all APR 1.x releases, but we don't need to explicitly add that flag to normal builds, IMO. Or, for maintainer mode, always relax the comments restriction only so we always add -Wno-error=comment. You can almost call this c99-lite which solves one c99'ism in newer system headers without allowing all possible c99'isms in system headers, and without overriding the -std flag defaults at all. Or, introduce no -std flag, similar to above, and only in the clang 5 case add -Wno-error=comment in apr maintainer mode so that even if -std=c89 is toggled elsewhere, that particular error is overriden. This is the current APR maintainer-mode logic; AC_ARG_ENABLE(maintainer-mode,[ --enable-maintainer-mode Turn on debugging and compile time warnings], [APR_ADDTO(CFLAGS,-g) if test "$GCC" = "yes"; then APR_ADDTO(CFLAGS,[-Wall -Wmissing-prototypes -Wstrict-prototypes -Wmissing-declarations]) case `($CC --version) 2>/dev/null` in *clang-900* | *"clang version 5.0.0"*) APR_ADDTO(CFLAGS,[-Wno-error=strict-prototypes]) ;; esac elif test "$AIX_XLC" = "yes"; then APR_ADDTO(CFLAGS,-qfullpath -qinitauto=FE -qcheck=all -qinfo=pro) fi ])dnl Note that there is no future-proofing here, clang 5.0.1 or 5.1.0 wouldn't escape the strict-prototypes omission (and wondering, why it was needed in the first place, if not for system headers again.) Contrast to httpd 2.4's set of flags; -std=c89 -Werror -Wall -Wstrict-prototypes -Wmissing-prototypes -Wmissing-declarations -Wdeclaration-after-statement -Wpointer-arith -Wformat -Wformat-security -Wunused So, what would folks propose we change to APR 1.7.0 maintainer mode, or does anyone have a positive thought on the above three options?