[PATCH] configure: allow tools/tests build to be disabled

[Christopher Obbard]

For some builds, e.g. inside a minimal buildroot image, we may not want the
tests to be built. Add an option --disable-tests to configure to disable
building the tests.

Signed-off-by: Christopher Obbard <christoph...@linaro.org>
---
Makefile.am | 9 ++++++++-
configure.ac | 12 +++++++++++-
tools/tests/Makefile.am | 3 +++
3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index bb97df255dfb9c8a145eba3ed8dcc58ab9bd124b..160d211c3f12d14cf7a136e0dd36e21fa5f0673a 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -369,8 +369,15 @@ clean-local-completion-pycache:

check-valgrind-local: $(GEN_VERSION_H)

+SUBDIRS = .
+
# Tests depend on libraries being built - start with "."
-SUBDIRS = . tools/tests
+if BUILD_TESTS
+SUBDIRS += tools/tests
+endif
+
+# Include the tests always for distribution
+EXTRA_DIST = tools/tests

FORCE:

diff --git a/configure.ac b/configure.ac
index 36fc8d55e4190766871b8f9b5ea5bbaff5e24dde..6bfc1ec295666137bf2fff00608f296ba67b4336 100644
--- a/configure.ac
+++ b/configure.ac
@@ -281,6 +281,12 @@ AS_IF([test "x$enable_completion" != "xno"],
])
AM_CONDITIONAL([COMPLETION], [test "x$enable_completion" != "xno"])

+AC_ARG_ENABLE([tests],
+ [AS_HELP_STRING([--disable-tests], [Disable building tests])],
+ [], [enable_tests=yes])
+
+AM_CONDITIONAL([BUILD_TESTS], [test "x$enable_tests" != "xno"])
+
AX_VALGRIND_DFLT(memcheck, on)
AX_VALGRIND_DFLT(helgrind, off)
AX_VALGRIND_DFLT(drd, off)
@@ -290,10 +296,14 @@ AX_VALGRIND_CHECK
# ------------------------------------------------------------------------------
AC_CONFIG_FILES([
Makefile
- tools/tests/Makefile
libebgenv.pc
])

