[Buildroot] [PATCH v5 2/2] systemd: select fsck wrapper from util-linux

[Buildroot] [PATCH v5 2/2] systemd: select fsck wrapper from util-linux

[Carlos Santos]

Select the fsck required by systemd provided by util-linux. This
prevents ending up with fsck from busybox, which is incompatible
with systemd.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
 package/systemd/Config.in | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/package/systemd/Config.in b/package/systemd/Config.in
index 0f54686..f5eb9d0 100644
--- a/package/systemd/Config.in
+++ b/package/systemd/Config.in
@@ -22,6 +22,9 @@ menuconfig BR2_PACKAGE_SYSTEMD
 	select BR2_PACKAGE_UTIL_LINUX_BINARIES
 	select BR2_PACKAGE_UTIL_LINUX_MOUNT
 	select BR2_PACKAGE_UTIL_LINUX_NOLOGIN
+	select BR2_PACKAGE_UTIL_LINUX_FSCK
+	select BR2_PACKAGE_E2FSPROGS
+	select BR2_PACKAGE_E2FSPROGS_E2FSCK
 	select BR2_PACKAGE_KMOD
 	select BR2_PACKAGE_BUSYBOX_SHOW_OTHERS # kmod-tools
 	select BR2_PACKAGE_KMOD_TOOLS
-- 
2.7.4

[Buildroot] [PATCH v5 2/2] systemd: select fsck wrapper from util-linux

[Arnout Vandecappelle]

On 10-04-17 05:54, Carlos Santos wrote:
> Select the fsck required by systemd provided by util-linux. This
> prevents ending up with fsck from busybox, which is incompatible
> with systemd.
> 
> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> ---

 The patch changelog is missing, which in this case would be relevant because...

>  package/systemd/Config.in | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/package/systemd/Config.in b/package/systemd/Config.in
> index 0f54686..f5eb9d0 100644
> --- a/package/systemd/Config.in
> +++ b/package/systemd/Config.in
> @@ -22,6 +22,9 @@ menuconfig BR2_PACKAGE_SYSTEMD
>  	select BR2_PACKAGE_UTIL_LINUX_BINARIES
>  	select BR2_PACKAGE_UTIL_LINUX_MOUNT
>  	select BR2_PACKAGE_UTIL_LINUX_NOLOGIN
> +	select BR2_PACKAGE_UTIL_LINUX_FSCK
> +	select BR2_PACKAGE_E2FSPROGS
> +	select BR2_PACKAGE_E2FSPROGS_E2FSCK

 In reply to my comment on v3 you wrote:

"Good point. I will submit a new patch set removing the selection of
e2fsprogs."

 Something went wrong? You changed your mind?

 Regards,
 Arnout

>  	select BR2_PACKAGE_KMOD
>  	select BR2_PACKAGE_BUSYBOX_SHOW_OTHERS # kmod-tools
>  	select BR2_PACKAGE_KMOD_TOOLS
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v6 1/2] e2fsprogs: refactor to fix conflicts with busybox and util-linux

[Carlos Santos]

So far we attempted to solve the conflicts between busybox and e2fsprogs
by removing busybox programs from /bin and /sbin, leaving the e2fsprogs
ones at /usr/bin and /usr/sbin. This fails with BR2_ROOTFS_MERGED_USR=y,
leading to situations like the one described in bug 9436.

We could provide a better solution by means of a fine-grained selection
of programs, like util-linux does, but this would require big changes in
e2fsprogs. So instead of resorting to dirty tricks we switch to a more
pragmatic approach:

1. Drop all configs to select/deselect utilities without corresponding
   --enable/disable options to the configure script. In other words, we
   always install the basic set of utilities.
2. Do not try to build UUID utilities, since they are disabled along
   with libuuid (the util-linux ones must be used instead).
3. Install e2fsprogs utilities at /bin and /sbin, overriding the ones
   eventually installed by busybox.

Notice that these changes do exactly the opposite of what is requested
in bug 9436. On the other hand the policy for e2fsprogs becomes coherent
with the one for util-linux: busybox never wins.

Since e2fsprogs depends on util-linux, it's built later. So in order to
have e2fsck from e2fsprogs and the fsck wrapper from util-linux (which
is better maintained and compatible with systemd) we disable e2fsprogs'
fsck if the one from util-linux is selected.

Fixes:
  https://bugs.busybox.net/show_bug.cgi?id=9436 (no fix, in fact)

Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
Changes v1->v2
  - Add depenndece on !BR2_PACKAGE_UTIL_LINUX_FSCK to
    BR2_PACKAGE_E2FSPROGS_FSCK
  - Add comment telling that the fsck from util-linux has preference.
  - Pass "--enable-fsck" conditionally to configure
  - Remove the guard around the removal of /usr/sbin/fsck, since the
    fsck from busybox and util-linux are installed at /sbin
  - Improve comments and commit message
Changes v2->v3
  - Pass "--disable-fsck" conditionaly to configure too, since enable
    is the default.
  - Do not attempt to remove $(TARGET_DIR)/usr/sbin/fsck, just the
    $(TARGET_DIR)/sbin/fsck from busybox.
Changes v3->v4
   - Improve commit message
Changes v4->v5
   - Extreme overhauling, deeply simplifying the recipe and following
     the suggestion from Arnout Vandecappelle to install e2fsprogs
     utilities on /bin and /sbin.
Changes v5->v6
   - Fix typos and make small changes in commit message
---
 package/e2fsprogs/Config.in      |  84 +++++++---------------
 package/e2fsprogs/Config.in.host |   1 +
 package/e2fsprogs/e2fsprogs.mk   | 149 +++++++++++----------------------------
 3 files changed, 68 insertions(+), 166 deletions(-)

diff --git a/package/e2fsprogs/Config.in b/package/e2fsprogs/Config.in
index d1914a9..776645f 100644
--- a/package/e2fsprogs/Config.in
+++ b/package/e2fsprogs/Config.in
@@ -1,101 +1,69 @@
-config BR2_PACKAGE_E2FSPROGS
+menuconfig BR2_PACKAGE_E2FSPROGS
 	bool "e2fsprogs"
 	depends on BR2_USE_MMU # util-linux/libblkid
 	select BR2_PACKAGE_UTIL_LINUX
 	select BR2_PACKAGE_UTIL_LINUX_LIBBLKID
-	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
 	help
 	  The EXT2 file system utilities.
 
-	  http://e2fsprogs.sf.net
+	  badblocks chattr debugfs dumpe2fs e2freefrag e2fsck e2image
+	  e2undo e4crypt e4defrag filefrag fsck fuse2fs logsave lsattr
+	  mke2fs mklost+found resize2fs tune2fs
 
-if BR2_PACKAGE_E2FSPROGS
 
-config BR2_PACKAGE_E2FSPROGS_BADBLOCKS
-	bool "badblocks"
-	default y
+	  http://e2fsprogs.sourceforge.net/
 
-config BR2_PACKAGE_E2FSPROGS_CHATTR
-	bool "chattr"
-	default y
+if BR2_PACKAGE_E2FSPROGS
+
+comment "Basic set: badblocks chattr dumpe2fs e2freefrag e2fsck e2undo"
+comment "e4crypt e4defrag filefrag logsave lsattr mke2fs mklost+found"
+comment "tune2fs"
 
 config BR2_PACKAGE_E2FSPROGS_DEBUGFS
 	bool "debugfs"
-
-config BR2_PACKAGE_E2FSPROGS_DUMPE2FS
-	bool "dumpe2fs"
-	default y
-
-config BR2_PACKAGE_E2FSPROGS_E2FREEFRAG
-	bool "e2freefrag"
-	default y
-
-config BR2_PACKAGE_E2FSPROGS_E2FSCK
-	bool "e2fsck"
-	default y
+	help
+	  ext2/ext3/ext4 file system debugger
 
 config BR2_PACKAGE_E2FSPROGS_E2IMAGE
 	bool "e2image"
-
-config BR2_PACKAGE_E2FSPROGS_E2LABEL
-	bool "e2label"
-	default y
-
-config BR2_PACKAGE_E2FSPROGS_E2UNDO
-	bool "e2undo"
-	default y
+	help
+	  Save critical ext2/ext3/ext4 filesystem metadata to a file
 
 config BR2_PACKAGE_E2FSPROGS_E4DEFRAG
 	bool "e4defrag"
 	depends on !BR2_nios2 # fallocate not implemented
 	depends on !BR2_TOOLCHAIN_USES_UCLIBC # sync_file_range not impl
