[Buildroot] [PATCH] mtd: add option to build mkfs.ubifs for target

[Buildroot] [PATCH] mtd: add option to build mkfs.ubifs for target

[Paul B. Henson]

Signed-off-by: Paul B. Henson <henson@acm.org>
---
 package/mtd/Config.in |    6 ++++++
 package/mtd/mtd.mk    |    7 +++++++
 2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/package/mtd/Config.in b/package/mtd/Config.in
index ddc3737..2e13157 100644
--- a/package/mtd/Config.in
+++ b/package/mtd/Config.in
@@ -54,6 +54,12 @@ config BR2_PACKAGE_MTD_MKFSJFFS2
 	select BR2_PACKAGE_ZLIB
 	select BR2_PACKAGE_LZO
 
+config BR2_PACKAGE_MTD_MKFSUBIFS
+	bool "mkfs.ubifs"
+	select BR2_PACKAGE_ZLIB
+	select BR2_PACKAGE_LZO
+	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
+
 config BR2_PACKAGE_MTD_MTD_DEBUG
 	bool "mtd_debug"
 	default y
diff --git a/package/mtd/mtd.mk b/package/mtd/mtd.mk
index 994a73a..55fa727 100644
--- a/package/mtd/mtd.mk
+++ b/package/mtd/mtd.mk
@@ -14,6 +14,10 @@ ifeq ($(BR2_PACKAGE_MTD_MKFSJFFS2),y)
 MTD_DEPENDENCIES = zlib lzo
 endif
 
+ifeq ($(BR2_PACKAGE_MTD_MKFSUBIFS),y)
+MTD_DEPENDENCIES += util-linux zlib lzo
+endif
+
 ifeq ($(BR2_PACKAGE_BUSYBOX),y)
 MTD_DEPENDENCIES += busybox
 endif
@@ -71,7 +75,10 @@ MTD_TARGETS_UBI_$(BR2_PACKAGE_MTD_UBIRMVOL)	+= ubirmvol
 MTD_TARGETS_UBI_$(BR2_PACKAGE_MTD_UBIRSVOL)	+= ubirsvol
 MTD_TARGETS_UBI_$(BR2_PACKAGE_MTD_UBIUPDATEVOL)	+= ubiupdatevol
 
+MTD_TARGETS_UBIFS_$(BR2_PACKAGE_MTD_MKFSUBIFS)	+= mkfs.ubifs
+
 MTD_TARGETS_y += $(addprefix ubi-utils/,$(MTD_TARGETS_UBI_y))
+MTD_TARGETS_y += $(addprefix mkfs.ubifs/,$(MTD_TARGETS_UBIFS_y))
 
 # only call make if atleast a single tool is enabled
 ifneq ($(MTD_TARGETS_y),)
-- 
1.7.8.6

[Buildroot] [PATCH] mtd: add option to build mkfs.ubifs for target

[Thomas De Schampheleire]

Hi Paul,

Thanks for contributing! See below for a small comment.

On Mon, Jul 29, 2013 at 4:11 AM, Paul B. Henson <henson@acm.org> wrote:
> Signed-off-by: Paul B. Henson <henson@acm.org>
> ---
>  package/mtd/Config.in |    6 ++++++
>  package/mtd/mtd.mk    |    7 +++++++
>  2 files changed, 13 insertions(+), 0 deletions(-)
>
> diff --git a/package/mtd/Config.in b/package/mtd/Config.in
> index ddc3737..2e13157 100644
> --- a/package/mtd/Config.in
> +++ b/package/mtd/Config.in
> @@ -54,6 +54,12 @@ config BR2_PACKAGE_MTD_MKFSJFFS2
>         select BR2_PACKAGE_ZLIB
>         select BR2_PACKAGE_LZO
>
> +config BR2_PACKAGE_MTD_MKFSUBIFS
> +       bool "mkfs.ubifs"
> +       select BR2_PACKAGE_ZLIB
> +       select BR2_PACKAGE_LZO
> +       select BR2_PACKAGE_UTIL_LINUX_LIBUUID
> +
>  config BR2_PACKAGE_MTD_MTD_DEBUG
>         bool "mtd_debug"
>         default y
> diff --git a/package/mtd/mtd.mk b/package/mtd/mtd.mk
> index 994a73a..55fa727 100644
> --- a/package/mtd/mtd.mk
> +++ b/package/mtd/mtd.mk
> @@ -14,6 +14,10 @@ ifeq ($(BR2_PACKAGE_MTD_MKFSJFFS2),y)
>  MTD_DEPENDENCIES = zlib lzo
>  endif
>
> +ifeq ($(BR2_PACKAGE_MTD_MKFSUBIFS),y)
> +MTD_DEPENDENCIES += util-linux zlib lzo
> +endif
> +
>  ifeq ($(BR2_PACKAGE_BUSYBOX),y)
>  MTD_DEPENDENCIES += busybox
>  endif
> @@ -71,7 +75,10 @@ MTD_TARGETS_UBI_$(BR2_PACKAGE_MTD_UBIRMVOL)  += ubirmvol
>  MTD_TARGETS_UBI_$(BR2_PACKAGE_MTD_UBIRSVOL)    += ubirsvol
>  MTD_TARGETS_UBI_$(BR2_PACKAGE_MTD_UBIUPDATEVOL)        += ubiupdatevol
>
> +MTD_TARGETS_UBIFS_$(BR2_PACKAGE_MTD_MKFSUBIFS) += mkfs.ubifs
> +
>  MTD_TARGETS_y += $(addprefix ubi-utils/,$(MTD_TARGETS_UBI_y))
> +MTD_TARGETS_y += $(addprefix mkfs.ubifs/,$(MTD_TARGETS_UBIFS_y))

