Patchwork [1/1] sysvinit: start .sh scripts correctly

login
register
mail settings
Submitter Qi.Chen@windriver.com
Date Feb. 26, 2013, 1:39 a.m.
Message ID <cdf763e0896938a5189aa1694b6769f5b6d7190b.1361783630.git.Qi.Chen@windriver.com>
Download mbox | patch
Permalink /patch/45067/
State New
Headers show

Comments

Qi.Chen@windriver.com - Feb. 26, 2013, 1:39 a.m.
From: Chen Qi <Qi.Chen@windriver.com>

Previously, scripts which end with '.sh' were sourced, so the arguments
like 'start' and 'stop' were just ignored.

This resulted in some init scripts not being able to start correctly.
For example, sourcing hwclock.sh in busybox actually does nothing.
It should be invoked as 'hwclock.sh start' or 'hwclock.sh stop'.

This patch fixes this issue.

[YOCTO #3612]

Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
---
 meta/recipes-core/sysvinit/sysvinit/rc |   16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)
Saul Wold - Feb. 26, 2013, 2 a.m.
On 02/25/2013 05:39 PM, Qi.Chen@windriver.com wrote:
> From: Chen Qi <Qi.Chen@windriver.com>
>
> Previously, scripts which end with '.sh' were sourced, so the arguments
> like 'start' and 'stop' were just ignored.
>
> This resulted in some init scripts not being able to start correctly.
> For example, sourcing hwclock.sh in busybox actually does nothing.
> It should be invoked as 'hwclock.sh start' or 'hwclock.sh stop'.
>
> This patch fixes this issue.
>
> [YOCTO #3612]
>
> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
> ---
>   meta/recipes-core/sysvinit/sysvinit/rc |   16 +---------------
>   1 file changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/meta/recipes-core/sysvinit/sysvinit/rc b/meta/recipes-core/sysvinit/sysvinit/rc
> index 44bc9bf..50951da 100755
> --- a/meta/recipes-core/sysvinit/sysvinit/rc
> +++ b/meta/recipes-core/sysvinit/sysvinit/rc
> @@ -41,21 +41,7 @@ startup_progress() {
>   startup() {
>     # Handle verbosity
>     [ "$VERBOSE" = very ] && echo "INIT: Running $@..."
> -
> -  case "$1" in
> -	*.sh)
> -		# Source shell script for speed.
> -		(
> -			trap - INT QUIT TSTP

Are you sure you don't want the trap still?  I realize this reset it to 
default..

Sau!

> -			scriptname=$1
> -			shift
> -			. $scriptname
> -		)
> -		;;
> -	*)
> -		"$@"
> -		;;
> -  esac
> +  "$@"
>     startup_progress
>   }
>
>
Qi.Chen@windriver.com - Feb. 26, 2013, 2:45 a.m.
On 02/26/2013 10:00 AM, Saul Wold wrote:
> On 02/25/2013 05:39 PM, Qi.Chen@windriver.com wrote:
>> From: Chen Qi <Qi.Chen@windriver.com>
>>
>> Previously, scripts which end with '.sh' were sourced, so the arguments
>> like 'start' and 'stop' were just ignored.
>>
>> This resulted in some init scripts not being able to start correctly.
>> For example, sourcing hwclock.sh in busybox actually does nothing.
>> It should be invoked as 'hwclock.sh start' or 'hwclock.sh stop'.
>>
>> This patch fixes this issue.
>>
>> [YOCTO #3612]
>>
>> Signed-off-by: Chen Qi <Qi.Chen@windriver.com>
>> ---
>>   meta/recipes-core/sysvinit/sysvinit/rc |   16 +---------------
>>   1 file changed, 1 insertion(+), 15 deletions(-)
>>
>> diff --git a/meta/recipes-core/sysvinit/sysvinit/rc 
>> b/meta/recipes-core/sysvinit/sysvinit/rc
>> index 44bc9bf..50951da 100755
>> --- a/meta/recipes-core/sysvinit/sysvinit/rc
>> +++ b/meta/recipes-core/sysvinit/sysvinit/rc
>> @@ -41,21 +41,7 @@ startup_progress() {
>>   startup() {
>>     # Handle verbosity
>>     [ "$VERBOSE" = very ] && echo "INIT: Running $@..."
>> -
>> -  case "$1" in
>> -    *.sh)
>> -        # Source shell script for speed.
>> -        (
>> -            trap - INT QUIT TSTP
>
> Are you sure you don't want the trap still?  I realize this reset it 
> to default..
>
Yes.

Code snippet from rc:
   # Ignore CTRL-C only in this shell, so we can interrupt subprocesses.
   trap ":" INT QUIT TSTP

So these signals are ignored in rc but passed on to subprocesses.
And the reset is not needed.

If in rc we use
     trap "" INIT QUIT TSTP
then the reset is needed.

Best Regards,
Chen Qi

> Sau!
>
>> -            scriptname=$1
>> -            shift
>> -            . $scriptname
>> -        )
>> -        ;;
>> -    *)
>> -        "$@"
>> -        ;;
>> -  esac
>> +  "$@"
>>     startup_progress
>>   }
>>
>>
>
>
Ross Burton - Feb. 26, 2013, 11:04 a.m.
On 26 February 2013 01:39,  <Qi.Chen@windriver.com> wrote:
> -
> -  case "$1" in
> -       *.sh)
> -               # Source shell script for speed.
> -               (
> -                       trap - INT QUIT TSTP
> -                       scriptname=$1
> -                       shift
> -                       . $scriptname
> -               )
> -               ;;
> -       *)
> -               "$@"
> -               ;;
> -  esac
> +  "$@"
>    startup_progress
>  }

NACK.

By "fix" you mean "remove the tested and proven optimisation"? The "if
.sh use ." test was designed to speed up booting by not forking a new
bash, and it's been demonstrated to have a noticeable difference on
slower hardware.

You can pass arguments to "." as this little test demonstrates:

$ cat service.sh
echo My arguments are "$@"
$ . service.sh foo bar
My arguments are foo bar

The "shift" command shows that passing the arguments to the script was
the intention, and a few lines of micro-test demonstrate that it
*should* work:

$ cat rc.sh
startup() {
    scriptname=$1
    shift
    . $scriptname
}
startup ./service.sh start
$ busybox sh ./rc.sh
My arguments are start

So, something else is going wrong.

Ross
Qi.Chen@windriver.com - Feb. 27, 2013, 3:26 a.m.
On 02/26/2013 07:04 PM, Burton, Ross wrote:
> On 26 February 2013 01:39,  <Qi.Chen@windriver.com> wrote:
>> -
>> -  case "$1" in
>> -       *.sh)
>> -               # Source shell script for speed.
>> -               (
>> -                       trap - INT QUIT TSTP
>> -                       scriptname=$1
>> -                       shift
>> -                       . $scriptname
>> -               )
>> -               ;;
>> -       *)
>> -               "$@"
>> -               ;;
>> -  esac
>> +  "$@"
>>     startup_progress
>>   }
> NACK.
>
> By "fix" you mean "remove the tested and proven optimisation"? The "if
> .sh use ." test was designed to speed up booting by not forking a new
> bash, and it's been demonstrated to have a noticeable difference on
> slower hardware.
>
> You can pass arguments to "." as this little test demonstrates:
>
> $ cat service.sh
> echo My arguments are "$@"
> $ . service.sh foo bar
> My arguments are foo bar
>
> The "shift" command shows that passing the arguments to the script was
> the intention, and a few lines of micro-test demonstrate that it
> *should* work:
>
> $ cat rc.sh
> startup() {
>      scriptname=$1
>      shift
>      . $scriptname
> }
> startup ./service.sh start
> $ busybox sh ./rc.sh
> My arguments are start
>
> So, something else is going wrong.
>
> Ross
>
>
Yes, you're right.
I absolutely made a mistake.

Thank you for pointing it out.

Best Regards,
Chen Qi

Patch

diff --git a/meta/recipes-core/sysvinit/sysvinit/rc b/meta/recipes-core/sysvinit/sysvinit/rc
index 44bc9bf..50951da 100755
--- a/meta/recipes-core/sysvinit/sysvinit/rc
+++ b/meta/recipes-core/sysvinit/sysvinit/rc
@@ -41,21 +41,7 @@  startup_progress() {
 startup() {
   # Handle verbosity
   [ "$VERBOSE" = very ] && echo "INIT: Running $@..."
-
-  case "$1" in
-	*.sh)
-		# Source shell script for speed.
-		(
-			trap - INT QUIT TSTP
-			scriptname=$1
-			shift
-			. $scriptname
-		)
-		;;
-	*)
-		"$@"
-		;;
-  esac
+  "$@"
   startup_progress
 }