+	help
+	  Online defragmenter for ext4 filesystem
 
 comment "e4defrag needs a glibc or musl toolchain"
 	depends on BR2_TOOLCHAIN_USES_UCLIBC
 
-config BR2_PACKAGE_E2FSPROGS_FILEFRAG
-	bool "filefrag"
-	default y
-
 config BR2_PACKAGE_E2FSPROGS_FSCK
 	bool "fsck"
+	depends on !BR2_PACKAGE_UTIL_LINUX_FSCK
 	default y
+	help
+	  Check and repair a Linux file system
+
+comment "the fsck from util-linux has preference over this one"
+	depends on BR2_PACKAGE_UTIL_LINUX_FSCK
 
 config BR2_PACKAGE_E2FSPROGS_FUSE2FS
 	bool "fuse2fs"
 	depends on !BR2_STATIC_LIBS # libfuse
 	depends on BR2_TOOLCHAIN_HAS_THREADS # libfuse
 	select BR2_PACKAGE_LIBFUSE
+	help
+	  FUSE file system client for ext2/ext3/ext4 file systems
 
 comment "fuse2fs needs a toolchain w/ threads, dynamic library"
 	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS
 
-config BR2_PACKAGE_E2FSPROGS_LOGSAVE
-	bool "logsave"
-	default y
-
-config BR2_PACKAGE_E2FSPROGS_LSATTR
-	bool "lsattr"
-	default y
-
-config BR2_PACKAGE_E2FSPROGS_MKE2FS
-	bool "mke2fs"
-	default y
-
-config BR2_PACKAGE_E2FSPROGS_MKLOSTFOUND
-	bool "mklost+found"
-	default y
-
 config BR2_PACKAGE_E2FSPROGS_RESIZE2FS
 	bool "resize2fs"
-
-config BR2_PACKAGE_E2FSPROGS_TUNE2FS
-	bool "tune2fs"
-	default y
-	select BR2_PACKAGE_E2FSPROGS_E2LABEL
-
-config BR2_PACKAGE_E2FSPROGS_UUIDGEN
-	bool "uuidgen"
 	default y
+	help
+	  ext2/ext3/ext4 file system resizer
 
 endif
diff --git a/package/e2fsprogs/Config.in.host b/package/e2fsprogs/Config.in.host
index de9148e..7591bbc 100644
--- a/package/e2fsprogs/Config.in.host
+++ b/package/e2fsprogs/Config.in.host
@@ -1,5 +1,6 @@
 config BR2_PACKAGE_HOST_E2FSPROGS
 	bool "host e2fsprogs"
+	select BR2_PACKAGE_HOST_UTIL_LINUX
 	help
 	  The EXT2/3/4 file system utilities.
 
diff --git a/package/e2fsprogs/e2fsprogs.mk b/package/e2fsprogs/e2fsprogs.mk
index 3f235c5..e172914 100644
--- a/package/e2fsprogs/e2fsprogs.mk
+++ b/package/e2fsprogs/e2fsprogs.mk
@@ -10,36 +10,58 @@ E2FSPROGS_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/people/tytso/e2fsprogs/v$(E2F
 E2FSPROGS_LICENSE = GPL-2.0, BSD-3-Clause (libuuid), MIT-like with advertising clause (libss and libet)
 E2FSPROGS_LICENSE_FILES = NOTICE lib/uuid/COPYING lib/ss/mit-sipb-copyright.h lib/et/internal.h
 E2FSPROGS_INSTALL_STAGING = YES
+
 E2FSPROGS_DEPENDENCIES = host-pkgconf util-linux
-# we don't have a host-util-linux
-HOST_E2FSPROGS_DEPENDENCIES = host-pkgconf
+
+HOST_E2FSPROGS_DEPENDENCIES = host-pkgconf host-util-linux
+
+# If both e2fsprogs and busybox are selected, make certain e2fsprogs
+# wins the fight over who gets to have their utils actually installed
+ifeq ($(BR2_PACKAGE_BUSYBOX),y)
+E2FSPROGS_DEPENDENCIES += busybox
+endif
+
+ifeq ($(BR2_PACKAGE_E2FSPROGS_FUSE2FS),y)
+E2FSPROGS_DEPENDENCIES += libfuse
+endif
 
 # e4defrag doesn't build on older systems like RHEL5.x, and we don't
 # need it on the host anyway.
 # Disable fuse2fs as well to avoid carrying over deps, and it's unused
-HOST_E2FSPROGS_CONF_OPTS += --disable-defrag --disable-fuse2fs
+# Set the binary directories to "$(HOST_DIR)/bin" and "$(HOST_DIR)/sbin"
+# to match the target paths (the $(HOST_DIR) prefix is required because
+# passing DESTDIR=$(HOST_DIR) in the install command does not work).
+HOST_E2FSPROGS_CONF_OPTS = \
+	--disable-defrag \
+	--disable-fuse2fs \
+	--bindir=$(HOST_DIR)/bin \
+	--sbindir=$(HOST_DIR)/sbin \
+	--enable-symlink-install \
+	--disable-libblkid \
+	--disable-libuuid \
+	--disable-e2initrd-helper \
+	--disable-testio-debug \
+	--disable-rpath
 
+# Set the binary directories to "/bin" and "/sbin" to override programs
+# installed by busybox.
 E2FSPROGS_CONF_OPTS = \
-	$(if $(BR2_STATIC_LIBS),,--enable-elf-shlibs) \
-	$(if $(BR2_PACKAGE_E2FSPROGS_DEBUGFS),,--disable-debugfs) \
-	$(if $(BR2_PACKAGE_E2FSPROGS_E2IMAGE),,--disable-imager) \
-	$(if $(BR2_PACKAGE_E2FSPROGS_E4DEFRAG),,--disable-defrag) \
-	$(if $(BR2_PACKAGE_E2FSPROGS_RESIZE2FS),,--disable-resizer) \
-	--disable-uuidd \
+	$(if $(BR2_STATIC_LIBS),--disable-elf-shlibs,--enable-elf-shlibs) \
+	$(if $(BR2_PACKAGE_E2FSPROGS_DEBUGFS),--enable-debugfs,--disable-debugfs) \
+	$(if $(BR2_PACKAGE_E2FSPROGS_E2IMAGE),--enable-imager,--disable-imager) \
+	$(if $(BR2_PACKAGE_E2FSPROGS_E4DEFRAG),--enable-defrag,--disable-defrag) \
+	$(if $(BR2_PACKAGE_E2FSPROGS_FSCK),--enable-fsck,--disable-fsck) \
+	$(if $(BR2_PACKAGE_E2FSPROGS_FUSE2FS),--enable-fuse2fs,--disable-fuse2fs) \
+	$(if $(BR2_PACKAGE_E2FSPROGS_RESIZE2FS),--enable-resizer,--disable-resizer) \
+	--bindir=/bin \
+	--sbindir=/sbin \
+	--enable-symlink-install \
 	--disable-libblkid \
 	--disable-libuuid \
-	--enable-fsck \
 	--disable-e2initrd-helper \
 	--disable-testio-debug \
 	--disable-rpath
 
-ifeq ($(BR2_PACKAGE_E2FSPROGS_FUSE2FS),y)
-E2FSPROGS_CONF_OPTS += --enable-fuse2fs
-E2FSPROGS_DEPENDENCIES += libfuse
-else
-E2FSPROGS_CONF_OPTS += --disable-fuse2fs
-endif
-
 ifeq ($(BR2_nios2),y)
 E2FSPROGS_CONF_ENV += ac_cv_func_fallocate=no
 endif
@@ -56,17 +78,13 @@ HOST_E2FSPROGS_CONF_ENV += \
 	ac_cv_header_magic_h=no \
 	ac_cv_lib_magic_magic_file=no
 
-ifeq ($(BR2_NEEDS_GETTEXT_IF_LOCALE),y)
-# util-linux libuuid pulls in libintl if needed, so ensure we also
-# link against it, otherwise static linking fails
-E2FSPROGS_CONF_ENV += LIBS=-lintl
-endif
-
 E2FSPROGS_MAKE_OPTS = LDCONFIG=true
+
 E2FSPROGS_INSTALL_STAGING_OPTS = \
 	DESTDIR=$(STAGING_DIR) \
 	LDCONFIG=true \
 	install-libs
+
 E2FSPROGS_INSTALL_TARGET_OPTS = \
 	DESTDIR=$(TARGET_DIR) \
 	LDCONFIG=true \
@@ -76,90 +94,5 @@ define HOST_E2FSPROGS_INSTALL_CMDS
 	$(HOST_MAKE_ENV) $(MAKE) -C $(@D) install install-libs
 endef
 
-# binaries to keep or remove
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_BADBLOCKS) += usr/sbin/badblocks
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_CHATTR) += usr/bin/chattr
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_DUMPE2FS) += usr/sbin/dumpe2fs
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_E2FREEFRAG) += usr/sbin/e2freefrag
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_E2FSCK) += usr/sbin/e2fsck
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_E2LABEL) += usr/sbin/e2label
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_E2UNDO) += usr/sbin/e2undo
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_E4DEFRAG) += usr/sbin/e4defrag
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_FILEFRAG) += usr/sbin/filefrag
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_FSCK) += usr/sbin/fsck
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_LOGSAVE) += usr/sbin/logsave
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_LSATTR) += usr/bin/lsattr
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_MKE2FS) += usr/sbin/mke2fs
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_MKLOSTFOUND) += usr/sbin/mklost+found
-E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_UUIDGEN) += usr/bin/uuidgen
-
-# files to remove
-E2FSPROGS_TXTTARGETS_ = \
-	usr/sbin/mkfs.ext[234] \
-	usr/sbin/mkfs.ext4dev \
-	usr/sbin/fsck.ext[234] \
-	usr/sbin/fsck.ext4dev \
-	usr/sbin/tune2fs
-
-define E2FSPROGS_TARGET_REMOVE_UNNEEDED
-	rm -f $(addprefix $(TARGET_DIR)/, $(E2FSPROGS_BINTARGETS_))
-	rm -f $(addprefix $(TARGET_DIR)/, $(E2FSPROGS_TXTTARGETS_))
-endef
-
-E2FSPROGS_POST_INSTALL_TARGET_HOOKS += E2FSPROGS_TARGET_REMOVE_UNNEEDED
-
-define E2FSPROGS_TARGET_MKE2FS_SYMLINKS
-	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext2
-	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext3
-	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext4
-	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext4dev
-endef
-
-ifeq ($(BR2_PACKAGE_E2FSPROGS_MKE2FS),y)
-E2FSPROGS_POST_INSTALL_TARGET_HOOKS += E2FSPROGS_TARGET_MKE2FS_SYMLINKS
-endif
-
-define E2FSPROGS_TARGET_E2FSCK_SYMLINKS
-	ln -sf e2fsck $(TARGET_DIR)/usr/sbin/fsck.ext2
-	ln -sf e2fsck $(TARGET_DIR)/usr/sbin/fsck.ext3
-	ln -sf e2fsck $(TARGET_DIR)/usr/sbin/fsck.ext4
-	ln -sf e2fsck $(TARGET_DIR)/usr/sbin/fsck.ext4dev
-endef
-
-ifeq ($(BR2_PACKAGE_E2FSPROGS_E2FSCK),y)
-E2FSPROGS_POST_INSTALL_TARGET_HOOKS += E2FSPROGS_TARGET_E2FSCK_SYMLINKS
-endif
-
-# If BusyBox is included, its configuration may supply its own variant
-# of ext2-related tools. Since Buildroot desires having full blown
-# variants take precedence (in this case, e2fsprogs), we want to remove
-# BusyBox's variant of e2fsprogs provided binaries. e2fsprogs targets
-# /usr/{bin,sbin} where BusyBox targets /{bin,sbin}. We will attempt to
-# remove BusyBox-generated ext2-related tools from /{bin,sbin}. We need
-# to do this in the pre-install stage to ensure we do not accidentally
-# remove e2fsprogs's binaries in usr-merged environments (ie. if they
-# are removed, they would be re-installed in this package's install
-# stage).
-ifeq ($(BR2_PACKAGE_BUSYBOX),y)
-E2FSPROGS_DEPENDENCIES += busybox
-
-define E2FSPROGS_REMOVE_BUSYBOX_APPLETS
-	$(RM) -f $(TARGET_DIR)/bin/chattr
-	$(RM) -f $(TARGET_DIR)/bin/lsattr
-	$(RM) -f $(TARGET_DIR)/sbin/fsck
-	$(RM) -f $(TARGET_DIR)/sbin/tune2fs
-	$(RM) -f $(TARGET_DIR)/sbin/e2label
-endef
-E2FSPROGS_PRE_INSTALL_TARGET_HOOKS += E2FSPROGS_REMOVE_BUSYBOX_APPLETS
-endif
-
-define E2FSPROGS_TARGET_TUNE2FS_SYMLINK
-	ln -sf e2label $(TARGET_DIR)/usr/sbin/tune2fs
-endef
-
-ifeq ($(BR2_PACKAGE_E2FSPROGS_TUNE2FS),y)
-E2FSPROGS_POST_INSTALL_TARGET_HOOKS += E2FSPROGS_TARGET_TUNE2FS_SYMLINK
-endif
-
 $(eval $(autotools-package))
 $(eval $(host-autotools-package))
