[PATCH] mtd-utils: Add configure switches to disable jffsX or ubifs tools

[PATCH] mtd-utils: Add configure switches to disable jffsX or ubifs tools

[David Oberhollenzer]

For some applications, like building a root filesystem for an embedded
device, it may be desireable to only build and install a subset of the
mtd-utils. This can be done throught the targets of the generated
Makefile and hand picking executables, however the jffsX and ubifs
utilities have external build dependencies that may not be needed.

This patch adds configure switches to disable building the jffsX and
ubifs utilities. Their respective build dependencies (zlib, lzo, uuid)
are only requested if the tools are being built.

Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>
---
 Makefile.am  |  8 +++++++-
 configure.ac | 49 ++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 23c1940..31b21bc 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -36,11 +36,17 @@ EXTRA_DIST += $(GLOBAL_HEADER) $(GLOBAL_EXTRA)
 
 include lib/Makemodule.am
 include ubi-utils/Makemodule.am
-include ubifs-utils/Makemodule.am
 include misc-utils/Makemodule.am
 include nand-utils/Makemodule.am
 include nor-utils/Makemodule.am
+
+if BUILD_UBIFS
+include ubifs-utils/Makemodule.am
+endif
+
+if BUILD_JFFSX
 include jffsX-utils/Makemodule.am
+endif
 
 if BUILD_TESTS
 include tests/ubi-tests/Makemodule.am
diff --git a/configure.ac b/configure.ac
index 5196478..be0d17e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -66,6 +66,48 @@ AC_CONFIG_FILES([tests/fs-tests/fs_help_all.sh
 	tests/ubi-tests/ubi-stress-test.sh])
 
 
+need_uuid="no"
+need_zlib="no"
+need_lzo="no"
+
+AC_ARG_WITH([jffs],
+	[AS_HELP_STRING([--without-jffs], [Disable jffsX utilities])],
+	[case "${withval}" in
+	yes) AM_CONDITIONAL([BUILD_JFFSX], [true]) ;;
+	no)  AM_CONDITIONAL([BUILD_JFFSX], [false]) ;;
+	*) AC_MSG_ERROR([bad value ${withval} for --without-jffs]) ;;
+	esac],
+	[AM_CONDITIONAL([BUILD_JFFSX], [true])])
+
+AC_ARG_WITH([ubifs],
+	[AS_HELP_STRING([--without-ubifs], [Disable ubifs utilities])],
+	[case "${withval}" in
+	yes) AM_CONDITIONAL([BUILD_UBIFS], [true]) ;;
+	no)  AM_CONDITIONAL([BUILD_UBIFS], [false]) ;;
+	*) AC_MSG_ERROR([bad value ${withval} for --without-ubifs]) ;;
+	esac],
+	[AM_CONDITIONAL([BUILD_UBIFS], [true])])
+
+AM_COND_IF([BUILD_UBIFS], [
+	need_uuid="yes"
+	need_zlib="yes"
+	need_lzo="yes"
+])
+
+AM_COND_IF([BUILD_JFFSX], [
+	need_zlib="yes"
+	need_lzo="yes"
+])
+
+if test "x$need_zlib" = "xyes"; then
+	PKG_CHECK_MODULES(ZLIB, [zlib])
+fi
+
+if test "x$need_uuid" = "xyes"; then
+	PKG_CHECK_MODULES(UUID, [uuid])
+fi
+
+
 AC_ARG_WITH([xattr],
 	[AS_HELP_STRING([--without-xattr],
 		[Disable support forextended file attributes])],
@@ -83,19 +125,16 @@ AC_CHECK_HEADER(sys/acl.h, [], [AM_CONDITIONAL([WITHOUT_XATTR], [true])])
 AC_ARG_WITH([lzo],
 	[AS_HELP_STRING([--without-lzo], [Disable support for LZO compression])],
 	[case "${withval}" in
-	yes) AM_CONDITIONAL([WITHOUT_LZO], [false]) ;;
+	yes) AM_CONDITIONAL([WITHOUT_LZO], [test "x$need_lzo" != "xyes"]) ;;
 	no)  AM_CONDITIONAL([WITHOUT_LZO], [true]) ;;
 	*) AC_MSG_ERROR([bad value ${withval} for --without-lzo]) ;;
 	esac],
-	[AM_CONDITIONAL([WITHOUT_LZO], [false])])
+	[AM_CONDITIONAL([WITHOUT_LZO], [test "x$need_lzo" != "xyes"])])
 
 
 AC_CHECK_HEADERS([execinfo.h], [execinfo_found=yes])
 AM_CONDITIONAL([HAVE_EXECINFO], [test "x$execinfo_found" == "xyes"])
 
