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?

Reply via email to