-- 
2.7.4

[Buildroot] [PATCH v6 2/2] systemd: select fsck wrapper from util-linux

[Carlos Santos]

Select the fsck required by systemd provided by util-linux. This
prevents ending up with fsck from busybox, which is incompatible
with systemd.

Change-Id: I61cfc9b8925169b2678ac911fcd5a4eb5d92c425
Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
[there were no v1, v2]
Changes v3
  - Select e2fsprogs + e2fsck
Changes v3->v4
  - Remove selection of e2fsprogs + e2fsck
  - Clean-up commit message
Changes v4->v5
  - Screwed-up everything, reverting the chances made in v4
Changes v5->v6
  - Restored changes made in v4.
  - Restore the patch changelog.
---
 package/systemd/Config.in | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/package/systemd/Config.in b/package/systemd/Config.in
index 0f54686..478cc10 100644
--- a/package/systemd/Config.in
+++ b/package/systemd/Config.in
@@ -22,6 +22,7 @@ menuconfig BR2_PACKAGE_SYSTEMD
 	select BR2_PACKAGE_UTIL_LINUX_BINARIES
 	select BR2_PACKAGE_UTIL_LINUX_MOUNT
 	select BR2_PACKAGE_UTIL_LINUX_NOLOGIN
+	select BR2_PACKAGE_UTIL_LINUX_FSCK
 	select BR2_PACKAGE_KMOD
 	select BR2_PACKAGE_BUSYBOX_SHOW_OTHERS # kmod-tools
 	select BR2_PACKAGE_KMOD_TOOLS
@@ -61,6 +62,11 @@ menuconfig BR2_PACKAGE_SYSTEMD
 	  - libgcrypt package will add support for journal sealing and
 	    DNSSEC verification in resolved.
 
+	  Notice that systemd selects the fsck wrapper from util-linux
+	  but no particular fsck.<fstype> is selected. You must choose
+	  the apropriate ones (e.g. e2fsck, from the e2fsprogs package)
+	  according to the system configuration.
+
 	  http://freedesktop.org/wiki/Software/systemd
 
 if BR2_PACKAGE_SYSTEMD
-- 
2.7.4

[Buildroot] [PATCH v6 2/2] systemd: select fsck wrapper from util-linux

[Arnout Vandecappelle]

On 10-04-17 13:49, Carlos Santos wrote:
> Select the fsck required by systemd provided by util-linux. This
> prevents ending up with fsck from busybox, which is incompatible
> with systemd.
> 
> Change-Id: I61cfc9b8925169b2678ac911fcd5a4eb5d92c425

 The Change-Id shouldn't be there, since we don't use gerrit.

 Otherwise:

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>


 Regards,
 Arnout

> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> ---
> [there were no v1, v2]
> Changes v3
>   - Select e2fsprogs + e2fsck
> Changes v3->v4
>   - Remove selection of e2fsprogs + e2fsck
>   - Clean-up commit message
> Changes v4->v5
>   - Screwed-up everything, reverting the chances made in v4
> Changes v5->v6
>   - Restored changes made in v4.
>   - Restore the patch changelog.
> ---
>  package/systemd/Config.in | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/package/systemd/Config.in b/package/systemd/Config.in
> index 0f54686..478cc10 100644
> --- a/package/systemd/Config.in
> +++ b/package/systemd/Config.in
> @@ -22,6 +22,7 @@ menuconfig BR2_PACKAGE_SYSTEMD
>  	select BR2_PACKAGE_UTIL_LINUX_BINARIES
>  	select BR2_PACKAGE_UTIL_LINUX_MOUNT
>  	select BR2_PACKAGE_UTIL_LINUX_NOLOGIN
> +	select BR2_PACKAGE_UTIL_LINUX_FSCK
>  	select BR2_PACKAGE_KMOD
>  	select BR2_PACKAGE_BUSYBOX_SHOW_OTHERS # kmod-tools
>  	select BR2_PACKAGE_KMOD_TOOLS
> @@ -61,6 +62,11 @@ menuconfig BR2_PACKAGE_SYSTEMD
>  	  - libgcrypt package will add support for journal sealing and
>  	    DNSSEC verification in resolved.
>  
> +	  Notice that systemd selects the fsck wrapper from util-linux
> +	  but no particular fsck.<fstype> is selected. You must choose
> +	  the apropriate ones (e.g. e2fsck, from the e2fsprogs package)
> +	  according to the system configuration.
> +
>  	  http://freedesktop.org/wiki/Software/systemd
>  
>  if BR2_PACKAGE_SYSTEMD
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v6 1/2] e2fsprogs: refactor to fix conflicts with busybox and util-linux

[Arnout Vandecappelle]

 Hm, I had half-finished a review of v5 when this came in :-) And some of the
typos I was going to say are already fixed, so that's good.


On 10-04-17 13:49, Carlos Santos wrote:
> So far we attempted to solve the conflicts between busybox and e2fsprogs
> by removing busybox programs from /bin and /sbin, leaving the e2fsprogs
> ones at /usr/bin and /usr/sbin. This fails with BR2_ROOTFS_MERGED_USR=y,
> leading to situations like the one described in bug 9436.
> 
> We could provide a better solution by means of a fine-grained selection
> of programs, like util-linux does, but this would require big changes in
> e2fsprogs. So instead of resorting to dirty tricks we switch to a more
> pragmatic approach:
> 
> 1. Drop all configs to select/deselect utilities without corresponding
>    --enable/disable options to the configure script. In other words, we
>    always install the basic set of utilities.
> 2. Do not try to build UUID utilities, since they are disabled along
>    with libuuid (the util-linux ones must be used instead).
> 3. Install e2fsprogs utilities at /bin and /sbin, overriding the ones
>    eventually installed by busybox.
> 
> Notice that these changes do exactly the opposite of what is requested
> in bug 9436. On the other hand the policy for e2fsprogs becomes coherent
> with the one for util-linux: busybox never wins.
> 
> Since e2fsprogs depends on util-linux, it's built later. So in order to
> have e2fsck from e2fsprogs and the fsck wrapper from util-linux (which
> is better maintained and compatible with systemd) we disable e2fsprogs'
> fsck if the one from util-linux is selected.
> 
> Fixes:
>   https://bugs.busybox.net/show_bug.cgi?id=9436 (no fix, in fact)
> 
> Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>

 This has changed so much that I doubt the Sob of Maxime is still correct...
Just turn it into a Cc. Cc makes sure that:

- Maxime is kept in Cc, obviously, but also
- the git log shows that Maxime was involved in this series.

 Regards,
 Arnout


> Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
> ---
> Changes v1->v2
>   - Add depenndece on !BR2_PACKAGE_UTIL_LINUX_FSCK to
>     BR2_PACKAGE_E2FSPROGS_FSCK
>   - Add comment telling that the fsck from util-linux has preference.
>   - Pass "--enable-fsck" conditionally to configure
>   - Remove the guard around the removal of /usr/sbin/fsck, since the
>     fsck from busybox and util-linux are installed at /sbin
>   - Improve comments and commit message
> Changes v2->v3
>   - Pass "--disable-fsck" conditionaly to configure too, since enable
>     is the default.
>   - Do not attempt to remove $(TARGET_DIR)/usr/sbin/fsck, just the
>     $(TARGET_DIR)/sbin/fsck from busybox.
> Changes v3->v4
>    - Improve commit message
> Changes v4->v5
>    - Extreme overhauling, deeply simplifying the recipe and following
>      the suggestion from Arnout Vandecappelle to install e2fsprogs
>      utilities on /bin and /sbin.
> Changes v5->v6
>    - Fix typos and make small changes in commit message
> ---
>  package/e2fsprogs/Config.in      |  84 +++++++---------------
>  package/e2fsprogs/Config.in.host |   1 +
>  package/e2fsprogs/e2fsprogs.mk   | 149 +++++++++++----------------------------
>  3 files changed, 68 insertions(+), 166 deletions(-)
> 
> diff --git a/package/e2fsprogs/Config.in b/package/e2fsprogs/Config.in
> index d1914a9..776645f 100644
> --- a/package/e2fsprogs/Config.in
> +++ b/package/e2fsprogs/Config.in
> @@ -1,101 +1,69 @@
> -config BR2_PACKAGE_E2FSPROGS
> +menuconfig BR2_PACKAGE_E2FSPROGS

  Wow, you're changing way too much here in a single patch... Changing the
config into a menuconfig may be a good idea (we more or less decided that
starting from 5 suboptions it should be a menuconfig), but it certainly should
be a separate patch.

 I propose that you split up into:

- config -> menuconfig

- update upstream URL (with as commit log: "e2fsprogs.sf.net redirects to
e2fsprogs.sourceforge.net, so use that URL directly")

- don't build uuid tools

- remove options for basic set

- add help text for remaining options

- move from /usr/{bin,sbin} to /{bin,sbin}

- whatever you're doing with host-e2fsprogs and for which no explanation is
given in the commit message

 I may have missed something that also needs to be split off, but this patch is
too complicated to detect even that :-)


 I'll give a few other comments below still, but I'm afraid there will be more
in v7...


>  	bool "e2fsprogs"
>  	depends on BR2_USE_MMU # util-linux/libblkid
>  	select BR2_PACKAGE_UTIL_LINUX
>  	select BR2_PACKAGE_UTIL_LINUX_LIBBLKID
> -	select BR2_PACKAGE_UTIL_LINUX_LIBUUID
>  	help
>  	  The EXT2 file system utilities.
>  
> -	  http://e2fsprogs.sf.net
> +	  badblocks chattr debugfs dumpe2fs e2freefrag e2fsck e2image
> +	  e2undo e4crypt e4defrag filefrag fsck fuse2fs logsave lsattr
> +	  mke2fs mklost+found resize2fs tune2fs

 This could be a little more explicit:

	  The following programs are always built and installed:
	  badblocks ...

	  Other programs can be selected below.

>  
> -if BR2_PACKAGE_E2FSPROGS
>  
> -config BR2_PACKAGE_E2FSPROGS_BADBLOCKS
> -	bool "badblocks"
> -	default y
> +	  http://e2fsprogs.sourceforge.net/
>  
> -config BR2_PACKAGE_E2FSPROGS_CHATTR
> -	bool "chattr"
> -	default y
> +if BR2_PACKAGE_E2FSPROGS
> +
> +comment "Basic set: badblocks chattr dumpe2fs e2freefrag e2fsck e2undo"
> +comment "e4crypt e4defrag filefrag logsave lsattr mke2fs mklost+found"
> +comment "tune2fs"

 This comment is not needed. It's already said in the help text.

>  
>  config BR2_PACKAGE_E2FSPROGS_DEBUGFS
>  	bool "debugfs"
> -
> -config BR2_PACKAGE_E2FSPROGS_DUMPE2FS
> -	bool "dumpe2fs"
> -	default y
> -
> -config BR2_PACKAGE_E2FSPROGS_E2FREEFRAG
> -	bool "e2freefrag"
> -	default y
> -
> -config BR2_PACKAGE_E2FSPROGS_E2FSCK
> -	bool "e2fsck"
> -	default y
> +	help
> +	  ext2/ext3/ext4 file system debugger
>  
>  config BR2_PACKAGE_E2FSPROGS_E2IMAGE
>  	bool "e2image"
> -
> -config BR2_PACKAGE_E2FSPROGS_E2LABEL
> -	bool "e2label"
> -	default y
> -
> -config BR2_PACKAGE_E2FSPROGS_E2UNDO
> -	bool "e2undo"
> -	default y
> +	help
> +	  Save critical ext2/ext3/ext4 filesystem metadata to a file
>  
>  config BR2_PACKAGE_E2FSPROGS_E4DEFRAG
>  	bool "e4defrag"
>  	depends on !BR2_nios2 # fallocate not implemented
>  	depends on !BR2_TOOLCHAIN_USES_UCLIBC # sync_file_range not impl
> +	help
> +	  Online defragmenter for ext4 filesystem
>  
>  comment "e4defrag needs a glibc or musl toolchain"
>  	depends on BR2_TOOLCHAIN_USES_UCLIBC

 Hm, looks like there is a
	depends on !BR2_nios2
