[nft PATCH] Makefile: Fix for 'make distcheck'

[nft PATCH] Makefile: Fix for 'make distcheck'

[Phil Sutter]

Make sure the files in tools/ are added to the tarball and that the
created nftables.service file is removed upon 'make clean'.

Fixes: c4b17cf830510 ("tools: add a systemd unit for static rulesets")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 Makefile.am | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 14915a58cdced..3345412991289 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -409,8 +409,12 @@ EXTRA_DIST += \
 EXTRA_DIST += \
 	files \
 	tests \
+	tools/nftables.service.8 \
+	tools/nftables.service.in \
 	$(NULL)
 
+CLEANFILES += tools/nftables.service
+
 AM_TESTS_ENVIRONMENT = RUN_FULL_TESTSUITE=1; export RUN_FULL_TESTSUITE;
 TESTS = tests/json_echo/run-test.py \
 	tests/monitor/run-tests.sh \
-- 
2.51.0

Re: [nft PATCH] Makefile: Fix for 'make distcheck'

[Pablo Neira Ayuso]

On Tue, Aug 26, 2025 at 07:06:43PM +0200, Phil Sutter wrote:
> Make sure the files in tools/ are added to the tarball and that the
> created nftables.service file is removed upon 'make clean'.
> 
> Fixes: c4b17cf830510 ("tools: add a systemd unit for static rulesets")
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>

Thanks

Re: [nft PATCH] Makefile: Fix for 'make distcheck'

[Pablo Neira Ayuso]

On Tue, Aug 26, 2025 at 09:41:27PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Aug 26, 2025 at 07:06:43PM +0200, Phil Sutter wrote:
> > Make sure the files in tools/ are added to the tarball and that the
> > created nftables.service file is removed upon 'make clean'.
> > 
> > Fixes: c4b17cf830510 ("tools: add a systemd unit for static rulesets")
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> 
> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>

I was preparing the release, however, this fix is not enough.

When I run:

        make distcheck

 /usr/bin/mkdir -p '/lib/systemd/system'
 /usr/bin/install -c -m 644 tools/nftables.service '/lib/systemd/system'
/usr/bin/install: cannot create regular file '/lib/systemd/system/nftables.service': Permission denied

This is not regarding the _inst directory, ie. nftables-x.y.z/_inst/
that make distcheck creates as non-root.

This needs a fix.

Re: [nft PATCH] Makefile: Fix for 'make distcheck'

[Jan Engelhardt]

On Wednesday 2025-08-27 14:22, Pablo Neira Ayuso wrote:
>On Tue, Aug 26, 2025 at 09:41:27PM +0200, Pablo Neira Ayuso wrote:
>> On Tue, Aug 26, 2025 at 07:06:43PM +0200, Phil Sutter wrote:
>> > Make sure the files in tools/ are added to the tarball and that the
>> > created nftables.service file is removed upon 'make clean'.
>> > 
>        make distcheck
>
>/usr/bin/install: cannot create regular file '/lib/systemd/system/nftables.service': Permission denied
>
>This needs a fix.

I encountered this previously in other software and know a pattern;
let me cook up a concrete patch .

[PATCH] build: make `make distcheck` succeed in the face of absolute paths

[Jan Engelhardt]

`make distcheck` has an expectation that, if only --prefix is
specified, all other potentially-configurable paths are somehow
relative to '${prefix}', e.g. bindir defaults to '${prefix}/bin'.

We get an absolute path from $(pkg-config systemd ...) at all times
in case systemd.pc is present, and an empty path in case it is not,
which collides with the aforementioned expectation two ways. Add an
internal --with-dcprefix configure option for the sake of distcheck.
---
 Makefile.am  | 1 +
 configure.ac | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index e292d3b9..52a3e6c4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -23,6 +23,7 @@ libnftables_LIBVERSION = 2:0:1
 ###############################################################################
 
 ACLOCAL_AMFLAGS = -I m4
+AM_DISTCHECK_CONFIGURE_FLAGS = --with-dcprefix='$${prefix}'
 
 EXTRA_DIST =
 BUILT_SOURCES =
diff --git a/configure.ac b/configure.ac
index 626c641b..198b3be8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -122,6 +122,11 @@ AC_ARG_WITH([unitdir],
 		AS_IF([test -z "$unitdir"], [unitdir='${prefix}/lib/systemd/system'])
 	])
 AC_SUBST([unitdir])
