[Buildroot] [PATCH v3] package/coreutils: allow building individual binaries

[Buildroot] [PATCH v3] package/coreutils: allow building individual binaries

[Markus Mayer]

We add configuration option BR2_PACKAGE_COREUTILS_INDIVIDUAL_BINARIES
so that coreutils can be built and installed as individual binaries.
It can be used if the target file system doesn't support symlinks or
symlinks are undesirable.

The approach is modelled after Busybox's similarly named configuration
option.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
---
 package/coreutils/Config.in    | 18 ++++++++++++++
 package/coreutils/coreutils.mk | 45 +++++++++++++++++++++++++++++-----
 2 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/package/coreutils/Config.in b/package/coreutils/Config.in
index 11a6019e820f..a0a84c220a48 100644
--- a/package/coreutils/Config.in
+++ b/package/coreutils/Config.in
@@ -19,3 +19,21 @@ comment "coreutils needs a toolchain w/ wchar"
 	depends on BR2_USE_MMU
 	depends on !BR2_USE_WCHAR
 	depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS
+
+if BR2_PACKAGE_COREUTILS
+
+config BR2_PACKAGE_COREUTILS_INDIVIDUAL_BINARIES
+	bool "Individual binaries"
+	depends on !BR2_STATIC_LIBS
+	help
+	  By default (i.e. with this option disabled), coreutils is
+	  installed as a single binary (Busybox style) called
+	  /usr/bin/coreutils, and all core utilities are symbolic
+	  links to this binary.
+
+	  With this option enabled, each utility is a separate binary.
+
+comment "coreutils individual binaries need a toolchain w/ dynamic library"
+	depends on BR2_STATIC_LIBS
+
+endif
diff --git a/package/coreutils/coreutils.mk b/package/coreutils/coreutils.mk
index 8e5518f6383e..05232163a747 100644
--- a/package/coreutils/coreutils.mk
+++ b/package/coreutils/coreutils.mk
@@ -14,8 +14,13 @@ COREUTILS_LICENSE_FILES = COPYING
 COREUTILS_AUTORECONF = YES
 COREUTILS_GETTEXTIZE = YES
 
-COREUTILS_CONF_OPTS = --disable-rpath --enable-single-binary=symlinks \
+COREUTILS_CONF_OPTS = --disable-rpath \
 	$(if $(BR2_TOOLCHAIN_USES_MUSL),--with-included-regex)
+
+ifeq ($(BR2_PACKAGE_COREUTILS_INDIVIDUAL_BINARIES),)
+COREUTILS_CONF_OPTS += --enable-single-binary=symlinks
+endif
+
 COREUTILS_CONF_ENV = ac_cv_c_restrict=no \
 	ac_cv_func_chown_works=yes \
 	ac_cv_func_euidaccess=no \
@@ -59,6 +64,13 @@ COREUTILS_BIN_PROGS = base64 cat chgrp chmod chown cp date dd df dir echo false
 	kill link ln ls mkdir mknod mktemp mv nice printenv pwd rm rmdir \
 	vdir sleep stty sync touch true uname join
 
+# This is where our '[' symlink will point to.
+ifeq ($(BR2_PACKAGE_COREUTILS_INDIVIDUAL_BINARIES),)
+TEST_BINARY = coreutils
+else
+TEST_BINARY = test
+endif
+
 ifeq ($(BR2_PACKAGE_ACL),y)
 COREUTILS_DEPENDENCIES += acl
 else
@@ -96,13 +108,24 @@ COREUTILS_DEPENDENCIES += openssl
 endif
 
 ifeq ($(BR2_ROOTFS_MERGED_USR),)
+# We want to move a few binaries from /usr/bin to /bin. In the case of
+# coreutils being built as multi-call binary, we do so by re-creating the
+# corresponding symlinks. If coreutils is built with individual binaries, we
+# actually move the binaries.
+ifeq ($(BR2_PACKAGE_COREUTILS_INDIVIDUAL_BINARIES),)
 define COREUTILS_CLEANUP_BIN
-	# some things go in /bin rather than /usr/bin
 	$(foreach f,$(COREUTILS_BIN_PROGS), \
 		rm -f $(TARGET_DIR)/usr/bin/$(f) && \
 		ln -sf ../usr/bin/coreutils $(TARGET_DIR)/bin/$(f)
 	)
 endef