missing here...

>  
> -config BR2_PACKAGE_E2FSPROGS_FILEFRAG
> -	bool "filefrag"
> -	default y
> -
>  config BR2_PACKAGE_E2FSPROGS_FSCK
>  	bool "fsck"
> +	depends on !BR2_PACKAGE_UTIL_LINUX_FSCK
>  	default y
> +	help
> +	  Check and repair a Linux file system

 This sounds as if fsck does the actual filesystem check, which it doesn't. How
about:

	  Check and repair Linux file systems. This is a wrapper around
	  the filesystem-specific fsck tools.

 Yes, I do realize that makes it differ from the help text in util-linux, which
is not ideal. But when you add something, it's better to do the right thing :-)

> +
> +comment "the fsck from util-linux has preference over this one"
> +	depends on BR2_PACKAGE_UTIL_LINUX_FSCK
>  
>  config BR2_PACKAGE_E2FSPROGS_FUSE2FS
>  	bool "fuse2fs"
>  	depends on !BR2_STATIC_LIBS # libfuse
>  	depends on BR2_TOOLCHAIN_HAS_THREADS # libfuse
>  	select BR2_PACKAGE_LIBFUSE
> +	help
> +	  FUSE file system client for ext2/ext3/ext4 file systems
>  
>  comment "fuse2fs needs a toolchain w/ threads, dynamic library"
>  	depends on !BR2_TOOLCHAIN_HAS_THREADS || BR2_STATIC_LIBS
>  
> -config BR2_PACKAGE_E2FSPROGS_LOGSAVE
> -	bool "logsave"
> -	default y
> -
> -config BR2_PACKAGE_E2FSPROGS_LSATTR
> -	bool "lsattr"
> -	default y
> -
> -config BR2_PACKAGE_E2FSPROGS_MKE2FS
> -	bool "mke2fs"
> -	default y
> -
> -config BR2_PACKAGE_E2FSPROGS_MKLOSTFOUND
> -	bool "mklost+found"
> -	default y
> -
>  config BR2_PACKAGE_E2FSPROGS_RESIZE2FS
>  	bool "resize2fs"
> -
> -config BR2_PACKAGE_E2FSPROGS_TUNE2FS
> -	bool "tune2fs"
> -	default y
> -	select BR2_PACKAGE_E2FSPROGS_E2LABEL
> -
> -config BR2_PACKAGE_E2FSPROGS_UUIDGEN
> -	bool "uuidgen"
>  	default y
> +	help
> +	  ext2/ext3/ext4 file system resizer
>  
>  endif
> diff --git a/package/e2fsprogs/Config.in.host b/package/e2fsprogs/Config.in.host
> index de9148e..7591bbc 100644
> --- a/package/e2fsprogs/Config.in.host
> +++ b/package/e2fsprogs/Config.in.host
> @@ -1,5 +1,6 @@
>  config BR2_PACKAGE_HOST_E2FSPROGS
>  	bool "host e2fsprogs"
> +	select BR2_PACKAGE_HOST_UTIL_LINUX
>  	help
>  	  The EXT2/3/4 file system utilities.
>  
> diff --git a/package/e2fsprogs/e2fsprogs.mk b/package/e2fsprogs/e2fsprogs.mk
> index 3f235c5..e172914 100644
> --- a/package/e2fsprogs/e2fsprogs.mk
> +++ b/package/e2fsprogs/e2fsprogs.mk
> @@ -10,36 +10,58 @@ E2FSPROGS_SITE = $(BR2_KERNEL_MIRROR)/linux/kernel/people/tytso/e2fsprogs/v$(E2F
>  E2FSPROGS_LICENSE = GPL-2.0, BSD-3-Clause (libuuid), MIT-like with advertising clause (libss and libet)
>  E2FSPROGS_LICENSE_FILES = NOTICE lib/uuid/COPYING lib/ss/mit-sipb-copyright.h lib/et/internal.h
>  E2FSPROGS_INSTALL_STAGING = YES
> +
>  E2FSPROGS_DEPENDENCIES = host-pkgconf util-linux
> -# we don't have a host-util-linux
> -HOST_E2FSPROGS_DEPENDENCIES = host-pkgconf
> +
> +HOST_E2FSPROGS_DEPENDENCIES = host-pkgconf host-util-linux

 Actually I'm not sure if we really want to depend on host-util-linux... Why
should we?

> +
> +# If both e2fsprogs and busybox are selected, make certain e2fsprogs
> +# wins the fight over who gets to have their utils actually installed
> +ifeq ($(BR2_PACKAGE_BUSYBOX),y)
> +E2FSPROGS_DEPENDENCIES += busybox
> +endif
> +
> +ifeq ($(BR2_PACKAGE_E2FSPROGS_FUSE2FS),y)
> +E2FSPROGS_DEPENDENCIES += libfuse
> +endif
>  
>  # e4defrag doesn't build on older systems like RHEL5.x, and we don't
>  # need it on the host anyway.
>  # Disable fuse2fs as well to avoid carrying over deps, and it's unused
> -HOST_E2FSPROGS_CONF_OPTS += --disable-defrag --disable-fuse2fs
> +# Set the binary directories to "$(HOST_DIR)/bin" and "$(HOST_DIR)/sbin"
> +# to match the target paths (the $(HOST_DIR) prefix is required because
> +# passing DESTDIR=$(HOST_DIR) in the install command does not work).
> +HOST_E2FSPROGS_CONF_OPTS = \
> +	--disable-defrag \
> +	--disable-fuse2fs \
> +	--bindir=$(HOST_DIR)/bin \
> +	--sbindir=$(HOST_DIR)/sbin \
> +	--enable-symlink-install \
> +	--disable-libblkid \
> +	--disable-libuuid \
> +	--disable-e2initrd-helper \
> +	--disable-testio-debug \
> +	--disable-rpath
>  
> +# Set the binary directories to "/bin" and "/sbin" to override programs
> +# installed by busybox.
>  E2FSPROGS_CONF_OPTS = \
> -	$(if $(BR2_STATIC_LIBS),,--enable-elf-shlibs) \
> -	$(if $(BR2_PACKAGE_E2FSPROGS_DEBUGFS),,--disable-debugfs) \
> -	$(if $(BR2_PACKAGE_E2FSPROGS_E2IMAGE),,--disable-imager) \
> -	$(if $(BR2_PACKAGE_E2FSPROGS_E4DEFRAG),,--disable-defrag) \
> -	$(if $(BR2_PACKAGE_E2FSPROGS_RESIZE2FS),,--disable-resizer) \
> -	--disable-uuidd \
> +	$(if $(BR2_STATIC_LIBS),--disable-elf-shlibs,--enable-elf-shlibs) \
> +	$(if $(BR2_PACKAGE_E2FSPROGS_DEBUGFS),--enable-debugfs,--disable-debugfs) \
> +	$(if $(BR2_PACKAGE_E2FSPROGS_E2IMAGE),--enable-imager,--disable-imager) \
> +	$(if $(BR2_PACKAGE_E2FSPROGS_E4DEFRAG),--enable-defrag,--disable-defrag) \
> +	$(if $(BR2_PACKAGE_E2FSPROGS_FSCK),--enable-fsck,--disable-fsck) \
> +	$(if $(BR2_PACKAGE_E2FSPROGS_FUSE2FS),--enable-fuse2fs,--disable-fuse2fs) \
> +	$(if $(BR2_PACKAGE_E2FSPROGS_RESIZE2FS),--enable-resizer,--disable-resizer) \
> +	--bindir=/bin \
> +	--sbindir=/sbin \
> +	--enable-symlink-install \
>  	--disable-libblkid \
>  	--disable-libuuid \
> -	--enable-fsck \
>  	--disable-e2initrd-helper \
>  	--disable-testio-debug \
>  	--disable-rpath
>  
> -ifeq ($(BR2_PACKAGE_E2FSPROGS_FUSE2FS),y)
> -E2FSPROGS_CONF_OPTS += --enable-fuse2fs
> -E2FSPROGS_DEPENDENCIES += libfuse
> -else
> -E2FSPROGS_CONF_OPTS += --disable-fuse2fs
> -endif

 Don't change this, it was better like that.

> -
>  ifeq ($(BR2_nios2),y)
>  E2FSPROGS_CONF_ENV += ac_cv_func_fallocate=no
>  endif
> @@ -56,17 +78,13 @@ HOST_E2FSPROGS_CONF_ENV += \
>  	ac_cv_header_magic_h=no \
>  	ac_cv_lib_magic_magic_file=no
>  
> -ifeq ($(BR2_NEEDS_GETTEXT_IF_LOCALE),y)
> -# util-linux libuuid pulls in libintl if needed, so ensure we also
> -# link against it, otherwise static linking fails
> -E2FSPROGS_CONF_ENV += LIBS=-lintl
> -endif

 Why can this be removed? IIUC, e2fsprogs will now use the uuid from util-linux
if that is enabled, and will not use uuid at all if not. Correct? So if
util-linux libuuid is enabled, this is still needed, no?

> -
>  E2FSPROGS_MAKE_OPTS = LDCONFIG=true
> +
>  E2FSPROGS_INSTALL_STAGING_OPTS = \
>  	DESTDIR=$(STAGING_DIR) \
>  	LDCONFIG=true \
>  	install-libs
> +
>  E2FSPROGS_INSTALL_TARGET_OPTS = \
>  	DESTDIR=$(TARGET_DIR) \
>  	LDCONFIG=true \
> @@ -76,90 +94,5 @@ define HOST_E2FSPROGS_INSTALL_CMDS
>  	$(HOST_MAKE_ENV) $(MAKE) -C $(@D) install install-libs
>  endef
>  
> -# binaries to keep or remove
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_BADBLOCKS) += usr/sbin/badblocks
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_CHATTR) += usr/bin/chattr
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_DUMPE2FS) += usr/sbin/dumpe2fs
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_E2FREEFRAG) += usr/sbin/e2freefrag
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_E2FSCK) += usr/sbin/e2fsck
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_E2LABEL) += usr/sbin/e2label
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_E2UNDO) += usr/sbin/e2undo
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_E4DEFRAG) += usr/sbin/e4defrag
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_FILEFRAG) += usr/sbin/filefrag
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_FSCK) += usr/sbin/fsck
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_LOGSAVE) += usr/sbin/logsave
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_LSATTR) += usr/bin/lsattr
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_MKE2FS) += usr/sbin/mke2fs
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_MKLOSTFOUND) += usr/sbin/mklost+found
> -E2FSPROGS_BINTARGETS_$(BR2_PACKAGE_E2FSPROGS_UUIDGEN) += usr/bin/uuidgen
> -
> -# files to remove
> -E2FSPROGS_TXTTARGETS_ = \
> -	usr/sbin/mkfs.ext[234] \
> -	usr/sbin/mkfs.ext4dev \
> -	usr/sbin/fsck.ext[234] \
> -	usr/sbin/fsck.ext4dev \
> -	usr/sbin/tune2fs
> -
> -define E2FSPROGS_TARGET_REMOVE_UNNEEDED
> -	rm -f $(addprefix $(TARGET_DIR)/, $(E2FSPROGS_BINTARGETS_))
> -	rm -f $(addprefix $(TARGET_DIR)/, $(E2FSPROGS_TXTTARGETS_))
> -endef
> -
> -E2FSPROGS_POST_INSTALL_TARGET_HOOKS += E2FSPROGS_TARGET_REMOVE_UNNEEDED
> -
> -define E2FSPROGS_TARGET_MKE2FS_SYMLINKS
> -	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext2
> -	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext3
> -	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext4
> -	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext4dev

 Can you explain why this is no longer needed? I don't see how any of your other
