All of lore.kernel.org
 help / color / mirror / Atom feed
* libnetfilter_queue: automake portability warning
@ 2021-08-27 21:09 Jeremy Sowden
  2021-08-28  3:31 ` Duncan Roe
  2021-08-28 13:39 ` Jan Engelhardt
  0 siblings, 2 replies; 6+ messages in thread
From: Jeremy Sowden @ 2021-08-27 21:09 UTC (permalink / raw)
  To: Duncan Roe; +Cc: pablo, netfilter-devel

[-- Attachment #1: Type: text/plain, Size: 2381 bytes --]

Running autogen.sh gives the following output when it gets to
doxygen/Makefile.am:

  doxygen/Makefile.am:3: warning: shell find $(top_srcdir: non-POSIX variable name
  doxygen/Makefile.am:3: (probably a GNU make extension)

Automake doesn't understand the GNU make $(shell ...) function and tries
to interpret it as an Automake variable.  The Automake people would
probably say we shouldn't do that 'cause it's not portable:

  https://www.gnu.org/software/automake/manual/automake.html#Wildcards

However, if we accept that we are targetting GNU make, but we want to
get rid of the warning, I believe there are two ways to do so.  We can
tell Automake not to warn about non-portable constructions:

  diff --git a/configure.ac b/configure.ac
  index 4721eebbab1f..7cd34d079e67 100644
  --- a/configure.ac
  +++ b/configure.ac
  @@ -6,7 +6,7 @@ AC_CANONICAL_HOST
   AC_CONFIG_MACRO_DIR([m4])
   AC_CONFIG_HEADERS([config.h])

  -AM_INIT_AUTOMAKE([-Wall foreign subdir-objects
  +AM_INIT_AUTOMAKE([-Wall -Wno-portability foreign subdir-objects
          tar-pax no-dist-gzip dist-bzip2 1.6])
   m4_ifdef([AM_PROG_AR], [AM_PROG_AR])

On the other hand, if we want to suppress the warning for just this one
GNU-ism, we can hide it from automake:

  diff --git a/configure.ac b/configure.ac
  index 4721eebbab1f..b2b54d3168ad 100644
  --- a/configure.ac
  +++ b/configure.ac
  @@ -56,6 +56,19 @@ AS_IF([test "x$DOXYGEN" = x], [
                  with_doxygen=no
          ])
   ])
  +#
  +# Putting $(shell ... ) directly into the doyxgen Makefile.am confuses automake,
  +# which tries to interpret it as an automake variable:
  +#
  +#   doxygen/Makefile.am:3: warning: shell find $(top_srcdir: non-POSIX variable name
  +#   doxygen/Makefile.am:3: (probably a GNU make extension)
  +#
  +# Instead, we use autoconf to substitute it into place after automake has run.
  +#
  +AS_IF([test "x$with_doxygen" != no], [
  +  AC_SUBST([DOC_SRCS], ['$(shell find $(top_srcdir)/src -name '"'"'*.c'"'"')'])
  +])
  +
   AC_OUTPUT

   echo "
  diff --git a/doxygen/Makefile.am b/doxygen/Makefile.am
  index f38009b24114..6ed30e21ff75 100644
  --- a/doxygen/Makefile.am
  +++ b/doxygen/Makefile.am
  @@ -1,6 +1,6 @@
   if HAVE_DOXYGEN

  -doc_srcs = $(shell find $(top_srcdir)/src -name '*.c')
  +doc_srcs = @DOC_SRCS@

   doxyfile.stamp: $(doc_srcs) Makefile.am
          rm -rf html man

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: libnetfilter_queue: automake portability warning
  2021-08-27 21:09 libnetfilter_queue: automake portability warning Jeremy Sowden
@ 2021-08-28  3:31 ` Duncan Roe
  2021-08-28 13:39 ` Jan Engelhardt
  1 sibling, 0 replies; 6+ messages in thread
From: Duncan Roe @ 2021-08-28  3:31 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Pablo Neira Ayuso, Netfilter Development

Hi Jeremy,

On Fri, Aug 27, 2021 at 10:09:58PM +0100, Jeremy Sowden wrote:
> Running autogen.sh gives the following output when it gets to
> doxygen/Makefile.am:
>
>   doxygen/Makefile.am:3: warning: shell find $(top_srcdir: non-POSIX variable name
>   doxygen/Makefile.am:3: (probably a GNU make extension)
>
> Automake doesn't understand the GNU make $(shell ...) function and tries
> to interpret it as an Automake variable.  The Automake people would
> probably say we shouldn't do that 'cause it's not portable:
>
>   https://www.gnu.org/software/automake/manual/automake.html#Wildcards
>
> However, if we accept that we are targetting GNU make, but we want to
> get rid of the warning, I believe there are two ways to do so.  We can
> tell Automake not to warn about non-portable constructions:
>
>   diff --git a/configure.ac b/configure.ac
>   index 4721eebbab1f..7cd34d079e67 100644
>   --- a/configure.ac
>   +++ b/configure.ac
>   @@ -6,7 +6,7 @@ AC_CANONICAL_HOST
>    AC_CONFIG_MACRO_DIR([m4])
>    AC_CONFIG_HEADERS([config.h])
>
>   -AM_INIT_AUTOMAKE([-Wall foreign subdir-objects
>   +AM_INIT_AUTOMAKE([-Wall -Wno-portability foreign subdir-objects
>           tar-pax no-dist-gzip dist-bzip2 1.6])
>    m4_ifdef([AM_PROG_AR], [AM_PROG_AR])
>
> On the other hand, if we want to suppress the warning for just this one
> GNU-ism, we can hide it from automake:
>
>   diff --git a/configure.ac b/configure.ac
>   index 4721eebbab1f..b2b54d3168ad 100644
>   --- a/configure.ac
>   +++ b/configure.ac
>   @@ -56,6 +56,19 @@ AS_IF([test "x$DOXYGEN" = x], [
>                   with_doxygen=no
>           ])
>    ])
>   +#
>   +# Putting $(shell ... ) directly into the doyxgen Makefile.am confuses automake,
>   +# which tries to interpret it as an automake variable:
>   +#
>   +#   doxygen/Makefile.am:3: warning: shell find $(top_srcdir: non-POSIX variable name
>   +#   doxygen/Makefile.am:3: (probably a GNU make extension)
>   +#
>   +# Instead, we use autoconf to substitute it into place after automake has run.
>   +#
>   +AS_IF([test "x$with_doxygen" != no], [
>   +  AC_SUBST([DOC_SRCS], ['$(shell find $(top_srcdir)/src -name '"'"'*.c'"'"')'])
>   +])
>   +
>    AC_OUTPUT
>
>    echo "
>   diff --git a/doxygen/Makefile.am b/doxygen/Makefile.am
>   index f38009b24114..6ed30e21ff75 100644
>   --- a/doxygen/Makefile.am
>   +++ b/doxygen/Makefile.am
>   @@ -1,6 +1,6 @@
>    if HAVE_DOXYGEN
>
>   -doc_srcs = $(shell find $(top_srcdir)/src -name '*.c')
>   +doc_srcs = @DOC_SRCS@
>
>    doxyfile.stamp: $(doc_srcs) Makefile.am
>           rm -rf html man
>
> J.


Thanks for the suggestioms. I strongly prefer adding -Wno-portability to
AM_INIT_AUTOMAKE.

The alternative patch is longer, and IMHO less clear to follow. Also it's
error-prone - consider this line:
> AS_IF([test "x$with_doxygen" != no], [
The test needs to be one of the following:
> [test "x$with_doxygen" != xno]
> [test "$with_doxygen" != no]
> [test "$with_doxygen" = yes]

I'd like to get rid of the warning so will send a v3.

Cheers ... Duncan.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: libnetfilter_queue: automake portability warning
  2021-08-27 21:09 libnetfilter_queue: automake portability warning Jeremy Sowden
  2021-08-28  3:31 ` Duncan Roe
@ 2021-08-28 13:39 ` Jan Engelhardt
  2021-08-28 19:45   ` Duncan Roe
  1 sibling, 1 reply; 6+ messages in thread
From: Jan Engelhardt @ 2021-08-28 13:39 UTC (permalink / raw)
  To: Jeremy Sowden; +Cc: Duncan Roe, pablo, netfilter-devel


On Friday 2021-08-27 23:09, Jeremy Sowden wrote:

>Running autogen.sh gives the following output when it gets to
>doxygen/Makefile.am:
>
>  doxygen/Makefile.am:3: warning: shell find $(top_srcdir: non-POSIX variable name
>  doxygen/Makefile.am:3: (probably a GNU make extension)
>
>Automake doesn't understand the GNU make $(shell ...) [...]

Or, third option, ditch the wildcarding and just name the sources. If going for
a single Makefile (ditching recursive make), that will also be beneficial for
parallel building, and the repo is not too large for such undertaking to be
infeasible.


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: libnetfilter_queue: automake portability warning
  2021-08-28 13:39 ` Jan Engelhardt
@ 2021-08-28 19:45   ` Duncan Roe
  2021-08-28 20:27     ` Pablo Neira Ayuso
  0 siblings, 1 reply; 6+ messages in thread
From: Duncan Roe @ 2021-08-28 19:45 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: Jeremy Sowden, Pablo Neira Ayuso, Netfilter Development

On Sat, Aug 28, 2021 at 03:39:38PM +0200, Jan Engelhardt wrote:
>
> On Friday 2021-08-27 23:09, Jeremy Sowden wrote:
>
> >Running autogen.sh gives the following output when it gets to
> >doxygen/Makefile.am:
> >
> >  doxygen/Makefile.am:3: warning: shell find $(top_srcdir: non-POSIX variable name
> >  doxygen/Makefile.am:3: (probably a GNU make extension)
> >
> >Automake doesn't understand the GNU make $(shell ...) [...]
>
> Or, third option, ditch the wildcarding and just name the sources. If going for
> a single Makefile (ditching recursive make), that will also be beneficial for
> parallel building, and the repo is not too large for such undertaking to be
> infeasible.
>
Certainly naming the sources would work.

But, with wildcarding, Makefile.am works unmodified in other projects, such as
libmnl. Indeed I was planning to have libmnl/autogen.sh fetch both
doxygen/Makefile.am and doxygen/build_man.sh

If the project ends up with a single Makefile, it could `include` nearly all of
the existing doxygen/Makefile.am, and autogen.sh could fetch that in other
projects.

In any case, is wildcarding incompatible with having a single Makefile?

Cheers ... Duncan.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: libnetfilter_queue: automake portability warning
  2021-08-28 19:45   ` Duncan Roe
@ 2021-08-28 20:27     ` Pablo Neira Ayuso
  2021-08-29  4:01       ` Duncan Roe
  0 siblings, 1 reply; 6+ messages in thread
From: Pablo Neira Ayuso @ 2021-08-28 20:27 UTC (permalink / raw)
  To: Jan Engelhardt, Jeremy Sowden, Netfilter Development

On Sun, Aug 29, 2021 at 05:45:29AM +1000, Duncan Roe wrote:
> On Sat, Aug 28, 2021 at 03:39:38PM +0200, Jan Engelhardt wrote:
> >
> > On Friday 2021-08-27 23:09, Jeremy Sowden wrote:
> >
> > >Running autogen.sh gives the following output when it gets to
> > >doxygen/Makefile.am:
> > >
> > >  doxygen/Makefile.am:3: warning: shell find $(top_srcdir: non-POSIX variable name
> > >  doxygen/Makefile.am:3: (probably a GNU make extension)
> > >
> > >Automake doesn't understand the GNU make $(shell ...) [...]
> >
> > Or, third option, ditch the wildcarding and just name the sources. If going for
> > a single Makefile (ditching recursive make), that will also be beneficial for
> > parallel building, and the repo is not too large for such undertaking to be
> > infeasible.
> >
> Certainly naming the sources would work.
> 
> But, with wildcarding, Makefile.am works unmodified in other projects, such as
> libmnl. Indeed I was planning to have libmnl/autogen.sh fetch both
> doxygen/Makefile.am and doxygen/build_man.sh
> 
> If the project ends up with a single Makefile, it could `include` nearly all of
> the existing doxygen/Makefile.am, and autogen.sh could fetch that in other
> projects.

doxygen's Makefile.am is relatively small now.

autogen.sh can be used to keep build_man.sh in sync, that should be
good enough, my main concern is that addressed with shell script split
off.

So I'd suggest remove the wildcarding from Makefile.am too.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: libnetfilter_queue: automake portability warning
  2021-08-28 20:27     ` Pablo Neira Ayuso
@ 2021-08-29  4:01       ` Duncan Roe
  0 siblings, 0 replies; 6+ messages in thread
From: Duncan Roe @ 2021-08-29  4:01 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: Netfilter Development, Jan Engelhardt, Jeremy Sowden

On Sat, Aug 28, 2021 at 10:27:52PM +0200, Pablo Neira Ayuso wrote:
> On Sun, Aug 29, 2021 at 05:45:29AM +1000, Duncan Roe wrote:
> > On Sat, Aug 28, 2021 at 03:39:38PM +0200, Jan Engelhardt wrote:
> > >
> > > On Friday 2021-08-27 23:09, Jeremy Sowden wrote:
> > >
> > > >Running autogen.sh gives the following output when it gets to
> > > >doxygen/Makefile.am:
> > > >
> > > >  doxygen/Makefile.am:3: warning: shell find $(top_srcdir: non-POSIX variable name
> > > >  doxygen/Makefile.am:3: (probably a GNU make extension)
> > > >
> > > >Automake doesn't understand the GNU make $(shell ...) [...]
> > >
> > > Or, third option, ditch the wildcarding and just name the sources. If going for
> > > a single Makefile (ditching recursive make), that will also be beneficial for
> > > parallel building, and the repo is not too large for such undertaking to be
> > > infeasible.
> > >
> > Certainly naming the sources would work.
> >
> > But, with wildcarding, Makefile.am works unmodified in other projects, such as
> > libmnl. Indeed I was planning to have libmnl/autogen.sh fetch both
> > doxygen/Makefile.am and doxygen/build_man.sh
> >
> > If the project ends up with a single Makefile, it could `include` nearly all of
> > the existing doxygen/Makefile.am, and autogen.sh could fetch that in other
> > projects.
>
> doxygen's Makefile.am is relatively small now.
>
> autogen.sh can be used to keep build_man.sh in sync, that should be
> good enough, my main concern is that addressed with shell script split
> off.
>
> So I'd suggest remove the wildcarding from Makefile.am too.

I pushed a v2 which does that.

Cheers ... Duncan.

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-29  4:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-27 21:09 libnetfilter_queue: automake portability warning Jeremy Sowden
2021-08-28  3:31 ` Duncan Roe
2021-08-28 13:39 ` Jan Engelhardt
2021-08-28 19:45   ` Duncan Roe
2021-08-28 20:27     ` Pablo Neira Ayuso
2021-08-29  4:01       ` Duncan Roe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.