+else
+define COREUTILS_CLEANUP_BIN
+	$(foreach f,$(COREUTILS_BIN_PROGS), \
+                mv $(TARGET_DIR)/usr/bin/$(f) $(TARGET_DIR)/bin
+        )
+endef
+endif
 COREUTILS_POST_INSTALL_TARGET_HOOKS += COREUTILS_CLEANUP_BIN
 endif
 
@@ -110,14 +133,24 @@ ifeq ($(BR2_STATIC_LIBS),y)
 COREUTILS_CONF_OPTS += --enable-no-install-program=stdbuf
 endif
 
-define COREUTILS_CLEANUP
+define COREUTILS_TEST_SYMLINK
 	# link for archaic shells
-	ln -fs coreutils $(TARGET_DIR)/usr/bin/[
-	# gnu thinks chroot is in bin, debian thinks it's in sbin
+	ln -fs $(TEST_BINARY) $(TARGET_DIR)/usr/bin/[
+endef
+COREUTILS_POST_INSTALL_TARGET_HOOKS += COREUTILS_TEST_SYMLINK
+
+# GNU thinks chroot is in bin, debian thinks it's in sbin. We move chroot to
+# /usr/sbin, either by re-creating a symlink or by actually moving it.
+ifeq ($(BR2_PACKAGE_COREUTILS_INDIVIDUAL_BINARIES),)
+define COREUTILS_CLEANUP
 	rm -f $(TARGET_DIR)/usr/bin/chroot
 	ln -sf ../bin/coreutils $(TARGET_DIR)/usr/sbin/chroot
 endef
-
+else
+define COREUTILS_CLEANUP
+        mv $(TARGET_DIR)/usr/bin/chroot $(TARGET_DIR)/usr/sbin
+endef
+endif
 COREUTILS_POST_INSTALL_TARGET_HOOKS += COREUTILS_CLEANUP
 
 $(eval $(autotools-package))
-- 
2.17.1

[Buildroot] [PATCH v3] package/coreutils: allow building individual binaries

[Thomas Petazzoni]

Hello Markus,

On Thu,  6 Jun 2019 19:15:34 -0700
Markus Mayer <mmayer@broadcom.com> wrote:

> We add configuration option BR2_PACKAGE_COREUTILS_INDIVIDUAL_BINARIES
> so that coreutils can be built and installed as individual binaries.
> It can be used if the target file system doesn't support symlinks or
> symlinks are undesirable.
> 
> The approach is modelled after Busybox's similarly named configuration
> option.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>

I have applied your patch, but after some changes.

First I did some preparation commits:

  https://git.buildroot.org/buildroot/commit/?id=14644a66b60a4115f20f101f08966414852e446c
  https://git.buildroot.org/buildroot/commit/?id=840cda91152dd8f702de8e77af34cddfbe4c98c8

And then some changes (see below).

> diff --git a/package/coreutils/Config.in b/package/coreutils/Config.in
> index 11a6019e820f..a0a84c220a48 100644
> --- a/package/coreutils/Config.in
> +++ b/package/coreutils/Config.in
> @@ -19,3 +19,21 @@ comment "coreutils needs a toolchain w/ wchar"
>  	depends on BR2_USE_MMU
>  	depends on !BR2_USE_WCHAR
>  	depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS

This comment should have been at the end of the Config.in file. Indeed
having the comment between the main option and the sub-options prevent
the sub-options from being indented "under" the main option in
"menuconfig".

> -COREUTILS_CONF_OPTS = --disable-rpath --enable-single-binary=symlinks \
> +COREUTILS_CONF_OPTS = --disable-rpath \
>  	$(if $(BR2_TOOLCHAIN_USES_MUSL),--with-included-regex)
> +
> +ifeq ($(BR2_PACKAGE_COREUTILS_INDIVIDUAL_BINARIES),)
> +COREUTILS_CONF_OPTS += --enable-single-binary=symlinks
> +endif

I changed this to:

+ifeq ($(BR2_PACKAGE_COREUTILS_INDIVIDUAL_BINARIES),y)
+COREUTILS_CONF_OPTS += --disable-single-binary
+else
+COREUTILS_CONF_OPTS += --enable-single-binary=symlinks
+endif

To be fully explicit.

>  COREUTILS_CONF_ENV = ac_cv_c_restrict=no \
>  	ac_cv_func_chown_works=yes \
>  	ac_cv_func_euidaccess=no \
> @@ -59,6 +64,13 @@ COREUTILS_BIN_PROGS = base64 cat chgrp chmod chown cp date dd df dir echo false
>  	kill link ln ls mkdir mknod mktemp mv nice printenv pwd rm rmdir \
>  	vdir sleep stty sync touch true uname join
>  
> +# This is where our '[' symlink will point to.
> +ifeq ($(BR2_PACKAGE_COREUTILS_INDIVIDUAL_BINARIES),)
> +TEST_BINARY = coreutils
> +else
> +TEST_BINARY = test
> +endif

The namespace of variables in Buildroot is global, so using the name
TEST_BINARY is a *very* bad idea. It should have been
COREUTILS_TEST_BINARY. But in fact, I did this differently, and simply
added two definitions of COREUTILS_CREATE_TEST_SYMLINK.

>  ifeq ($(BR2_PACKAGE_ACL),y)
>  COREUTILS_DEPENDENCIES += acl
>  else
> @@ -96,13 +108,24 @@ COREUTILS_DEPENDENCIES += openssl
>  endif
>  
>  ifeq ($(BR2_ROOTFS_MERGED_USR),)
> +# We want to move a few binaries from /usr/bin to /bin. In the case
> of +# coreutils being built as multi-call binary, we do so by
> re-creating the +# corresponding symlinks. If coreutils is built with
> individual binaries, we +# actually move the binaries.
> +ifeq ($(BR2_PACKAGE_COREUTILS_INDIVIDUAL_BINARIES),)

I swapped the two cases to have the INDIVIDUAL_BINARIES=y case first
(so that all conditions use INDIVIDUAL_BINARIES=y).

>  define COREUTILS_CLEANUP_BIN
> -	# some things go in /bin rather than /usr/bin
>  	$(foreach f,$(COREUTILS_BIN_PROGS), \
>  		rm -f $(TARGET_DIR)/usr/bin/$(f) && \
>  		ln -sf ../usr/bin/coreutils $(TARGET_DIR)/bin/$(f)
>  	)
>  endef
> +else
> +define COREUTILS_CLEANUP_BIN
> +	$(foreach f,$(COREUTILS_BIN_PROGS), \
> +                mv $(TARGET_DIR)/usr/bin/$(f) $(TARGET_DIR)/bin
> +        )
> +endef
> +endif
>  COREUTILS_POST_INSTALL_TARGET_HOOKS += COREUTILS_CLEANUP_BIN
>  endif
>  
> @@ -110,14 +133,24 @@ ifeq ($(BR2_STATIC_LIBS),y)
>  COREUTILS_CONF_OPTS += --enable-no-install-program=stdbuf
>  endif
>  
> -define COREUTILS_CLEANUP
> +define COREUTILS_TEST_SYMLINK
>  	# link for archaic shells
> -	ln -fs coreutils $(TARGET_DIR)/usr/bin/[
> -	# gnu thinks chroot is in bin, debian thinks it's in sbin
> +	ln -fs $(TEST_BINARY) $(TARGET_DIR)/usr/bin/[

And I said above, I did two separate definitions of
COREUTILS_CREATE_TEST_SYMLINK instead.

See the final commit at
https://git.buildroot.org/buildroot/commit/?id=7989818466f8a963c4b6cc8b5014b04703535d3a.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH v3] package/coreutils: allow building individual binaries

[Markus Mayer]

On Thu, 20 Jun 2019 at 12:27, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Markus,
>
> On Thu,  6 Jun 2019 19:15:34 -0700
> Markus Mayer <mmayer@broadcom.com> wrote:
>
> > We add configuration option BR2_PACKAGE_COREUTILS_INDIVIDUAL_BINARIES
> > so that coreutils can be built and installed as individual binaries.
> > It can be used if the target file system doesn't support symlinks or
> > symlinks are undesirable.
> >
> > The approach is modelled after Busybox's similarly named configuration
> > option.
> >
> > Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>
> I have applied your patch, but after some changes.

[...]

> The namespace of variables in Buildroot is global, so using the name
> TEST_BINARY is a *very* bad idea. It should have been
> COREUTILS_TEST_BINARY. But in fact, I did this differently, and simply
> added two definitions of COREUTILS_CREATE_TEST_SYMLINK.

Good to know. I'll keep that in mind.

> See the final commit at
> https://git.buildroot.org/buildroot/commit/?id=7989818466f8a963c4b6cc8b5014b04703535d3a.

Looks good. Thanks for tweaking things to make the solution clearer.

Regards,
-Markus