changes could obviate this one...

 If it actually wasn't needed to begin with, please also make it a separate
patch with a good explanation.


 Regards,
 Arnout

> -endef
> -
> -ifeq ($(BR2_PACKAGE_E2FSPROGS_MKE2FS),y)
> -E2FSPROGS_POST_INSTALL_TARGET_HOOKS += E2FSPROGS_TARGET_MKE2FS_SYMLINKS
> -endif
> -
> -define E2FSPROGS_TARGET_E2FSCK_SYMLINKS
> -	ln -sf e2fsck $(TARGET_DIR)/usr/sbin/fsck.ext2
> -	ln -sf e2fsck $(TARGET_DIR)/usr/sbin/fsck.ext3
> -	ln -sf e2fsck $(TARGET_DIR)/usr/sbin/fsck.ext4
> -	ln -sf e2fsck $(TARGET_DIR)/usr/sbin/fsck.ext4dev
> -endef
> -
> -ifeq ($(BR2_PACKAGE_E2FSPROGS_E2FSCK),y)
> -E2FSPROGS_POST_INSTALL_TARGET_HOOKS += E2FSPROGS_TARGET_E2FSCK_SYMLINKS
> -endif
> -
> -# If BusyBox is included, its configuration may supply its own variant
> -# of ext2-related tools. Since Buildroot desires having full blown
> -# variants take precedence (in this case, e2fsprogs), we want to remove
> -# BusyBox's variant of e2fsprogs provided binaries. e2fsprogs targets
> -# /usr/{bin,sbin} where BusyBox targets /{bin,sbin}. We will attempt to
> -# remove BusyBox-generated ext2-related tools from /{bin,sbin}. We need
> -# to do this in the pre-install stage to ensure we do not accidentally
> -# remove e2fsprogs's binaries in usr-merged environments (ie. if they
> -# are removed, they would be re-installed in this package's install
> -# stage).
> -ifeq ($(BR2_PACKAGE_BUSYBOX),y)
> -E2FSPROGS_DEPENDENCIES += busybox
> -
> -define E2FSPROGS_REMOVE_BUSYBOX_APPLETS
> -	$(RM) -f $(TARGET_DIR)/bin/chattr
> -	$(RM) -f $(TARGET_DIR)/bin/lsattr
> -	$(RM) -f $(TARGET_DIR)/sbin/fsck
> -	$(RM) -f $(TARGET_DIR)/sbin/tune2fs
> -	$(RM) -f $(TARGET_DIR)/sbin/e2label
> -endef
> -E2FSPROGS_PRE_INSTALL_TARGET_HOOKS += E2FSPROGS_REMOVE_BUSYBOX_APPLETS
> -endif
> -
> -define E2FSPROGS_TARGET_TUNE2FS_SYMLINK
> -	ln -sf e2label $(TARGET_DIR)/usr/sbin/tune2fs
> -endef
> -
> -ifeq ($(BR2_PACKAGE_E2FSPROGS_TUNE2FS),y)
> -E2FSPROGS_POST_INSTALL_TARGET_HOOKS += E2FSPROGS_TARGET_TUNE2FS_SYMLINK
> -endif
> -
>  $(eval $(autotools-package))
>  $(eval $(host-autotools-package))
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v6 1/2] e2fsprogs: refactor to fix conflicts with busybox and util-linux

[Carlos Santos]

> From: "Arnout Vandecappelle" <arnout@mind.be>
> To: "Carlos Santos" <casantos@datacom.ind.br>, buildroot at buildroot.org
> Cc: "Maxime Hadjinlian" <maxime.hadjinlian@gmail.com>, "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>, "Yann E
> . MORIN" <yann.morin.1998@free.fr>
> Sent: Monday, April 10, 2017 9:45:05 AM
> Subject: Re: [PATCH v6 1/2] e2fsprogs: refactor to fix conflicts with busybox and util-linux

---8<---

>> Signed-off-by: Maxime Hadjinlian <maxime.hadjinlian@gmail.com>
> 
> This has changed so much that I doubt the Sob of Maxime is still correct...
> Just turn it into a Cc. Cc makes sure that:
> 
> - Maxime is kept in Cc, obviously, but also
> - the git log shows that Maxime was involved in this series.
---8<---
>> Changes v4->v5
>>    - Extreme overhauling, deeply simplifying the recipe and following
>>      the suggestion from Arnout Vandecappelle to install e2fsprogs
>>      utilities on /bin and /sbin.
>> Changes v5->v6
>>    - Fix typos and make small changes in commit message
---8<---
>  Wow, you're changing way too much here in a single patch... Changing the
> config into a menuconfig may be a good idea (we more or less decided that
> starting from 5 suboptions it should be a menuconfig), but it certainly should
> be a separate patch.
> 
> I propose that you split up into:
> 
> - config -> menuconfig
> 
> - update upstream URL (with as commit log: "e2fsprogs.sf.net redirects to
> e2fsprogs.sourceforge.net, so use that URL directly")
> 
> - don't build uuid tools
> 
> - remove options for basic set
> 
> - add help text for remaining options
> 
> - move from /usr/{bin,sbin} to /{bin,sbin}
> 
> - whatever you're doing with host-e2fsprogs and for which no explanation is
> given in the commit message
> 
> I may have missed something that also needs to be split off, but this patch is
> too complicated to detect even that :-)