+# Only configure tools/tests/Makefile if tests are enabled
+if test "x$enable_tests" != "xno"; then
+ AC_CONFIG_FILES([tools/tests/Makefile])
+fi
+
AC_OUTPUT
AC_MSG_RESULT([
$PACKAGE_NAME $VERSION
diff --git a/tools/tests/Makefile.am b/tools/tests/Makefile.am
index d3fa5368c99f0b484fbb6fd57b848c581957aaef..7e3ac1e74b3456950033daa2586e60bea5913e0b 100644
--- a/tools/tests/Makefile.am
+++ b/tools/tests/Makefile.am
@@ -11,6 +11,7 @@
#
# SPDX-License-Identifier: GPL-2.0-only
#
+if BUILD_TESTS

OBJCOPY ?= objcopy

@@ -110,3 +111,5 @@ test_fat_LDADD = $(FAT_TESTLIB) $(LIBCHECK_LIBS)
TESTS = $(check_PROGRAMS)

@VALGRIND_CHECK_RULES@
+
+endif

---
base-commit: 452020920b3fe00ae0e2dc9d06f103107ded15c4
change-id: 20250224-wip-obbardc-disable-tests-fd6857b8ee49

Best regards,
--
Christopher Obbard <christoph...@linaro.org>

[Jan Kiszka]

That line is new, isn't it? Why do we need that now?

Rest looks good to me.

Thanks,
Jan

--
Siemens AG, Foundational Technologies
Linux Expert Center

[Christopher Obbard]

Hi Jan,

This was so I could be sure that tools/tests is always included. To be
honest, I don't know autotools so this is most likely not right ;-).
I will remove this line and send v2.

Thanks!

Chris

[Christopher Obbard]

For some builds, e.g. inside a minimal buildroot image, we may not want the
tests to be built. Add an option --disable-tests to configure to disable
building the tests.

Signed-off-by: Christopher Obbard <christoph...@linaro.org>
---

Changes in v2:
- Remove re-definition of EXTRA_DIST (thanks Jan).
- Rework confusing comment around SUBDIRS.
- Link to v1: https://groups.google.com/g/efibootguard-dev/c/tBKE9qT2hSo
---
Makefile.am | 8 ++++++--

configure.ac | 12 +++++++++++-
tools/tests/Makefile.am | 3 +++

3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index bb97df255dfb9c8a145eba3ed8dcc58ab9bd124b..911112da9d19bc97d248912fe394a89d85732fe4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -369,8 +369,12 @@ clean-local-completion-pycache:

check-valgrind-local: $(GEN_VERSION_H)

-# Tests depend on libraries being built - start with "."
-SUBDIRS = . tools/tests
+SUBDIRS = .
+
+# Only include tools/tests if tests are enabled

+if BUILD_TESTS
+SUBDIRS += tools/tests
+endif

--
Christopher Obbard <christoph...@linaro.org>

[Jan Kiszka]

Ah, you were concerned that if someone does "configure --disable-tests"
and then "make dist", the folder would be missing? Maybe worth a try -
I'm not automake guru either. :)

[Christopher Obbard]

Hi Jan,

Exactly that, but I doubt that's going to happen in practice ever ;-).
In V1 I actually redefined EXTRA_DIST anyway, it seems. So it was broken anyway.
I removed that EXTRA_DIST part in V2.

[Jan Kiszka]

On 25.02.25 19:02, Christopher Obbard wrote:
> For some builds, e.g. inside a minimal buildroot image, we may not want the
> tests to be built. Add an option --disable-tests to configure to disable
> building the tests.

Now playing with it again: tests are only build if explicitly requested
(make check), not via plain "make". What exactly is the problem you are
facing in that case?

Jan

PS: "make dist" is broken :(

[Christopher Obbard]

Hi Jan,

On Tue, 25 Feb 2025 at 18:11, Jan Kiszka <jan.k...@siemens.com> wrote:
>
> On 25.02.25 19:02, Christopher Obbard wrote:
> > For some builds, e.g. inside a minimal buildroot image, we may not want the
> > tests to be built. Add an option --disable-tests to configure to disable
> > building the tests.
>
> Now playing with it again: tests are only build if explicitly requested
> (make check), not via plain "make". What exactly is the problem you are
> facing in that case?

Thanks for that hint. Hmm, I am building efibootguard with buildroot
using the autoconf built-in abstraction (not entirely sure what
commands it is calling under the hood), which builds the tests
(linking to check and lots of other things).
When building the host tools, building the tests causes some
additional dependencies. So I am disabling the building of the tests
to workaround needing to package more things in buildroot.

I guess I need to check if/how buildroot is calling "make check" in my
efibootguard package. Again: buildroot is something I don't know well
enough, but know enough to cause trouble ;-).

[Jan Kiszka]

Just tested (after fixing "make dist" which NEVER worked): automake
seems to also consider those conditional SUBDIRS, automatically.

[Jan Kiszka]

On 25.02.25 19:16, Christopher Obbard wrote:
> Hi Jan,
>
> On Tue, 25 Feb 2025 at 18:11, Jan Kiszka <jan.k...@siemens.com> wrote:
>>
>> On 25.02.25 19:02, Christopher Obbard wrote:
>>> For some builds, e.g. inside a minimal buildroot image, we may not want the
>>> tests to be built. Add an option --disable-tests to configure to disable
>>> building the tests.
>>
>> Now playing with it again: tests are only build if explicitly requested
>> (make check), not via plain "make". What exactly is the problem you are
>> facing in that case?
>
> Thanks for that hint. Hmm, I am building efibootguard with buildroot
> using the autoconf built-in abstraction (not entirely sure what
> commands it is calling under the hood), which builds the tests
> (linking to check and lots of other things).
> When building the host tools, building the tests causes some
> additional dependencies. So I am disabling the building of the tests
> to workaround needing to package more things in buildroot.
>
> I guess I need to check if/how buildroot is calling "make check" in my
> efibootguard package. Again: buildroot is something I don't know well
> enough, but know enough to cause trouble ;-).

buildroot is just many makefiles and a little bit of kconfig ;).

Should be possible to figure out the exact rule it calls for building
(verbose mode?), and maybe you only need to tell it to not run package
tests? But if here is something in addition that causes the tests to be
built, we can still merge this here. I just want to understand the
reason better.

Jan

[Christopher Obbard]

Hi Jan,