+AC_ARG_WITH([dcprefix],
+        [AS_HELP_STRING([Extra path inserted for distcheck])],
+        [dcprefix="$withval"])
+AC_SUBST([dcprefix])
+AS_IF([test -z "$unitdir"], [unitdir='${prefix}/lib/systemd/system'], [unitdir='${dcprefix}'"$unitdir"])
 
 
 AC_CONFIG_FILES([					\
-- 
2.51.0

Re: [PATCH] build: make `make distcheck` succeed in the face of absolute paths

[Pablo Neira Ayuso]

On Wed, Aug 27, 2025 at 02:43:07PM +0200, Jan Engelhardt wrote:
> `make distcheck` has an expectation that, if only --prefix is
> specified, all other potentially-configurable paths are somehow
> relative to '${prefix}', e.g. bindir defaults to '${prefix}/bin'.
> 
> We get an absolute path from $(pkg-config systemd ...) at all times
> in case systemd.pc is present, and an empty path in case it is not,
> which collides with the aforementioned expectation two ways. Add an
> internal --with-dcprefix configure option for the sake of distcheck.

Subtle internal detail is exposed:

nft configuration:
  cli support:                  editline
  enable debugging symbols:     yes
  use mini-gmp:                 no
  enable man page:              yes
  libxtables support:           no
  json output support:          no
  systemd unit:                 ${dcprefix}/lib/systemd/system
                                ^---------^

When running ./configure on the extracted tarball file.

I modified your patch to expose the systemd unit: file to provide a
hint that service file is being installed.

Any suggestion to refine this?

> ---
>  Makefile.am  | 1 +
>  configure.ac | 5 +++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/Makefile.am b/Makefile.am
> index e292d3b9..52a3e6c4 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -23,6 +23,7 @@ libnftables_LIBVERSION = 2:0:1
>  ###############################################################################
>  
>  ACLOCAL_AMFLAGS = -I m4
> +AM_DISTCHECK_CONFIGURE_FLAGS = --with-dcprefix='$${prefix}'
>  
>  EXTRA_DIST =
>  BUILT_SOURCES =
> diff --git a/configure.ac b/configure.ac
> index 626c641b..198b3be8 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -122,6 +122,11 @@ AC_ARG_WITH([unitdir],
>  		AS_IF([test -z "$unitdir"], [unitdir='${prefix}/lib/systemd/system'])
>  	])
>  AC_SUBST([unitdir])
> +AC_ARG_WITH([dcprefix],
> +        [AS_HELP_STRING([Extra path inserted for distcheck])],
> +        [dcprefix="$withval"])
> +AC_SUBST([dcprefix])
> +AS_IF([test -z "$unitdir"], [unitdir='${prefix}/lib/systemd/system'], [unitdir='${dcprefix}'"$unitdir"])
>  
>  
>  AC_CONFIG_FILES([					\
> -- 
> 2.51.0
> 

Re: [PATCH] build: make `make distcheck` succeed in the face of absolute paths

[Jan Engelhardt]

On Wednesday 2025-08-27 14:57, Pablo Neira Ayuso wrote:
>On Wed, Aug 27, 2025 at 02:43:07PM +0200, Jan Engelhardt wrote:
>> `make distcheck` has an expectation that, if only --prefix is
>> specified, all other potentially-configurable paths are somehow
>> relative to '${prefix}', e.g. bindir defaults to '${prefix}/bin'.
>> 
>> We get an absolute path from $(pkg-config systemd ...) at all times
>> in case systemd.pc is present, and an empty path in case it is not,
>> which collides with the aforementioned expectation two ways. Add an
>> internal --with-dcprefix configure option for the sake of distcheck.
>
>Subtle internal detail is exposed:
>
>nft configuration:
>  cli support:                  editline
>  enable debugging symbols:     yes
>  use mini-gmp:                 no
>  enable man page:              yes
>  libxtables support:           no
>  json output support:          no
>  systemd unit:                 ${dcprefix}/lib/systemd/system
>                                ^---------^

That is unavoidable. Variables with paths are expected to be recursively
expanded -- make will do that, sh won't.
And that is why you will see '${var}' when directly echoing from configure.ac.
This is nothing specific to ${dcprefix}, you will also observe this when doing
something as trivial as

	# configure.ac
	echo "bindir: ${bindir}"
	AC_OUTPUT

Re: [PATCH] build: make `make distcheck` succeed in the face of absolute paths

[Phil Sutter]

On Wed, Aug 27, 2025 at 02:43:07PM +0200, Jan Engelhardt wrote:
> `make distcheck` has an expectation that, if only --prefix is
> specified, all other potentially-configurable paths are somehow
> relative to '${prefix}', e.g. bindir defaults to '${prefix}/bin'.
> 
> We get an absolute path from $(pkg-config systemd ...) at all times
> in case systemd.pc is present, and an empty path in case it is not,
> which collides with the aforementioned expectation two ways. Add an
> internal --with-dcprefix configure option for the sake of distcheck.
> ---
>  Makefile.am  | 1 +
>  configure.ac | 5 +++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/Makefile.am b/Makefile.am
> index e292d3b9..52a3e6c4 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -23,6 +23,7 @@ libnftables_LIBVERSION = 2:0:1
>  ###############################################################################
>  
>  ACLOCAL_AMFLAGS = -I m4
> +AM_DISTCHECK_CONFIGURE_FLAGS = --with-dcprefix='$${prefix}'
>  
>  EXTRA_DIST =
>  BUILT_SOURCES =
> diff --git a/configure.ac b/configure.ac
> index 626c641b..198b3be8 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -122,6 +122,11 @@ AC_ARG_WITH([unitdir],
>  		AS_IF([test -z "$unitdir"], [unitdir='${prefix}/lib/systemd/system'])
>  	])
>  AC_SUBST([unitdir])
> +AC_ARG_WITH([dcprefix],
> +        [AS_HELP_STRING([Extra path inserted for distcheck])],
> +        [dcprefix="$withval"])
> +AC_SUBST([dcprefix])
> +AS_IF([test -z "$unitdir"], [unitdir='${prefix}/lib/systemd/system'], [unitdir='${dcprefix}'"$unitdir"])

Shouldn't this test for '-z "$dcprefix"'?

Cheers, Phil

Re: [nft PATCH] Makefile: Fix for 'make distcheck'

[Phil Sutter]

On Tue, Aug 26, 2025 at 07:06:43PM +0200, Phil Sutter wrote:
> Make sure the files in tools/ are added to the tarball and that the
> created nftables.service file is removed upon 'make clean'.
> 
> Fixes: c4b17cf830510 ("tools: add a systemd unit for static rulesets")
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Patch applied.