[v2] coreutils: use single binary in symlink method, with merged usr

Message ID 1506536724-10603-1-git-send-email-casantos@datacom.ind.br
State New
Headers show
Series
  • [v2] coreutils: use single binary in symlink method, with merged usr
Related show

Commit Message

Carlos Santos Sept. 27, 2017, 6:25 p.m.
The symlink method is faster, since there is no shell fork/exec, and
provides extra space savings. Keep using the shebang method without
merged usr, because it makes easier to move binaries into other
directories.

Change-Id: Iad19df24a7428f8a3c0d87d62da57c659b0378d1
Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
 package/coreutils/coreutils.mk | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Thomas Petazzoni Sept. 27, 2017, 7:12 p.m. | #1
Hello,

On Wed, 27 Sep 2017 15:25:24 -0300, Carlos Santos wrote:
> The symlink method is faster, since there is no shell fork/exec, and
> provides extra space savings. Keep using the shebang method without
> merged usr, because it makes easier to move binaries into other
> directories.

I'm not sure why we would want to make a difference between merged /usr
and non-merged /usr. Is the shebang method really useful/interesting?
Busybox is using symlinks, and that is just fine. Why not do the same
for coreutils?

In addition, having too many differences between merged /usr and
non-merged /usr is a test/maintenance burden.

Thomas
Carlos Santos Sept. 27, 2017, 8:10 p.m. | #2
> From: "Thomas Petazzoni" <thomas.petazzoni@free-electrons.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: buildroot@buildroot.org
> Sent: Wednesday, September 27, 2017 4:12:10 PM
> Subject: Re: [Buildroot] [PATCH v2] coreutils: use single binary in symlink method, with merged usr

> Hello,
> 
> On Wed, 27 Sep 2017 15:25:24 -0300, Carlos Santos wrote:
>> The symlink method is faster, since there is no shell fork/exec, and
>> provides extra space savings. Keep using the shebang method without
>> merged usr, because it makes easier to move binaries into other
>> directories.
> 
> I'm not sure why we would want to make a difference between merged /usr
> and non-merged /usr. Is the shebang method really useful/interesting?
> Busybox is using symlinks, and that is just fine. Why not do the same
> for coreutils?

It is useful just because some binaries need to be moved after the
installation (see commit 03f87430e54d11912e707cad105fc4f2a2590aba message)

> In addition, having too many differences between merged /usr and
> non-merged /usr is a test/maintenance burden.

Yeah, I'd rater declare merged usr as the standard and stop worrying
about this. BusyBox, coreutils, util-linux, sysvinit and procps-ng
(just to name a few), competing for the ownership of the same utilities.

Even worst, they install those utilities in different places, so we
may end-up with two different implementations of link, mktemp, nice
printenv, free, top, uptime, w, killall5, getopt, linux32, linux64,
setarch, setpriv and fsfreeze (and I'm suspicious that this list is
not complete).

Patch

diff --git a/package/coreutils/coreutils.mk b/package/coreutils/coreutils.mk
index 6a8a31b..4b9dc99 100644
--- a/package/coreutils/coreutils.mk
+++ b/package/coreutils/coreutils.mk
@@ -14,7 +14,7 @@  COREUTILS_LICENSE_FILES = COPYING
 COREUTILS_AUTORECONF = YES
 COREUTILS_GETTEXTIZE = YES
 
-COREUTILS_CONF_OPTS = --disable-rpath --enable-single-binary=shebangs \
+COREUTILS_CONF_OPTS = --disable-rpath \
 	$(if $(BR2_TOOLCHAIN_USES_MUSL),--with-included-regex)
 COREUTILS_CONF_ENV = ac_cv_c_restrict=no \
 	ac_cv_func_chown_works=yes \
@@ -101,9 +101,15 @@  COREUTILS_CONF_OPTS += --with-openssl=yes
 COREUTILS_DEPENDENCIES += openssl
 endif
 
-ifeq ($(BR2_ROOTFS_MERGED_USR),)
+# Build as a single binary for some neat space savings. Use the symlink method
+# with merged usr. Otherwise use the shebang method since it allows us to move
+# binaries into other directories whereas for symlink that wouldn't be so easy.
+ifeq ($(BR2_ROOTFS_MERGED_USR),y)
+COREUTILS_CONF_OPTS += --enable-single-binary=symlinks
+else
+COREUTILS_CONF_OPTS += --enable-single-binary=shebangs
 define COREUTILS_CLEANUP_BIN
-	# some things go in root rather than usr
+	# some things go in /bin rather than /usr/bin
 	for f in $(COREUTILS_BIN_PROGS); do \
 		mv -f $(TARGET_DIR)/usr/bin/$$f $(TARGET_DIR)/bin/$$f || exit 1; \
 	done