I find it a bit overkill to use $(addprefix) for just one binary. My
suggestion would be to simply:
MTD_TARGETS_$(BR2_PACKAGE_MTD_MKFSUBIFS)          += mkfs.ubifs/mkfs.ubifs

Otherwise, the patch looks good.

Best regards,
Thomas

[Buildroot] [PATCH] mtd: add option to build mkfs.ubifs for target

[Paul B. Henson]

On Mon, Jul 29, 2013 at 02:47:34PM +0200, Thomas De Schampheleire wrote:

> I find it a bit overkill to use $(addprefix) for just one binary. My
> suggestion would be to simply:
> MTD_TARGETS_$(BR2_PACKAGE_MTD_MKFSUBIFS)          += mkfs.ubifs/mkfs.ubifs

I must confess my patch was based on observation and duplication, not
on a deep understanding of internals ;). I'll make this change and
resubmit the patch, thanks for the feedback.

[Buildroot] [PATCH] mtd: add option to build mkfs.ubifs for target

[Thomas Petazzoni]

Dear Paul B. Henson,

On Sun, 28 Jul 2013 19:11:50 -0700, Paul B. Henson wrote:
> Signed-off-by: Paul B. Henson <henson@acm.org>
> ---
>  package/mtd/Config.in |    6 ++++++
>  package/mtd/mtd.mk    |    7 +++++++
>  2 files changed, 13 insertions(+), 0 deletions(-)

Applied, thanks. I've made the changes suggested by Thomas De
Schampheleire, as well as added the necessary 'depends on' statements
for the requires toolchain features.

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH] mtd: add option to build mkfs.ubifs for target

[Paul B. Henson]

On 7/29/2013 11:51 AM, Thomas Petazzoni wrote:

> Applied, thanks. I've made the changes suggested by Thomas De
> Schampheleire, as well as added the necessary 'depends on' statements
> for the requires toolchain features.

Cool, I appreciate you tuning it up for me.

It looks like you copied the dependencies from util-linux? I'm not that 
familiar with kconfig, there's no concept of transitive dependencies, 
such that if mkfs.ubifs selects util-linux, it will automatically 
inherit its dependencies? If I'm understanding correctly, if at some 
point util-linux changes dependencies, the mtd config for mkfs.ubifs 
would also need to be updated explicitly to capture them?

Thanks much?

[Buildroot] [PATCH] mtd: add option to build mkfs.ubifs for target

[Thomas Petazzoni]

Dear Paul B. Henson,

On Tue, 30 Jul 2013 19:13:06 -0700, Paul B. Henson wrote:

> It looks like you copied the dependencies from util-linux? I'm not that 
> familiar with kconfig, there's no concept of transitive dependencies, 
> such that if mkfs.ubifs selects util-linux, it will automatically 
> inherit its dependencies? If I'm understanding correctly, if at some 
> point util-linux changes dependencies, the mtd config for mkfs.ubifs 
> would also need to be updated explicitly to capture them?

That's correct. When you "select FOO", kconfig ignores the dependencies
that "config FOO" may have, so you have to manually propagate them.
That's a bit annoying, but we don't have (yet?) enough understanding of
kconfig internals to change that. Yann E. Morin is getting more and
more knowledge about kconfig internals, so I'm hoping it may happen
some day, but for now, we have to propagate those dependencies manually.

Thanks!

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com