Sorry, no.

I made a deep overhauling because the original recipe was so messy that
small changes did not work. It is now reasonable to artificially split the
change in seven patches now to end up with exactly the same result. It's
harder to implement, test and review, considering all possible options I
need to deal with:

- glibc, musl or uClibc
- shared-only, static-only or shared/static libraries
- split or merged /bin|sbin and /usr/bin|sbin (triggered by systemd).

---8<---
>>  
>> -	  http://e2fsprogs.sf.net
>> +	  badblocks chattr debugfs dumpe2fs e2freefrag e2fsck e2image
>> +	  e2undo e4crypt e4defrag filefrag fsck fuse2fs logsave lsattr
>> +	  mke2fs mklost+found resize2fs tune2fs
> 
> This could be a little more explicit:
> 
>	  The following programs are always built and installed:
>	  badblocks ...
> 
>	  Other programs can be selected below.

Ok

---8<---
>> +comment "Basic set: badblocks chattr dumpe2fs e2freefrag e2fsck e2undo"
>> +comment "e4crypt e4defrag filefrag logsave lsattr mke2fs mklost+found"
>> +comment "tune2fs"
> 
> This comment is not needed. It's already said in the help text.

Ok

---8<---
>>  config BR2_PACKAGE_E2FSPROGS_E4DEFRAG
>>  	bool "e4defrag"
>>  	depends on !BR2_nios2 # fallocate not implemented
>>  	depends on !BR2_TOOLCHAIN_USES_UCLIBC # sync_file_range not impl
>> +	help
>> +	  Online defragmenter for ext4 filesystem
>>  
>>  comment "e4defrag needs a glibc or musl toolchain"
>>  	depends on BR2_TOOLCHAIN_USES_UCLIBC
> 
> Hm, looks like there is a
>	depends on !BR2_nios2
> missing here...

Ok

>>  
>> -config BR2_PACKAGE_E2FSPROGS_FILEFRAG
>> -	bool "filefrag"
>> -	default y
>> -
>>  config BR2_PACKAGE_E2FSPROGS_FSCK
>>  	bool "fsck"
>> +	depends on !BR2_PACKAGE_UTIL_LINUX_FSCK
>>  	default y
>> +	help
>> +	  Check and repair a Linux file system
> 
> This sounds as if fsck does the actual filesystem check, which it doesn't. How
> about:
> 
>	  Check and repair Linux file systems. This is a wrapper around
>	  the filesystem-specific fsck tools.
> 
> Yes, I do realize that makes it differ from the help text in util-linux, which
> is not ideal. But when you add something, it's better to do the right thing :-)

Ok

---8<---
>> -# we don't have a host-util-linux
>> -HOST_E2FSPROGS_DEPENDENCIES = host-pkgconf
>> +
>> +HOST_E2FSPROGS_DEPENDENCIES = host-pkgconf host-util-linux
> 
> Actually I'm not sure if we really want to depend on host-util-linux... Why
> should we?

Because host-e2fsprogs now ues libblkid and libuuid from util-linux.
This prevents overriding them with e2fsprogs' ones, which may cause
problems for other packages.
---
>> +HOST_E2FSPROGS_CONF_OPTS = \
>> +	--disable-defrag \
>> +	--disable-fuse2fs \
>> +	--bindir=$(HOST_DIR)/bin \
>> +	--sbindir=$(HOST_DIR)/sbin \
>> +	--enable-symlink-install \
>> +	--disable-libblkid \
>> +	--disable-libuuid \
        ^^^^^^^^^^^^^^^^^^^^
              Here
>> +	--disable-e2initrd-helper \
>> +	--disable-testio-debug \
>> +	--disable-rpath
>>  
>> -ifeq ($(BR2_PACKAGE_E2FSPROGS_FUSE2FS),y)
>> -E2FSPROGS_CONF_OPTS += --enable-fuse2fs
>> -E2FSPROGS_DEPENDENCIES += libfuse
>> -else
>> -E2FSPROGS_CONF_OPTS += --disable-fuse2fs
>> -endif
> 
> Don't change this, it was better like that.

No, it was not. My change keeps all --enable/disable- together,
improving readability.

---8<---
>> -ifeq ($(BR2_NEEDS_GETTEXT_IF_LOCALE),y)
>> -# util-linux libuuid pulls in libintl if needed, so ensure we also
>> -# link against it, otherwise static linking fails
>> -E2FSPROGS_CONF_ENV += LIBS=-lintl
>> -endif
> 
> Why can this be removed? IIUC, e2fsprogs will now use the uuid from util-linux
> if that is enabled, and will not use uuid at all if not. Correct? So if
> util-linux libuuid is enabled, this is still needed, no?

Hum, adding a uClibc build to the test set. In fact the test can be

    ifeq ($(BR2_NEEDS_GETTEXT_IF_LOCALE)$(BR2_STATIC_LIBS),yy)

>> -define E2FSPROGS_TARGET_MKE2FS_SYMLINKS
>> -	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext2
>> -	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext3
>> -	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext4
>> -	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext4dev
> 
> Can you explain why this is no longer needed? I don't see how any of your other
> changes could obviate this one...
> 
> If it actually wasn't needed to begin with, please also make it a separate
> patch with a good explanation.

Perhaps it was needed by old versions of e2fsprogs but with v1.43.4 the
symlinks are created (see the --enable-symlink-install in *_CONF_OPTS).

Also, fsck.ext4dev and mkfs.ext4dev were dropped in v1.43.4, according
to the release notes.

-- 
Carlos Santos (Casantos) - DATACOM, P&D
?The greatest triumph that modern PR can offer is the transcendent 
success of having your words and actions judged by your reputation, 
rather than the other way about.? ? Christopher Hitchens

[Buildroot] [PATCH v6 1/2] e2fsprogs: refactor to fix conflicts with busybox and util-linux

[Arnout Vandecappelle]

On 11-04-17 18:59, Carlos Santos wrote:
>> From: "Arnout Vandecappelle" <arnout@mind.be>
>> To: "Carlos Santos" <casantos@datacom.ind.br>, buildroot at buildroot.org
>> Cc: "Maxime Hadjinlian" <maxime.hadjinlian@gmail.com>, "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>, "Yann E
>> . MORIN" <yann.morin.1998@free.fr>
>> Sent: Monday, April 10, 2017 9:45:05 AM
>> Subject: Re: [PATCH v6 1/2] e2fsprogs: refactor to fix conflicts with busybox and util-linux
[snip]
>>  Wow, you're changing way too much here in a single patch... Changing the
>> config into a menuconfig may be a good idea (we more or less decided that
>> starting from 5 suboptions it should be a menuconfig), but it certainly should
>> be a separate patch.
>>
>> I propose that you split up into:
>>
>> - config -> menuconfig
>>
>> - update upstream URL (with as commit log: "e2fsprogs.sf.net redirects to
>> e2fsprogs.sourceforge.net, so use that URL directly")
>>
>> - don't build uuid tools
>>
>> - remove options for basic set
>>
>> - add help text for remaining options
>>
>> - move from /usr/{bin,sbin} to /{bin,sbin}
>>
>> - whatever you're doing with host-e2fsprogs and for which no explanation is
>> given in the commit message
>>
>> I may have missed something that also needs to be split off, but this patch is
>> too complicated to detect even that :-)
> 
> Sorry, no.
> 
> I made a deep overhauling because the original recipe was so messy that
> small changes did not work. It is now reasonable to artificially split the
> change in seven patches now to end up with exactly the same result. It's
> harder to implement, test and review, considering all possible options I
> need to deal with:

 I agree that it's a little bit harder to implement, but not to test and review.
And it's pretty likely that you will not get the same result, because proper
review will be done, problems will be discovered, and you end up with a
different result :-)

 If you don't split this up, don't expect the patch to be applied for the next
year or so. The review burden is simply too high.

 In addition, most of the splits I mentioned hardly take any effort, and they