I had a quick look at `host-autotools-package` in buildroot, and ran
my package build with `make V=1` it doesn't appear to call `make
check`.
So I guess my patch is still valid or there's something else wrong ?

[Jan Kiszka]

$ make
...
Making all in tools/tests
make[2]: Nothing to be done for 'all'.

Or are you already failing during configure, due to missing libcheck?
But that is something your patch is not even addressing.

[Christopher Obbard]

For some builds, e.g. inside a minimal buildroot image, we may not want the
tests to be built. Add an option --disable-tests to configure to disable
building the tests.

Signed-off-by: Christopher Obbard <christoph...@linaro.org>
---

Changes in v3:
- Only include libcheck if tests are enabled (thanks Jan).
- Link to v2: https://groups.google.com/g/efibootguard-dev/c/tBKE9qT2hSo/m/5xVQkMFoAgAJ

Changes in v2:
- Remove re-definition of EXTRA_DIST (thanks Jan).
- Rework confusing comment around SUBDIRS.
- Link to v1: https://groups.google.com/g/efibootguard-dev/c/tBKE9qT2hSo
---
Makefile.am | 8 ++++++--

configure.ac | 17 +++++++++++++++--
tools/tests/Makefile.am | 3 +++
3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index bb97df255dfb9c8a145eba3ed8dcc58ab9bd124b..911112da9d19bc97d248912fe394a89d85732fe4 100644
--- a/Makefile.am
+++ b/Makefile.am

@@ -369,8 +369,12 @@ clean-local-completion-pycache:

check-valgrind-local: $(GEN_VERSION_H)

-# Tests depend on libraries being built - start with "."
-SUBDIRS = . tools/tests

+SUBDIRS = .
+
+# Only include tools/tests if tests are enabled

+if BUILD_TESTS
+SUBDIRS += tools/tests
+endif

FORCE:

diff --git a/configure.ac b/configure.ac

index 36fc8d55e4190766871b8f9b5ea5bbaff5e24dde..fbfcc2c96f1d61ab5aa19e82f952507109160ca1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -261,7 +261,10 @@ AS_IF([test "x$enable_bootloader" != "xno"],
])
AM_CONDITIONAL([BOOTLOADER], [test "x$enable_bootloader" != "xno"])

-PKG_CHECK_MODULES(LIBCHECK, check)
+# Only include libcheck if tests are enabled

+if test "x$enable_tests" != "xno"; then

+ PKG_CHECK_MODULES(LIBCHECK, check)
+fi

PKG_INSTALLDIR
AC_SUBST(LIBEBGENV_VERSION, $(echo $VERSION | cut -dv -f2))
@@ -281,6 +284,12 @@ AS_IF([test "x$enable_completion" != "xno"],

])
AM_CONDITIONAL([COMPLETION], [test "x$enable_completion" != "xno"])

+AC_ARG_ENABLE([tests],
+ [AS_HELP_STRING([--disable-tests], [Disable building tests])],
+ [], [enable_tests=yes])
+
+AM_CONDITIONAL([BUILD_TESTS], [test "x$enable_tests" != "xno"])
+
AX_VALGRIND_DFLT(memcheck, on)
AX_VALGRIND_DFLT(helgrind, off)
AX_VALGRIND_DFLT(drd, off)

@@ -290,10 +299,14 @@ AX_VALGRIND_CHECK

Christopher Obbard <christoph...@linaro.org>

[Christopher Obbard]

Hi Jan,

I just did a fresh build (without my patch), and that is exactly what
I am hitting.
I sent a V3 which only checks for libcheck if tests are enabled.

Thanks,

Chris

[Jan Kiszka]

Thanks, applied.

[Christopher Obbard]

Hi Jan,

Hm, did I botch this patch ? Should this part come after the
definition of `AC_ARG_ENABLE([tests]` ?

It seems to build fine, but feels wrong to me to use a variable before
its definition ?

Do you want me to send a v4 (and you can revert the applied patch in
the next branch) or would you rather a follow-up with Fixes tag?

Thanks

Chris

[Jan Kiszka]

Please send a v4, it will replace v3 in next (which can be rebasing, in
contrast to master).

[Christopher Obbard]

For some builds, e.g. inside a minimal buildroot image, we may not want the
tests to be built. Add an option --disable-tests to configure to disable
building the tests.

Signed-off-by: Christopher Obbard <christoph...@linaro.org>
---

Changes in v4:
- Move enable_tests definition to before its use.
- Link to v3: https://groups.google.com/g/efibootguard-dev/c/tBKE9qT2hSo/m/eeHFlYpsAgAJ

index 36fc8d55e4190766871b8f9b5ea5bbaff5e24dde..16d25b988a048c49aaefae4ecfa24b5f1d8a2b77 100644
--- a/configure.ac
+++ b/configure.ac
@@ -261,7 +261,16 @@ AS_IF([test "x$enable_bootloader" != "xno"],

])
AM_CONDITIONAL([BOOTLOADER], [test "x$enable_bootloader" != "xno"])

-PKG_CHECK_MODULES(LIBCHECK, check)

+AC_ARG_ENABLE([tests],
+ [AS_HELP_STRING([--disable-tests], [Disable building tests])],
+ [], [enable_tests=yes])
+
+AM_CONDITIONAL([BUILD_TESTS], [test "x$enable_tests" != "xno"])
+

+# Only include libcheck if tests are enabled
+if test "x$enable_tests" != "xno"; then
+ PKG_CHECK_MODULES(LIBCHECK, check)
+fi

PKG_INSTALLDIR
AC_SUBST(LIBEBGENV_VERSION, $(echo $VERSION | cut -dv -f2))

@@ -290,10 +299,14 @@ AX_VALGRIND_CHECK
# ------------------------------------------------------------------------------
AC_CONFIG_FILES([
Makefile
- tools/tests/Makefile
libebgenv.pc
])

+# Only configure tools/tests/Makefile if tests are enabled

+if test "x$enable_tests" != "xno"; then

--
Christopher Obbard <christoph...@linaro.org>

[Jan Kiszka]

Thanks, replaced v3 with this one in next.