[patch] ngx_cpp_test module build issue cleanup

[Vladimir Homutov]

On Thu, Nov 24, 2022 at 06:46:15PM +0300, Maxim Dounin wrote:
> Hello!
>
> On Thu, Nov 24, 2022 at 02:31:33PM +0300, Vladimir Homutov via nginx-devel wrote:
>
> > On Thu, Nov 24, 2022 at 01:25:30PM +0400, Sergey Kandaurov wrote:
> > >
> > > > On 23 Nov 2022, at 21:50, Vladimir Homutov via nginx-devel <nginx-devel at nginx.org> wrote:
> > > >
> > > > Hello,
> > > >
> > > > the simplest ./configure --with-cpp_test_module leads to build error
> > > > after successful configuration:
> > > >
> > > > src/misc/ngx_cpp_test_module.cpp:13:12: fatal error: ngx_mail.h: No such file or directory
> > > >   13 |   #include <ngx_mail.h>
> > > >      |            ^~~~~~~~~~~~
> > > > compilation terminated.
> > > >
> > > >
> > > > # HG changeset patch
> > > > # User Vladimir Khomutov <vl at inspert.ru>
> > > > # Date 1669225034 -10800
> > > > #      Wed Nov 23 20:37:14 2022 +0300
> > > > # Node ID 6237563c81707c8c2453cb0a7509ddaf64c02f4e
> > > > # Parent  49e7db44b57c9f4d54b87d19a696178b913aec5c
> > > > The ngx_cpp_test_module build requires mail and stream.
> > > >
> > > > # HG changeset patch
> > > > # User Vladimir Khomutov <vl at inspert.ru>
> > > > # Date 1669225742 -10800
> > > > #      Wed Nov 23 20:49:02 2022 +0300
> > > > # Node ID 12c04127e3fe4d6aa689ef3bcf3ae0834e7e9ed5
> > > > # Parent  b809f53d3f5bd04df36ac338845289d8e60a888b
> > > > The ngx_cpp_test_module build requires mail and stream.
> > > >
> > > > diff --git a/auto/modules b/auto/modules
> > > > --- a/auto/modules
> > > > +++ b/auto/modules
> > > > @@ -1358,6 +1358,17 @@ if [ $NGX_GOOGLE_PERFTOOLS = YES ]; then
> > > > fi
> > > >
> > > > if [ $NGX_CPP_TEST = YES ]; then
> > > > +
> > > > +    if [ $MAIL = NO ]; then
> > > > +        echo "$0: error: ngx_cpp_test_module assumes \"--with-mail\""
> > > > +        exit 1
> > > > +    fi
> > > > +
> > > > +    if [ $STREAM = NO ]; then
> > > > +        echo "$0: error: ngx_cpp_test_module assumes \"--with-stream\""
> > > > +        exit 1
> > > > +    fi
> > > > +
> > > >     ngx_module_name=
> > > >     ngx_module_incs=
> > > >     ngx_module_deps=
> > > >
> > >
> > > Hello,
> > >
> > > if at all try to fix it,
> > > --without-http would also need to be addressed.
> >
> > yes, you are right. missed that since it is enabled by default.
> >
> > A bit shorter patch:
> >
> > # HG changeset patch
> > # User Vladimir Khomutov <vl at inspert.ru>
> > # Date 1669289342 -10800
> > #      Thu Nov 24 14:29:02 2022 +0300
> > # Node ID fd671044ba73ab8a32e558ba9d4dbe718f2b7a54
> > # Parent  b809f53d3f5bd04df36ac338845289d8e60a888b
> > The ngx_cpp_test_module build requires http, mail and stream.
> >
> > diff --git a/auto/modules b/auto/modules
> > --- a/auto/modules
> > +++ b/auto/modules
> > @@ -1358,6 +1358,12 @@ if [ $NGX_GOOGLE_PERFTOOLS = YES ]; then
> >  fi
> >
> >  if [ $NGX_CPP_TEST = YES ]; then
> > +
> > +    if [ $HTTP = NO -o $MAIL = NO -o $STREAM = NO ]; then
> > +        echo "$0: error: ngx_cpp_test_module requires http, mail and stream"
> > +        exit 1
> > +    fi
> > +
> >      ngx_module_name=
> >      ngx_module_incs=
> >      ngx_module_deps=
>
> Following other configure error messages (see auto/lib/zlib/conf,
> auto/lib/pcre/conf, or auto/lib/google-perftools/conf), this
> should be "the C++ test module ...", if at all.
>
> Also, it probably should be in auto/options, since it is a
> verification of configure options, while the auto/modules is
> expected to construct various internal lists based on the modules
> being enabled.
>
> Overall, I'm not convinced it is actually needed, since the module
> is basically a development tool, and not expected to be used by
> users who are not aware of how it is expected to be used, why the
> compilation could fail if mail or stream aren't compiled in, and
> how to fix it even without reconfiguring nginx with mail and
> stream.
>

I tend to agree that it's not worth effort.
Probably we need to have macro defined depending on presence of
HTTP/MAIL/STREAM subsystems, and use them in source code.
But I can hardly imaging code that needs it.