result in very simple patches that can be applied quickly. I can imagine that
you'll still have a quite big patch after that that does all the complicated
stuff, but that complicated patch will be half the size of this one.


> - glibc, musl or uClibc

 Doing a 'test-pkg -p e2fsprogs -r 10' takes about 5 minutes. Just about the
time it takes to write the commit message :-)

> - shared-only, static-only or shared/static libraries

 test-pkg covers this.

> - split or merged /bin|sbin and /usr/bin|sbin (triggered by systemd).

 This will actually be simplified by splitting up, because only the "move from
/usr/{bin,sbin} to /{bin,sbin}" part is affected by it.

[snip]
>>>  config BR2_PACKAGE_E2FSPROGS_E4DEFRAG
>>>  	bool "e4defrag"
>>>  	depends on !BR2_nios2 # fallocate not implemented
>>>  	depends on !BR2_TOOLCHAIN_USES_UCLIBC # sync_file_range not impl
>>> +	help
>>> +	  Online defragmenter for ext4 filesystem
>>>  
>>>  comment "e4defrag needs a glibc or musl toolchain"
>>>  	depends on BR2_TOOLCHAIN_USES_UCLIBC
>>
>> Hm, looks like there is a
>> 	depends on !BR2_nios2
>> missing here...
> 
> Ok

 I should have been explicit, probably: that bit is completely unrelated, so it
should be a separate patch!

[snip]
>>> -# we don't have a host-util-linux
>>> -HOST_E2FSPROGS_DEPENDENCIES = host-pkgconf
>>> +
>>> +HOST_E2FSPROGS_DEPENDENCIES = host-pkgconf host-util-linux
>>
>> Actually I'm not sure if we really want to depend on host-util-linux... Why
>> should we?
> 
> Because host-e2fsprogs now ues libblkid and libuuid from util-linux.

 But you removed the select BR2_PACKAGE_UTIL_LINUX_LIBUUID... So for target, we
sometimes do use the libuuid from e2fsprogs? /me is confused...

> This prevents overriding them with e2fsprogs' ones, which may cause
> problems for other packages.
> ---
>>> +HOST_E2FSPROGS_CONF_OPTS = \
>>> +	--disable-defrag \
>>> +	--disable-fuse2fs \
>>> +	--bindir=$(HOST_DIR)/bin \
>>> +	--sbindir=$(HOST_DIR)/sbin \
>>> +	--enable-symlink-install \
>>> +	--disable-libblkid \
>>> +	--disable-libuuid \
>         ^^^^^^^^^^^^^^^^^^^^
>               Here
>>> +	--disable-e2initrd-helper \
>>> +	--disable-testio-debug \
>>> +	--disable-rpath
>>>  
>>> -ifeq ($(BR2_PACKAGE_E2FSPROGS_FUSE2FS),y)
>>> -E2FSPROGS_CONF_OPTS += --enable-fuse2fs
>>> -E2FSPROGS_DEPENDENCIES += libfuse
>>> -else
>>> -E2FSPROGS_CONF_OPTS += --disable-fuse2fs
>>> -endif
>>
>> Don't change this, it was better like that.
> 
> No, it was not. My change keeps all --enable/disable- together,
> improving readability.

 Begging to differ... Splitting up the DEPENDENCIES from the CONF_OPTS is less
readable.

 I actually don't consider the $(if $(...),--enable,--disable) construct very
readable. But it's historically like that so no need to change it.

> ---8<---
>>> -ifeq ($(BR2_NEEDS_GETTEXT_IF_LOCALE),y)
>>> -# util-linux libuuid pulls in libintl if needed, so ensure we also
>>> -# link against it, otherwise static linking fails
>>> -E2FSPROGS_CONF_ENV += LIBS=-lintl
>>> -endif
>>
>> Why can this be removed? IIUC, e2fsprogs will now use the uuid from util-linux
>> if that is enabled, and will not use uuid at all if not. Correct? So if
>> util-linux libuuid is enabled, this is still needed, no?
> 
> Hum, adding a uClibc build to the test set. In fact the test can be
> 
>     ifeq ($(BR2_NEEDS_GETTEXT_IF_LOCALE)$(BR2_STATIC_LIBS),yy)

 Yes, but not really needed to change it. If you do, it should be a separate
patch :-).

> 
>>> -define E2FSPROGS_TARGET_MKE2FS_SYMLINKS
>>> -	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext2
>>> -	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext3
>>> -	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext4
>>> -	ln -sf mke2fs $(TARGET_DIR)/usr/sbin/mkfs.ext4dev
>>
>> Can you explain why this is no longer needed? I don't see how any of your other
>> changes could obviate this one...
>>
>> If it actually wasn't needed to begin with, please also make it a separate
>> patch with a good explanation.
> 
> Perhaps it was needed by old versions of e2fsprogs but with v1.43.4 the
> symlinks are created (see the --enable-symlink-install in *_CONF_OPTS).
> 
> Also, fsck.ext4dev and mkfs.ext4dev were dropped in v1.43.4, according
> to the release notes.

 OK. Include this explanation in the commit log of the patch removing the symlinks.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH v6 1/2] e2fsprogs: refactor to fix conflicts with busybox and util-linux

[Carlos Santos]

> From: "Arnout Vandecappelle" <arnout@mind.be>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: buildroot at buildroot.org, "Maxime Hadjinlian" <maxime.hadjinlian@gmail.com>, "Thomas Petazzoni"
> <thomas.petazzoni@free-electrons.com>, "Yann E . MORIN" <yann.morin.1998@free.fr>
> Sent: Tuesday, April 11, 2017 7:14:27 PM
> Subject: Re: [PATCH v6 1/2] e2fsprogs: refactor to fix conflicts with busybox and util-linux

> On 11-04-17 18:59, Carlos Santos wrote:
---8<---
>> I made a deep overhauling because the original recipe was so messy that
>> small changes did not work. It is now reasonable to artificially split the
>> change in seven patches now to end up with exactly the same result. It's
>> harder to implement, test and review, considering all possible options I
>> need to deal with:
> 
> I agree that it's a little bit harder to implement, but not to test and review.
> And it's pretty likely that you will not get the same result, because proper
> review will be done, problems will be discovered, and you end up with a
> different result :-)
> 
> If you don't split this up, don't expect the patch to be applied for the next
> year or so. The review burden is simply too high.

I see. That's really sad.

-- 
Carlos Santos (Casantos) - DATACOM, P&D
?The greatest triumph that modern PR can offer is the transcendent 
success of having your words and actions judged by your reputation, 
rather than the other way about.? ? Christopher Hitchens

[Buildroot] [PATCH v6 1/2] e2fsprogs: refactor to fix conflicts with busybox and util-linux

[Arnout Vandecappelle]

On 12-04-17 01:09, Carlos Santos wrote:
>> From: "Arnout Vandecappelle" <arnout@mind.be>
>> To: "Carlos Santos" <casantos@datacom.ind.br>
>> Cc: buildroot at buildroot.org, "Maxime Hadjinlian" <maxime.hadjinlian@gmail.com>, "Thomas Petazzoni"
>> <thomas.petazzoni@free-electrons.com>, "Yann E . MORIN" <yann.morin.1998@free.fr>
>> Sent: Tuesday, April 11, 2017 7:14:27 PM
>> Subject: Re: [PATCH v6 1/2] e2fsprogs: refactor to fix conflicts with busybox and util-linux
> 
>> On 11-04-17 18:59, Carlos Santos wrote:
> ---8<---
>>> I made a deep overhauling because the original recipe was so messy that
>>> small changes did not work. It is now reasonable to artificially split the
>>> change in seven patches now to end up with exactly the same result. It's
>>> harder to implement, test and review, considering all possible options I
>>> need to deal with:
>>
>> I agree that it's a little bit harder to implement, but not to test and review.
>> And it's pretty likely that you will not get the same result, because proper
>> review will be done, problems will be discovered, and you end up with a
>> different result :-)
>>
>> If you don't split this up, don't expect the patch to be applied for the next
>> year or so. The review burden is simply too high.
> 
> I see. That's really sad.

 I agree. We simply don't have enough people doing review/maintenance, so
complicated stuff like this tends to get dropped.

 But if you don't have the time to split up the patch, I fully understand. Just
send a v7 that changes what you still can, and mention below the --- that you're
not going to split it up. Possibly someone else will take it over, or someone
*will* spend the effort to review it in detail.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF