[v2,2/2] coreutils: optimize the '[' symlink

Message ID 5821c4c9a85e95a15423616c8a7f674df515b714.1531338679.git.baruch@tkos.co.il
State Accepted
Headers show
Series
  • [v2,1/2] coreutils: fix chroot installation
Related show

Commit Message

Baruch Siach July 11, 2018, 7:51 p.m.
Link '[' directly to the coreutils binary instead of going through
'test'.

Suggested-by: Arnout Vandecappelle <arnout@mind.be>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
v2: New patch in this series
---
 package/coreutils/coreutils.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Carlos Santos July 11, 2018, 9:48 p.m. | #1
> From: "Baruch Siach" <baruch@tkos.co.il>
> To: "buildroot" <buildroot@busybox.net>
> Cc: "DATACOM" <casantos@datacom.com.br>, "Arnout Vandecappelle" <arnout@mind.be>, "Baruch Siach" <baruch@tkos.co.il>
> Sent: Wednesday, July 11, 2018 4:51:19 PM
> Subject: [PATCH v2 2/2] coreutils: optimize the '[' symlink

> Link '[' directly to the coreutils binary instead of going through
> 'test'.
> 
> Suggested-by: Arnout Vandecappelle <arnout@mind.be>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v2: New patch in this series
> ---
> package/coreutils/coreutils.mk | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/coreutils/coreutils.mk b/package/coreutils/coreutils.mk
> index 2fb4a32c794f..7e7251ff7396 100644
> --- a/package/coreutils/coreutils.mk
> +++ b/package/coreutils/coreutils.mk
> @@ -112,7 +112,7 @@ endif
> 
> define COREUTILS_CLEANUP
> 	# link for archaic shells
> -	ln -fs test $(TARGET_DIR)/usr/bin/[
> +	ln -fs coreutils $(TARGET_DIR)/usr/bin/[
> 	# gnu thinks chroot is in bin, debian thinks it's in sbin
> 	rm -f $(TARGET_DIR)/usr/bin/chroot
> 	ln -sf ../bin/coreutils $(TARGET_DIR)/usr/sbin/chroot
> --
> 2.18.0

Reviewd-by: Carlos Santos <casantos@datacom.com.br>
Arnout Vandecappelle July 11, 2018, 9:49 p.m. | #2
On 11-07-18 21:51, Baruch Siach wrote:
> Link '[' directly to the coreutils binary instead of going through
> 'test'.

 One more small question: did you do a runtime test to be sure that this
actually works? (That said, if this doesn't work, then the previous one also
can't have worked.)

 Regards,
 Arnout

> 
> Suggested-by: Arnout Vandecappelle <arnout@mind.be>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v2: New patch in this series
> ---
>  package/coreutils/coreutils.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/package/coreutils/coreutils.mk b/package/coreutils/coreutils.mk
> index 2fb4a32c794f..7e7251ff7396 100644
> --- a/package/coreutils/coreutils.mk
> +++ b/package/coreutils/coreutils.mk
> @@ -112,7 +112,7 @@ endif
>  
>  define COREUTILS_CLEANUP
>  	# link for archaic shells
> -	ln -fs test $(TARGET_DIR)/usr/bin/[
> +	ln -fs coreutils $(TARGET_DIR)/usr/bin/[
>  	# gnu thinks chroot is in bin, debian thinks it's in sbin
>  	rm -f $(TARGET_DIR)/usr/bin/chroot
>  	ln -sf ../bin/coreutils $(TARGET_DIR)/usr/sbin/chroot
>
Carlos Santos July 11, 2018, 10:06 p.m. | #3
> From: "Arnout Vandecappelle" <arnout@mind.be>
> To: "Baruch Siach" <baruch@tkos.co.il>, "buildroot" <buildroot@busybox.net>
> Cc: "DATACOM" <casantos@datacom.com.br>
> Sent: Wednesday, July 11, 2018 6:49:43 PM
> Subject: Re: [PATCH v2 2/2] coreutils: optimize the '[' symlink

> On 11-07-18 21:51, Baruch Siach wrote:
>> Link '[' directly to the coreutils binary instead of going through
>> 'test'.
> 
> One more small question: did you do a runtime test to be sure that this
> actually works? (That said, if this doesn't work, then the previous one also
> can't have worked.)

Yes, but it must be invoked directly since '[' is a shell built-in
command:

   # type [
   [ is a shell builtin
   # /usr/bin/[ -d / ] && echo ok
   ok
Baruch Siach July 12, 2018, 7:02 p.m. | #4
Hi Arnout,

Arnout Vandecappelle writes:
> On 11-07-18 21:51, Baruch Siach wrote:
>> Link '[' directly to the coreutils binary instead of going through
>> 'test'.
>
>  One more small question: did you do a runtime test to be sure that this
> actually works? (That said, if this doesn't work, then the previous one also
> can't have worked.)

Tested in chroot now:
# readlink /usr/bin/[
coreutils
# if /usr/bin/[ 0 -eq 0 ]; then echo true; else echo false; fi
true
# if /usr/bin/[ 1 -eq 0 ]; then echo true; else echo false; fi
false

baruch

>> Suggested-by: Arnout Vandecappelle <arnout@mind.be>
>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>> ---
>> v2: New patch in this series
>> ---
>>  package/coreutils/coreutils.mk | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/package/coreutils/coreutils.mk b/package/coreutils/coreutils.mk
>> index 2fb4a32c794f..7e7251ff7396 100644
>> --- a/package/coreutils/coreutils.mk
>> +++ b/package/coreutils/coreutils.mk
>> @@ -112,7 +112,7 @@ endif
>>  
>>  define COREUTILS_CLEANUP
>>  	# link for archaic shells
>> -	ln -fs test $(TARGET_DIR)/usr/bin/[
>> +	ln -fs coreutils $(TARGET_DIR)/usr/bin/[
>>  	# gnu thinks chroot is in bin, debian thinks it's in sbin
>>  	rm -f $(TARGET_DIR)/usr/bin/chroot
>>  	ln -sf ../bin/coreutils $(TARGET_DIR)/usr/sbin/chroot

Patch

diff --git a/package/coreutils/coreutils.mk b/package/coreutils/coreutils.mk
index 2fb4a32c794f..7e7251ff7396 100644
--- a/package/coreutils/coreutils.mk
+++ b/package/coreutils/coreutils.mk
@@ -112,7 +112,7 @@  endif
 
 define COREUTILS_CLEANUP
 	# link for archaic shells
-	ln -fs test $(TARGET_DIR)/usr/bin/[
+	ln -fs coreutils $(TARGET_DIR)/usr/bin/[
 	# gnu thinks chroot is in bin, debian thinks it's in sbin
 	rm -f $(TARGET_DIR)/usr/bin/chroot
 	ln -sf ../bin/coreutils $(TARGET_DIR)/usr/sbin/chroot