All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime
@ 2015-09-17 14:46 Jonathan Ben-Avraham
  2015-09-17 14:46 ` [Buildroot] [PATCH 2/2] Add dependency on gzip to lxc " Jonathan Ben-Avraham
  2015-09-17 15:01 ` [Buildroot] [PATCH 1/2] Add dependency on bash to gzip " Baruch Siach
  0 siblings, 2 replies; 17+ messages in thread
From: Jonathan Ben-Avraham @ 2015-09-17 14:46 UTC (permalink / raw
  To: buildroot

From: Jonathan Ben Avraham <yba@tkos.co.il>

The GNU gzip package provides eleven executable files, all but one of
which are bash shell scripts. If we allow inclusion of gzip without
bash, then on executing commands such as lxc-checkconfig that actually
use these shell scripts, you will get errors like 'zgrep: not found',
even though the zgrep executable is in PATH.

This patch also sharpens up the help text and eliminates the claim to
provide gzcat.

Signed-off-by: Jonathan Ben Avraham <yba@tkos.co.il>
---
 package/gzip/Config.in |   13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/package/gzip/Config.in b/package/gzip/Config.in
index a251425..9e2dc0d 100644
--- a/package/gzip/Config.in
+++ b/package/gzip/Config.in
@@ -1,11 +1,14 @@
 config BR2_PACKAGE_GZIP
 	bool "gzip"
-	depends on BR2_USE_WCHAR
+	depends on BR2_USE_WCHAR && BR2_PACKAGE_BASH
 	help
-	  Standard GNU compressor. Provides things like gzip,
-	  gunzip, gzcat, etc...
+	  The GNU implementation of gzip (http://www.gzip.org).
+	  Provides: gunzip, gzexe, gzip, zcat, zcmp, zdiff, zforce,
+	  zgrep, zless, zmore, and znew. All of the executables except
+	  gzip are Bourne-Again shell scripts. Busybox provides a gzip
+	  applet.
 
 	  http://www.gnu.org/software/gzip/gzip.html
 
-comment "gzip needs a toolchain w/ wchar"
-	depends on !BR2_USE_WCHAR
+comment "gzip needs bash and a toolchain w/ wchar"
+	depends on !BR2_USE_WCHAR || !BR2_PACKAGE_BASH
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 2/2] Add dependency on gzip to lxc for runtime
  2015-09-17 14:46 [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime Jonathan Ben-Avraham
@ 2015-09-17 14:46 ` Jonathan Ben-Avraham
  2015-09-17 15:03   ` Baruch Siach
  2015-09-17 15:01 ` [Buildroot] [PATCH 1/2] Add dependency on bash to gzip " Baruch Siach
  1 sibling, 1 reply; 17+ messages in thread
From: Jonathan Ben-Avraham @ 2015-09-17 14:46 UTC (permalink / raw
  To: buildroot

From: Jonathan Ben Avraham <yba@tkos.co.il>

lxc executables such as lxc-checkconfig require scripts provided by
gzip such as zgrep.

Signed-off-by: Jonathan Ben Avraham <yba@tkos.co.il>
---
 package/lxc/Config.in |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/package/lxc/Config.in b/package/lxc/Config.in
index ffd9b4a..e774eee 100644
--- a/package/lxc/Config.in
+++ b/package/lxc/Config.in
@@ -9,6 +9,7 @@ config BR2_PACKAGE_LXC
 	depends on !BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII201305
 	depends on !BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII201405
 	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_0 # libcap
+	depends on BR2_PACKAGE_GZIP
 	help
 	  Linux Containers (LXC), provides the ability to group and isolate
 	  of a set of processes in a jail by virtualizing and accounting the
@@ -16,10 +17,11 @@ config BR2_PACKAGE_LXC
 
 	  https://linuxcontainers.org/
 
-comment "lxc needs a toolchain w/ threads, headers >= 3.0, dynamic library"
+comment "lxc needs gzip, toolchain w/ threads, headers >= 3.0, dynamic library"
 	depends on BR2_USE_MMU
 	depends on !BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII201305
 	depends on !BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII201405
 	depends on !BR2_TOOLCHAIN_HAS_THREADS \
 		|| !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_0 \
-		|| BR2_STATIC_LIBS
+		|| BR2_STATIC_LIBS \
+		|| !BR2_PACKAGE_GZIP
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime
  2015-09-17 14:46 [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime Jonathan Ben-Avraham
  2015-09-17 14:46 ` [Buildroot] [PATCH 2/2] Add dependency on gzip to lxc " Jonathan Ben-Avraham
@ 2015-09-17 15:01 ` Baruch Siach
  2015-09-17 15:16   ` Jonathan Ben Avraham
  1 sibling, 1 reply; 17+ messages in thread
From: Baruch Siach @ 2015-09-17 15:01 UTC (permalink / raw
  To: buildroot

Hi Yonatan,

On Thu, Sep 17, 2015 at 05:46:32PM +0300, Jonathan Ben-Avraham wrote:
> From: Jonathan Ben Avraham <yba@tkos.co.il>
> 
> The GNU gzip package provides eleven executable files, all but one of
> which are bash shell scripts. If we allow inclusion of gzip without
> bash, then on executing commands such as lxc-checkconfig that actually
> use these shell scripts, you will get errors like 'zgrep: not found',
> even though the zgrep executable is in PATH.

Which /bin/sh shell show this problem?

On my host the command

   busybox sh /bin/zgrep pattern file.gz

and also

   dash /bin/zgrep pattern file.gz

work as expected.

baruch

> This patch also sharpens up the help text and eliminates the claim to
> provide gzcat.
> 
> Signed-off-by: Jonathan Ben Avraham <yba@tkos.co.il>
> ---
>  package/gzip/Config.in |   13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/package/gzip/Config.in b/package/gzip/Config.in
> index a251425..9e2dc0d 100644
> --- a/package/gzip/Config.in
> +++ b/package/gzip/Config.in
> @@ -1,11 +1,14 @@
>  config BR2_PACKAGE_GZIP
>  	bool "gzip"
> -	depends on BR2_USE_WCHAR
> +	depends on BR2_USE_WCHAR && BR2_PACKAGE_BASH
>  	help
> -	  Standard GNU compressor. Provides things like gzip,
> -	  gunzip, gzcat, etc...
> +	  The GNU implementation of gzip (http://www.gzip.org).
> +	  Provides: gunzip, gzexe, gzip, zcat, zcmp, zdiff, zforce,
> +	  zgrep, zless, zmore, and znew. All of the executables except
> +	  gzip are Bourne-Again shell scripts. Busybox provides a gzip
> +	  applet.
>  
>  	  http://www.gnu.org/software/gzip/gzip.html
>  
> -comment "gzip needs a toolchain w/ wchar"
> -	depends on !BR2_USE_WCHAR
> +comment "gzip needs bash and a toolchain w/ wchar"
> +	depends on !BR2_USE_WCHAR || !BR2_PACKAGE_BASH

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 2/2] Add dependency on gzip to lxc for runtime
  2015-09-17 14:46 ` [Buildroot] [PATCH 2/2] Add dependency on gzip to lxc " Jonathan Ben-Avraham
@ 2015-09-17 15:03   ` Baruch Siach
  2015-09-17 15:20     ` Jonathan Ben Avraham
  0 siblings, 1 reply; 17+ messages in thread
From: Baruch Siach @ 2015-09-17 15:03 UTC (permalink / raw
  To: buildroot

Hi Yonatan,

On Thu, Sep 17, 2015 at 05:46:33PM +0300, Jonathan Ben-Avraham wrote:
> From: Jonathan Ben Avraham <yba@tkos.co.il>
> 
> lxc executables such as lxc-checkconfig require scripts provided by
> gzip such as zgrep.
> 
> Signed-off-by: Jonathan Ben Avraham <yba@tkos.co.il>
> ---
>  package/lxc/Config.in |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/package/lxc/Config.in b/package/lxc/Config.in
> index ffd9b4a..e774eee 100644
> --- a/package/lxc/Config.in
> +++ b/package/lxc/Config.in
> @@ -9,6 +9,7 @@ config BR2_PACKAGE_LXC
>  	depends on !BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII201305
>  	depends on !BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII201405
>  	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_0 # libcap
> +	depends on BR2_PACKAGE_GZIP

We generally use 'select' for non obvious dependencies like this one. You'll 
also need to propagate the wchar dependency of gzip here.

baruch

>  	help
>  	  Linux Containers (LXC), provides the ability to group and isolate
>  	  of a set of processes in a jail by virtualizing and accounting the
> @@ -16,10 +17,11 @@ config BR2_PACKAGE_LXC
>  
>  	  https://linuxcontainers.org/
>  
> -comment "lxc needs a toolchain w/ threads, headers >= 3.0, dynamic library"
> +comment "lxc needs gzip, toolchain w/ threads, headers >= 3.0, dynamic library"
>  	depends on BR2_USE_MMU
>  	depends on !BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII201305
>  	depends on !BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII201405
>  	depends on !BR2_TOOLCHAIN_HAS_THREADS \
>  		|| !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_0 \
> -		|| BR2_STATIC_LIBS
> +		|| BR2_STATIC_LIBS \
> +		|| !BR2_PACKAGE_GZIP

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime
  2015-09-17 15:01 ` [Buildroot] [PATCH 1/2] Add dependency on bash to gzip " Baruch Siach
@ 2015-09-17 15:16   ` Jonathan Ben Avraham
  2015-09-17 16:20     ` Baruch Siach
  0 siblings, 1 reply; 17+ messages in thread
From: Jonathan Ben Avraham @ 2015-09-17 15:16 UTC (permalink / raw
  To: buildroot

Hi Baruch,
See inlines...

On Thu, 17 Sep 2015, Baruch Siach wrote:

> Date: Thu, 17 Sep 2015 18:01:12 +0300
> From: Baruch Siach <baruch@tkos.co.il>
> To: Jonathan Ben-Avraham <yba@tkos.co.il>
> Cc: buildroot at buildroot.org
> Subject: Re: [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for
>     runtime
> 
> Hi Yonatan,
>
> On Thu, Sep 17, 2015 at 05:46:32PM +0300, Jonathan Ben-Avraham wrote:
>> From: Jonathan Ben Avraham <yba@tkos.co.il>
>>
>> The GNU gzip package provides eleven executable files, all but one of
>> which are bash shell scripts. If we allow inclusion of gzip without
>> bash, then on executing commands such as lxc-checkconfig that actually
>> use these shell scripts, you will get errors like 'zgrep: not found',
>> even though the zgrep executable is in PATH.
>
> Which /bin/sh shell show this problem?
>
> On my host the command
>
>   busybox sh /bin/zgrep pattern file.gz
>
> and also
>
>   dash /bin/zgrep pattern file.gz
>
> work as expected.

You tested *all* of the scripts or just this one? What if ash breaks even 
*one*?

  - yba


> baruch
>
>> This patch also sharpens up the help text and eliminates the claim to
>> provide gzcat.
>>
>> Signed-off-by: Jonathan Ben Avraham <yba@tkos.co.il>
>> ---
>>  package/gzip/Config.in |   13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/package/gzip/Config.in b/package/gzip/Config.in
>> index a251425..9e2dc0d 100644
>> --- a/package/gzip/Config.in
>> +++ b/package/gzip/Config.in
>> @@ -1,11 +1,14 @@
>>  config BR2_PACKAGE_GZIP
>>  	bool "gzip"
>> -	depends on BR2_USE_WCHAR
>> +	depends on BR2_USE_WCHAR && BR2_PACKAGE_BASH
>>  	help
>> -	  Standard GNU compressor. Provides things like gzip,
>> -	  gunzip, gzcat, etc...
>> +	  The GNU implementation of gzip (http://www.gzip.org).
>> +	  Provides: gunzip, gzexe, gzip, zcat, zcmp, zdiff, zforce,
>> +	  zgrep, zless, zmore, and znew. All of the executables except
>> +	  gzip are Bourne-Again shell scripts. Busybox provides a gzip
>> +	  applet.
>>
>>  	  http://www.gnu.org/software/gzip/gzip.html
>>
>> -comment "gzip needs a toolchain w/ wchar"
>> -	depends on !BR2_USE_WCHAR
>> +comment "gzip needs bash and a toolchain w/ wchar"
>> +	depends on !BR2_USE_WCHAR || !BR2_PACKAGE_BASH
>
>

-- 
  9590 8E58 D30D 1660 C349  673D B205 4FC4 B8F5 B7F9  ~. .~  Tk Open Systems
=}-------- Jonathan Ben-Avraham ("yba") ----------ooO--U--Ooo------------{=
mailto:yba at tkos.co.il tel:+972.52.486.3386 http://tkos.co.il skype:benavrhm

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 2/2] Add dependency on gzip to lxc for runtime
  2015-09-17 15:03   ` Baruch Siach
@ 2015-09-17 15:20     ` Jonathan Ben Avraham
  2015-09-17 17:16       ` Thomas Petazzoni
  0 siblings, 1 reply; 17+ messages in thread
From: Jonathan Ben Avraham @ 2015-09-17 15:20 UTC (permalink / raw
  To: buildroot

Hi Baruch,
See inlines...

On Thu, 17 Sep 2015, Baruch Siach wrote:

> Date: Thu, 17 Sep 2015 18:03:53 +0300
> From: Baruch Siach <baruch@tkos.co.il>
> To: Jonathan Ben-Avraham <yba@tkos.co.il>
> Cc: buildroot at buildroot.org
> Subject: Re: [Buildroot] [PATCH 2/2] Add dependency on gzip to lxc for runtime
> 
> Hi Yonatan,
>
> On Thu, Sep 17, 2015 at 05:46:33PM +0300, Jonathan Ben-Avraham wrote:
>> From: Jonathan Ben Avraham <yba@tkos.co.il>
>>
>> lxc executables such as lxc-checkconfig require scripts provided by
>> gzip such as zgrep.
>>
>> Signed-off-by: Jonathan Ben Avraham <yba@tkos.co.il>
>> ---
>>  package/lxc/Config.in |    6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/package/lxc/Config.in b/package/lxc/Config.in
>> index ffd9b4a..e774eee 100644
>> --- a/package/lxc/Config.in
>> +++ b/package/lxc/Config.in
>> @@ -9,6 +9,7 @@ config BR2_PACKAGE_LXC
>>  	depends on !BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII201305
>>  	depends on !BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII201405
>>  	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_0 # libcap
>> +	depends on BR2_PACKAGE_GZIP
>
> We generally use 'select' for non obvious dependencies like this one. You'll
> also need to propagate the wchar dependency of gzip here.

I didn't use 'select' because of its implicit, sneaky nature. Bash is too 
heavy to just "select" it into a rootfs without the user explicitly 
putting an 'X' in the checkbox.

  - yba


> baruch
>
>>  	help
>>  	  Linux Containers (LXC), provides the ability to group and isolate
>>  	  of a set of processes in a jail by virtualizing and accounting the
>> @@ -16,10 +17,11 @@ config BR2_PACKAGE_LXC
>>
>>  	  https://linuxcontainers.org/
>>
>> -comment "lxc needs a toolchain w/ threads, headers >= 3.0, dynamic library"
>> +comment "lxc needs gzip, toolchain w/ threads, headers >= 3.0, dynamic library"
>>  	depends on BR2_USE_MMU
>>  	depends on !BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII201305
>>  	depends on !BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII201405
>>  	depends on !BR2_TOOLCHAIN_HAS_THREADS \
>>  		|| !BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_0 \
>> -		|| BR2_STATIC_LIBS
>> +		|| BR2_STATIC_LIBS \
>> +		|| !BR2_PACKAGE_GZIP
>
>

-- 
  9590 8E58 D30D 1660 C349  673D B205 4FC4 B8F5 B7F9  ~. .~  Tk Open Systems
=}-------- Jonathan Ben-Avraham ("yba") ----------ooO--U--Ooo------------{=
mailto:yba at tkos.co.il tel:+972.52.486.3386 http://tkos.co.il skype:benavrhm

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime
  2015-09-17 15:16   ` Jonathan Ben Avraham
@ 2015-09-17 16:20     ` Baruch Siach
  2015-09-17 16:33       ` Jonathan Ben Avraham
  0 siblings, 1 reply; 17+ messages in thread
From: Baruch Siach @ 2015-09-17 16:20 UTC (permalink / raw
  To: buildroot

Hi Yonatan,

On Thu, Sep 17, 2015 at 06:16:42PM +0300, Jonathan Ben Avraham wrote:
> On Thu, 17 Sep 2015, Baruch Siach wrote:
> 
> >Date: Thu, 17 Sep 2015 18:01:12 +0300
> >From: Baruch Siach <baruch@tkos.co.il>
> >To: Jonathan Ben-Avraham <yba@tkos.co.il>
> >Cc: buildroot at buildroot.org
> >Subject: Re: [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for
> >    runtime
> >
> >On Thu, Sep 17, 2015 at 05:46:32PM +0300, Jonathan Ben-Avraham wrote:
> >>From: Jonathan Ben Avraham <yba@tkos.co.il>
> >>
> >>The GNU gzip package provides eleven executable files, all but one of
> >>which are bash shell scripts. If we allow inclusion of gzip without
> >>bash, then on executing commands such as lxc-checkconfig that actually
> >>use these shell scripts, you will get errors like 'zgrep: not found',
> >>even though the zgrep executable is in PATH.
> >
> >Which /bin/sh shell show this problem?
> >
> >On my host the command
> >
> >  busybox sh /bin/zgrep pattern file.gz
> >
> >and also
> >
> >  dash /bin/zgrep pattern file.gz
> >
> >work as expected.
> 
> You tested *all* of the scripts or just this one?

I didn't test all of them. I'm just trying to reproduce the 'zgrep: not found' 
problem that you report in the commit log, to see what went wrong.

> What if ash breaks even *one*?

Then we should probably patch the problem and send upstream. gzip bundled 
scripts are pretty trivial, and AFAICS, should work under any POSIX-like 
shell.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime
  2015-09-17 16:20     ` Baruch Siach
@ 2015-09-17 16:33       ` Jonathan Ben Avraham
  2015-09-17 17:23         ` Thomas Petazzoni
  0 siblings, 1 reply; 17+ messages in thread
From: Jonathan Ben Avraham @ 2015-09-17 16:33 UTC (permalink / raw
  To: buildroot

Hi Baruch,
See inlines...

On Thu, 17 Sep 2015, Baruch Siach wrote:

> Date: Thu, 17 Sep 2015 19:20:03 +0300
> From: Baruch Siach <baruch@tkos.co.il>
> To: Jonathan Ben Avraham <yba@tkos.co.il>
> Cc: buildroot at buildroot.org
> Subject: Re: [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for
>     runtime
> 
> Hi Yonatan,
>
> On Thu, Sep 17, 2015 at 06:16:42PM +0300, Jonathan Ben Avraham wrote:
>> On Thu, 17 Sep 2015, Baruch Siach wrote:
>>
>>> Date: Thu, 17 Sep 2015 18:01:12 +0300
>>> From: Baruch Siach <baruch@tkos.co.il>
>>> To: Jonathan Ben-Avraham <yba@tkos.co.il>
>>> Cc: buildroot at buildroot.org
>>> Subject: Re: [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for
>>>    runtime
>>>
>>> On Thu, Sep 17, 2015 at 05:46:32PM +0300, Jonathan Ben-Avraham wrote:
>>>> From: Jonathan Ben Avraham <yba@tkos.co.il>
>>>>
>>>> The GNU gzip package provides eleven executable files, all but one of
>>>> which are bash shell scripts. If we allow inclusion of gzip without
>>>> bash, then on executing commands such as lxc-checkconfig that actually
>>>> use these shell scripts, you will get errors like 'zgrep: not found',
>>>> even though the zgrep executable is in PATH.
>>>
>>> Which /bin/sh shell show this problem?
>>>
>>> On my host the command
>>>
>>>  busybox sh /bin/zgrep pattern file.gz
>>>
>>> and also
>>>
>>>  dash /bin/zgrep pattern file.gz
>>>
>>> work as expected.
>>
>> You tested *all* of the scripts or just this one?
>
> I didn't test all of them. I'm just trying to reproduce the 'zgrep: not found'
> problem that you report in the commit log, to see what went wrong.
>
>> What if ash breaks even *one*?
>
> Then we should probably patch the problem and send upstream. gzip bundled
> scripts are pretty trivial, and AFAICS, should work under any POSIX-like
> shell.

But we would at least need to ln -s /bin/<ash|dash> /bin/bash in 
Buildroot, no? so maybe the upstream fix is to change the hashbangs from 
/bin/bash to /bin/sh. Do you really think that the gzip upstream would 
accept this?

  - yba


> baruch
>
>

-- 
  9590 8E58 D30D 1660 C349  673D B205 4FC4 B8F5 B7F9  ~. .~  Tk Open Systems
=}-------- Jonathan Ben-Avraham ("yba") ----------ooO--U--Ooo------------{=
mailto:yba at tkos.co.il tel:+972.52.486.3386 http://tkos.co.il skype:benavrhm

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 2/2] Add dependency on gzip to lxc for runtime
  2015-09-17 15:20     ` Jonathan Ben Avraham
@ 2015-09-17 17:16       ` Thomas Petazzoni
  0 siblings, 0 replies; 17+ messages in thread
From: Thomas Petazzoni @ 2015-09-17 17:16 UTC (permalink / raw
  To: buildroot

Hello,

On Thu, 17 Sep 2015 18:20:34 +0300 (IDT), Jonathan Ben Avraham wrote:

> > Date: Thu, 17 Sep 2015 18:03:53 +0300
> > From: Baruch Siach <baruch@tkos.co.il>
> > To: Jonathan Ben-Avraham <yba@tkos.co.il>
> > Cc: buildroot at buildroot.org
> > Subject: Re: [Buildroot] [PATCH 2/2] Add dependency on gzip to lxc for runtime
> > 
> > Hi Yonatan,
> >
> > On Thu, Sep 17, 2015 at 05:46:33PM +0300, Jonathan Ben-Avraham wrote:
> >> From: Jonathan Ben Avraham <yba@tkos.co.il>
> >>
> >> lxc executables such as lxc-checkconfig require scripts provided by
> >> gzip such as zgrep.
> >>
> >> Signed-off-by: Jonathan Ben Avraham <yba@tkos.co.il>
> >> ---
> >>  package/lxc/Config.in |    6 ++++--
> >>  1 file changed, 4 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/package/lxc/Config.in b/package/lxc/Config.in
> >> index ffd9b4a..e774eee 100644
> >> --- a/package/lxc/Config.in
> >> +++ b/package/lxc/Config.in
> >> @@ -9,6 +9,7 @@ config BR2_PACKAGE_LXC
> >>  	depends on !BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII201305
> >>  	depends on !BR2_TOOLCHAIN_EXTERNAL_CODESOURCERY_NIOSII201405
> >>  	depends on BR2_TOOLCHAIN_HEADERS_AT_LEAST_3_0 # libcap
> >> +	depends on BR2_PACKAGE_GZIP
> >
> > We generally use 'select' for non obvious dependencies like this one. You'll
> > also need to propagate the wchar dependency of gzip here.
> 
> I didn't use 'select' because of its implicit, sneaky nature. Bash is too 
> heavy to just "select" it into a rootfs without the user explicitly 
> putting an 'X' in the checkbox.

Still, we want such dependencies to use a "select" like Baruch said.
They should even be noted this way:

	select BR2_PACKAGE_GZIP # runtime dep only

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime
  2015-09-17 16:33       ` Jonathan Ben Avraham
@ 2015-09-17 17:23         ` Thomas Petazzoni
  2015-09-17 19:04           ` Baruch Siach
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Petazzoni @ 2015-09-17 17:23 UTC (permalink / raw
  To: buildroot

Dear Jonathan Ben Avraham,

On Thu, 17 Sep 2015 19:33:24 +0300 (IDT), Jonathan Ben Avraham wrote:

> > Then we should probably patch the problem and send upstream. gzip bundled
> > scripts are pretty trivial, and AFAICS, should work under any POSIX-like
> > shell.
> 
> But we would at least need to ln -s /bin/<ash|dash> /bin/bash in 
> Buildroot, no? so maybe the upstream fix is to change the hashbangs from 
> /bin/bash to /bin/sh. Do you really think that the gzip upstream would 
> accept this?

All the gzip scripts are in fact generated. zgrep is generated from
zgrep.in. zgrep.in contains:

#!/bin/sh

which gets replaced by the gzip Makefile.am by the value of $(SHELL):

SUFFIXES = .in
.in:
        $(AM_V_GEN)sed \
                -e 's|/bin/sh|$(SHELL)|g' \
                -e 's|[@]bindir@|'\''$(bindir)'\''|g' \
                -e 's|[@]GREP@|$(GREP)|g' \
                -e 's|[@]VERSION@|$(VERSION)|g' \
                $(srcdir)/$@.in >$@-t \
          && chmod a+x $@-t \
          && mv $@-t $@

Which is completely wrong, because it makes the assumption that
$(SHELL) on your build machine is going to be available on your target.

So your patch is in fact wrong I believe, because if someone is using
zsh, or tcsh or some other shell on their build machine, $(SHELL) will
point to this shell and not to bash, and the scripts generated by gzip
will use zsh/tcsh, etc. on the target.

So the proper way is most likely to adjust the gzip build system so
that we can pass at configure time a different value for the shell path
on the target (and it would default to $(SHELL) is not passed).

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime
  2015-09-17 17:23         ` Thomas Petazzoni
@ 2015-09-17 19:04           ` Baruch Siach
  2015-09-17 20:15             ` Thomas Petazzoni
  0 siblings, 1 reply; 17+ messages in thread
From: Baruch Siach @ 2015-09-17 19:04 UTC (permalink / raw
  To: buildroot

Hi Thomas,

On Thu, Sep 17, 2015 at 07:23:15PM +0200, Thomas Petazzoni wrote:
> On Thu, 17 Sep 2015 19:33:24 +0300 (IDT), Jonathan Ben Avraham wrote:
> 
> > > Then we should probably patch the problem and send upstream. gzip bundled
> > > scripts are pretty trivial, and AFAICS, should work under any POSIX-like
> > > shell.
> > 
> > But we would at least need to ln -s /bin/<ash|dash> /bin/bash in 
> > Buildroot, no? so maybe the upstream fix is to change the hashbangs from 
> > /bin/bash to /bin/sh. Do you really think that the gzip upstream would 
> > accept this?
> 
> All the gzip scripts are in fact generated. zgrep is generated from
> zgrep.in. zgrep.in contains:
> 
> #!/bin/sh
> 
> which gets replaced by the gzip Makefile.am by the value of $(SHELL):
> 
> SUFFIXES = .in
> .in:
>         $(AM_V_GEN)sed \
>                 -e 's|/bin/sh|$(SHELL)|g' \
>                 -e 's|[@]bindir@|'\''$(bindir)'\''|g' \
>                 -e 's|[@]GREP@|$(GREP)|g' \
>                 -e 's|[@]VERSION@|$(VERSION)|g' \
>                 $(srcdir)/$@.in >$@-t \
>           && chmod a+x $@-t \
>           && mv $@-t $@
> 
> Which is completely wrong, because it makes the assumption that
> $(SHELL) on your build machine is going to be available on your target.
> 
> So your patch is in fact wrong I believe, because if someone is using
> zsh, or tcsh or some other shell on their build machine, $(SHELL) will
> point to this shell and not to bash, and the scripts generated by gzip
> will use zsh/tcsh, etc. on the target.
> 
> So the proper way is most likely to adjust the gzip build system so
> that we can pass at configure time a different value for the shell path
> on the target (and it would default to $(SHELL) is not passed).

Based on the following snippet from Buildroot top level Makefile I would 
assume that $(SHELL) is /bin/bash on my host machine:

# we want bash as shell
SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
         else if [ -x /bin/bash ]; then echo /bin/bash; \
         else echo sh; fi; fi)

However, 'make printvars' shows

   SHELL=/bin/sh (/bin/sh)

$(SHELL) expands to /bin/sh in the gzip Makefile, and generated scripts carry 
the /bin/sh shebang. I have no idea how.

As for the original problem, I managed to override the environment $(SHELL) as 
indicated in the gzip INSTALL file:

   GZIP_CONF_ENV = CONFIG_SHELL=/bin/bash

So the right fix, I guess, is just

   GZIP_CONF_ENV = CONFIG_SHELL=/bin/sh

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime
  2015-09-17 19:04           ` Baruch Siach
@ 2015-09-17 20:15             ` Thomas Petazzoni
  2015-09-19 20:36               ` Jonathan Ben Avraham
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Petazzoni @ 2015-09-17 20:15 UTC (permalink / raw
  To: buildroot

Baruch,

On Thu, 17 Sep 2015 22:04:24 +0300, Baruch Siach wrote:

> Based on the following snippet from Buildroot top level Makefile I would 
> assume that $(SHELL) is /bin/bash on my host machine:
> 
> # we want bash as shell
> SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
>          else if [ -x /bin/bash ]; then echo /bin/bash; \
>          else echo sh; fi; fi)
> 
> However, 'make printvars' shows
> 
>    SHELL=/bin/sh (/bin/sh)

Here:

SHELL=/bin/bash (/bin/bash)

> $(SHELL) expands to /bin/sh in the gzip Makefile, and generated scripts carry 
> the /bin/sh shebang. I have no idea how.
> 
> As for the original problem, I managed to override the environment $(SHELL) as 
> indicated in the gzip INSTALL file:
> 
>    GZIP_CONF_ENV = CONFIG_SHELL=/bin/bash
> 
> So the right fix, I guess, is just
> 
>    GZIP_CONF_ENV = CONFIG_SHELL=/bin/sh

It is not a completely correct fix. According to
https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/config_002estatus-Invocation.html,
CONFIG_SHELL is:

"""
The shell with which to run configure. It must be Bourne-compatible,
and the absolute name of the shell should be passed. The default is a
shell that supports LINENO if available, and /bin/sh otherwise.
"""

I believe there is no proper way with the current gzip build system to
fix the problem. It is inherently wrong to assume that the $(SHELL)
used on the host will be available on the target. The only sane option
I believe is to have a --with-shell=<foo> option, which if not given
defaults to $(SHELL) to retain the existing behavior.

Another simpler option is to simply add a post-install target hook that
will adjust the shebang of the shell scripts installed by gzip.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime
  2015-09-17 20:15             ` Thomas Petazzoni
@ 2015-09-19 20:36               ` Jonathan Ben Avraham
  2015-09-20  8:22                 ` Thomas Petazzoni
  0 siblings, 1 reply; 17+ messages in thread
From: Jonathan Ben Avraham @ 2015-09-19 20:36 UTC (permalink / raw
  To: buildroot

Hi Thomas,
See inline comment below.

On Thu, 17 Sep 2015, Thomas Petazzoni wrote:

> Date: Thu, 17 Sep 2015 22:15:46 +0200
> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> To: Baruch Siach <baruch@tkos.co.il>
> Cc: Jonathan Ben Avraham <yba@tkos.co.il>, buildroot at buildroot.org
> Subject: Re: [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for
>     runtime
> 
> Baruch,
>
> On Thu, 17 Sep 2015 22:04:24 +0300, Baruch Siach wrote:
>
>> Based on the following snippet from Buildroot top level Makefile I would
>> assume that $(SHELL) is /bin/bash on my host machine:
>>
>> # we want bash as shell
>> SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
>>          else if [ -x /bin/bash ]; then echo /bin/bash; \
>>          else echo sh; fi; fi)
>>
>> However, 'make printvars' shows
>>
>>    SHELL=/bin/sh (/bin/sh)
>
> Here:
>
> SHELL=/bin/bash (/bin/bash)
>
>> $(SHELL) expands to /bin/sh in the gzip Makefile, and generated scripts carry
>> the /bin/sh shebang. I have no idea how.
>>
>> As for the original problem, I managed to override the environment $(SHELL) as
>> indicated in the gzip INSTALL file:
>>
>>    GZIP_CONF_ENV = CONFIG_SHELL=/bin/bash
>>
>> So the right fix, I guess, is just
>>
>>    GZIP_CONF_ENV = CONFIG_SHELL=/bin/sh
>
> It is not a completely correct fix. According to
> https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/config_002estatus-Invocation.html,
> CONFIG_SHELL is:
>
> """
> The shell with which to run configure. It must be Bourne-compatible,
> and the absolute name of the shell should be passed. The default is a
> shell that supports LINENO if available, and /bin/sh otherwise.
> """
>
> I believe there is no proper way with the current gzip build system to
> fix the problem. It is inherently wrong to assume that the $(SHELL)
> used on the host will be available on the target. The only sane option
> I believe is to have a --with-shell=<foo> option, which if not given
> defaults to $(SHELL) to retain the existing behavior.

If I understand correctly you mean that the correct fix would require 
submitting a patch to the gzip maintainer to add the --with-shell 
configure option. This would not obviate the post-install hook you suggest 
below because we would need a heuristic to use with gzip --with-shell 
option.

> Another simpler option is to simply add a post-install target hook that
> will adjust the shebang of the shell scripts installed by gzip.

In order to do this post-install hook we would need to know the shells 
that the user selected and choose one of them using some heuristic rule 
such as:

1. If the user chose the bash package, then use /bin/bash
2. If the user chose Busybox and not bash then use /bin/ash
3. If the user chose neither Busybox nor bash then use /bin/sh and 
hope for the best

If the user then changes the configuration and un-selects bash we would 
need to cause the gzip package to be re-built.

I wonder if all of this is worth it. IMHO it would be enough to note in 
the gzip package help text that the user might need to adjust the 
hashbangs of the scripts in some post-build script.

  - yba



> Best regards,
>
> Thomas
>

-- 
  9590 8E58 D30D 1660 C349  673D B205 4FC4 B8F5 B7F9  ~. .~  Tk Open Systems
=}-------- Jonathan Ben-Avraham ("yba") ----------ooO--U--Ooo------------{=
mailto:yba at tkos.co.il tel:+972.52.486.3386 http://tkos.co.il skype:benavrhm

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime
  2015-09-19 20:36               ` Jonathan Ben Avraham
@ 2015-09-20  8:22                 ` Thomas Petazzoni
  2015-09-20 10:39                   ` Arnout Vandecappelle
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Petazzoni @ 2015-09-20  8:22 UTC (permalink / raw
  To: buildroot

Jonathan,

On Sat, 19 Sep 2015 23:36:47 +0300 (IDT), Jonathan Ben Avraham wrote:

> > I believe there is no proper way with the current gzip build system to
> > fix the problem. It is inherently wrong to assume that the $(SHELL)
> > used on the host will be available on the target. The only sane option
> > I believe is to have a --with-shell=<foo> option, which if not given
> > defaults to $(SHELL) to retain the existing behavior.
> 
> If I understand correctly you mean that the correct fix would require 
> submitting a patch to the gzip maintainer to add the --with-shell 
> configure option.

Correct.

> This would not obviate the post-install hook you suggest 
> below because we would need a heuristic to use with gzip --with-shell 
> option.

I'm not sure to follow you. We would just have to do:

GZIP_CONF_ENV += --with-shell=/the/right/value

> In order to do this post-install hook we would need to know the shells 
> that the user selected and choose one of them using some heuristic rule 
> such as:
> 
> 1. If the user chose the bash package, then use /bin/bash
> 2. If the user chose Busybox and not bash then use /bin/ash
> 3. If the user chose neither Busybox nor bash then use /bin/sh and 
> hope for the best
> 
> If the user then changes the configuration and un-selects bash we would 
> need to cause the gzip package to be re-built.

No: we don't handle such automatic rebuilds after configuration
changes. See
http://buildroot.org/downloads/manual/manual.html#full-rebuild.

> 
> I wonder if all of this is worth it. IMHO it would be enough to note in 
> the gzip package help text that the user might need to adjust the 
> hashbangs of the scripts in some post-build script.

There's some logic in the skeleton package that guarantees that /bin/sh
points to the system shell selected by the user in his configuration.
So forcing /bin/sh as the shebang should be fine.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime
  2015-09-20  8:22                 ` Thomas Petazzoni
@ 2015-09-20 10:39                   ` Arnout Vandecappelle
  2015-09-20 11:16                     ` Thomas Petazzoni
  0 siblings, 1 reply; 17+ messages in thread
From: Arnout Vandecappelle @ 2015-09-20 10:39 UTC (permalink / raw
  To: buildroot

On 20-09-15 10:22, Thomas Petazzoni wrote:
> Jonathan,
> 
> On Sat, 19 Sep 2015 23:36:47 +0300 (IDT), Jonathan Ben Avraham wrote:
[snip]
>> I wonder if all of this is worth it. IMHO it would be enough to note in 
>> the gzip package help text that the user might need to adjust the 
>> hashbangs of the scripts in some post-build script.
> 
> There's some logic in the skeleton package that guarantees that /bin/sh
> points to the system shell selected by the user in his configuration.
> So forcing /bin/sh as the shebang should be fine.

 Well, if the scripts indeed require Bourne extensions, then possibly ash and
dash will not work... And I have no idea about zsh either.

 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime
  2015-09-20 10:39                   ` Arnout Vandecappelle
@ 2015-09-20 11:16                     ` Thomas Petazzoni
  2015-09-20 11:35                       ` Baruch Siach
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Petazzoni @ 2015-09-20 11:16 UTC (permalink / raw
  To: buildroot

Hello,

On Sun, 20 Sep 2015 12:39:31 +0200, Arnout Vandecappelle wrote:

> > There's some logic in the skeleton package that guarantees that /bin/sh
> > points to the system shell selected by the user in his configuration.
> > So forcing /bin/sh as the shebang should be fine.
> 
>  Well, if the scripts indeed require Bourne extensions, then possibly ash and
> dash will not work... And I have no idea about zsh either.

I think Baruch said he had tried, and the script were working fine with
the Busybox shell.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime
  2015-09-20 11:16                     ` Thomas Petazzoni
@ 2015-09-20 11:35                       ` Baruch Siach
  0 siblings, 0 replies; 17+ messages in thread
From: Baruch Siach @ 2015-09-20 11:35 UTC (permalink / raw
  To: buildroot

Hi Thomas,

On Sun, Sep 20, 2015 at 01:16:47PM +0200, Thomas Petazzoni wrote:
> On Sun, 20 Sep 2015 12:39:31 +0200, Arnout Vandecappelle wrote:
> > > There's some logic in the skeleton package that guarantees that /bin/sh
> > > points to the system shell selected by the user in his configuration.
> > > So forcing /bin/sh as the shebang should be fine.
> > 
> >  Well, if the scripts indeed require Bourne extensions, then possibly ash and
> > dash will not work... And I have no idea about zsh either.
> 
> I think Baruch said he had tried, and the script were working fine with
> the Busybox shell.

I only tried zgrep. But as I said, all these scripts look pretty trivial, and 
I can't find any mention of shell dependencies in gzip sources. On Debian all 
of these scrips use the /bin/sh shebang, and AFAIK Debian does not guarantee 
/bin/sh to point to bash. I think we can safely assume that any POSIX-like 
shell will work.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-09-20 11:35 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-17 14:46 [Buildroot] [PATCH 1/2] Add dependency on bash to gzip for runtime Jonathan Ben-Avraham
2015-09-17 14:46 ` [Buildroot] [PATCH 2/2] Add dependency on gzip to lxc " Jonathan Ben-Avraham
2015-09-17 15:03   ` Baruch Siach
2015-09-17 15:20     ` Jonathan Ben Avraham
2015-09-17 17:16       ` Thomas Petazzoni
2015-09-17 15:01 ` [Buildroot] [PATCH 1/2] Add dependency on bash to gzip " Baruch Siach
2015-09-17 15:16   ` Jonathan Ben Avraham
2015-09-17 16:20     ` Baruch Siach
2015-09-17 16:33       ` Jonathan Ben Avraham
2015-09-17 17:23         ` Thomas Petazzoni
2015-09-17 19:04           ` Baruch Siach
2015-09-17 20:15             ` Thomas Petazzoni
2015-09-19 20:36               ` Jonathan Ben Avraham
2015-09-20  8:22                 ` Thomas Petazzoni
2015-09-20 10:39                   ` Arnout Vandecappelle
2015-09-20 11:16                     ` Thomas Petazzoni
2015-09-20 11:35                       ` Baruch Siach

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.