-PKG_CHECK_MODULES(ZLIB, [ zlib ])
-PKG_CHECK_MODULES(UUID, [ uuid ])
-
 AM_COND_IF([UNIT_TESTS], [PKG_CHECK_MODULES(CMOCKA, [ cmocka ])], [])
 
 AM_COND_IF([WITHOUT_LZO], [], [
-- 
2.11.0

Re: [PATCH] mtd-utils: Add configure switches to disable jffsX or ubifs tools

[Richard Weinberger]

On 21.12.2016 16:54, David Oberhollenzer wrote:
> For some applications, like building a root filesystem for an embedded
> device, it may be desireable to only build and install a subset of the
> mtd-utils. This can be done throught the targets of the generated
> Makefile and hand picking executables, however the jffsX and ubifs
> utilities have external build dependencies that may not be needed.
> 
> This patch adds configure switches to disable building the jffsX and
> ubifs utilities. Their respective build dependencies (zlib, lzo, uuid)
> are only requested if the tools are being built.
> 
> Signed-off-by: David Oberhollenzer <david.oberhollenzer@sigma-star.at>

Reviewed-by: Richard Weinberger <richard@nod.at>

Thanks,
//richard

Re: [PATCH] mtd-utils: Add configure switches to disable jffsX or ubifs tools

[Thomas Petazzoni]

Hello,

On Wed, 21 Dec 2016 16:54:12 +0100, David Oberhollenzer wrote:

> +AC_ARG_WITH([jffs],
> +	[AS_HELP_STRING([--without-jffs], [Disable jffsX utilities])],
> +	[case "${withval}" in
> +	yes) AM_CONDITIONAL([BUILD_JFFSX], [true]) ;;
> +	no)  AM_CONDITIONAL([BUILD_JFFSX], [false]) ;;
> +	*) AC_MSG_ERROR([bad value ${withval} for --without-jffs]) ;;
> +	esac],
> +	[AM_CONDITIONAL([BUILD_JFFSX], [true])])
> +
> +AC_ARG_WITH([ubifs],
> +	[AS_HELP_STRING([--without-ubifs], [Disable ubifs utilities])],
> +	[case "${withval}" in
> +	yes) AM_CONDITIONAL([BUILD_UBIFS], [true]) ;;
> +	no)  AM_CONDITIONAL([BUILD_UBIFS], [false]) ;;
> +	*) AC_MSG_ERROR([bad value ${withval} for --without-ubifs]) ;;
> +	esac],
> +	[AM_CONDITIONAL([BUILD_UBIFS], [true])])

If the only two correct values are "yes" or "no", then you should use
AC_ARG_ENABLE() and not AC_ARG_WITH().

Also, I believe a much more autoconf-ish way of writing this is:

AC_ARG_ENABLE([jffs],
	[AS_HELP_STRING([--without-jffs], [Disable jffsX utilities])],
	[], [enable_jffs=yes])

if "${enable_jffs}" = "yes" ; then
	need_uuid="yes"
	need_zlib="yes"
	need_lzo="yes"
fi

Indeed, using AM_CONDITIONAL() is useful to assign automake variable.
Here, BUILD_JFFS and BUILD_UBIFS are apparently only used within the
configure.ac, so using AM_CONDITIONAL() is useless.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH] mtd-utils: Add configure switches to disable jffsX or ubifs tools

[David Oberhollenzer]

On 12/22/2016 10:19 AM, Thomas Petazzoni wrote:
> If the only two correct values are "yes" or "no", then you should use
> AC_ARG_ENABLE() and not AC_ARG_WITH().
That is indeed an interesting consideration. The guides seem to be a bit
blury on the destinction on enable/disable vs with/without.

I was under the impression that one should use enable/disable for features
of a program and with/without for external dependencies or entire programs,
i.e. "features of a project".

> Indeed, using AM_CONDITIONAL() is useful to assign automake variable.
> Here, BUILD_JFFS and BUILD_UBIFS are apparently only used within the
> configure.ac, so using AM_CONDITIONAL() is useless.
Not quite. They are used in the automake file to skip building mkfs.ubifs
and the jffsx utils.


Thanks,

David

Re: [PATCH] mtd-utils: Add configure switches to disable jffsX or ubifs tools

[Thomas Petazzoni]

Hello,

On Thu, 22 Dec 2016 10:27:36 +0100, David Oberhollenzer wrote:

> That is indeed an interesting consideration. The guides seem to be a bit
> blury on the destinction on enable/disable vs with/without.
> 
> I was under the impression that one should use enable/disable for features
> of a program and with/without for external dependencies or entire programs,
> i.e. "features of a project".

Yes, indeed that the general distinction between enable/disable and
with/without, you're right.

What I found odd in your code is the fact that you're in the end only
accepting yes or no, and the duplication of the AM_CONDITIONAL()
statements.

> > Indeed, using AM_CONDITIONAL() is useful to assign automake variable.
> > Here, BUILD_JFFS and BUILD_UBIFS are apparently only used within the
> > configure.ac, so using AM_CONDITIONAL() is useless.  
> Not quite. They are used in the automake file to skip building mkfs.ubifs
> and the jffsx utils.

Ah, okay. I haven't looked at the code base, just the patch itself, so
I definitely missed that. So an AM_CONDITIONAL() is definitely good.

But regardless of whether you go with AC_ARG_WITH() or AC_ARG_ENABLE(),
I still believe it's a lot more readable to have the AM_CONDITIONAL()
test *after* the AC_ARG_{WITH,ENABLE}.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com