Re: [OE-core] [PATCH] bash2dash conversion

2016-09-22 Thread He Zhe 


On 09/19/2016 08:07 PM, Burton, Ross wrote:
>
> On 19 September 2016 at 12:57, Peter Kjellerstedt 
> >wrote:
>
> > ++mem_exists="`grep 
> "^$groupname:[!:]*:[!:]*:\([!,]*,\)*$username\(,[!,]*\)*"$rootdir/etc/group 
> || true`"
> >   if test "x$mem_exists" = "x"; then
> >   bbfatal "${PN}: groupmems command did not 
> succeed."
> >   fi
>
> The above change cannot be correct. Changing the expression "[^:]"
> (which means "anything but :") to "[!:]" (which means "! or :") is
> definitely not the same
>
>
> My prediction is that this series is generated by running checkbashism over 
> the fragments, it fires a false-positive here (from memory because it thinks 
> it is processing a bash glob or something, not grep regex).
>
> Note that I just submitted my verify-bashism script which has a whitelist for 
> things - such as use of command - which whilst not in POSIX are in fact in 
> bash/dash/ash so for all intents and purposes will work.
>

Yes, I'm using checkbashism and have found some false-positive reports but 
missed this one. Thank you for pointing out.

Zhe

> Ross

-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [OE-core] [PATCH] bash2dash conversion

2016-09-22 Thread He Zhe 


On 09/19/2016 07:57 PM, Peter Kjellerstedt wrote:
>> -Original Message-
>> From: openembedded-core-boun...@lists.openembedded.org
>> [mailto:openembedded-core-boun...@lists.openembedded.org] On Behalf Of
>> zhe...@windriver.com
>> Sent: den 18 september 2016 04:52
>> To: openembedded-core@lists.openembedded.org
>> Subject: [OE-core] [PATCH] bash2dash conversion
>  ^
> Typically this is referred to as "Remove bashisms"...

Yes.

>
>> From: "Tim K. Chan" 
>>
>> Change bash style to dash style
>>
>> Signed-off-by: Tim K. Chan 
>> [Adjust context]
>> Signed-off-by: He Zhe 
>> ---
>>  meta/classes/systemd.bbclass|  4 ++--
>>  meta/classes/update-rc.d.bbclass|  6 +++---
>>  meta/classes/useradd_base.bbclass   |  4 ++--
>>  meta/recipes-connectivity/resolvconf/resolvconf_1.79.bb |  2 +-
>>  meta/recipes-core/glibc/glibc-package.inc   |  2 +-
>>  meta/recipes-devtools/guile/guile_2.0.12.bb |  6 --
>>  meta/recipes-devtools/rpm/rpm_5.4.16.bb | 13 +++--
>>  7 files changed, 24 insertions(+), 13 deletions(-)
> [cut]
>
>> diff --git a/meta/classes/useradd_base.bbclass
>> b/meta/classes/useradd_base.bbclass
>> index f4dc713..35d5f06 100644
>> --- a/meta/classes/useradd_base.bbclass
>> +++ b/meta/classes/useradd_base.bbclass
>> @@ -59,10 +59,10 @@ perform_groupmems () {
>>  gshadow="no"
>>  touch $rootdir${sysconfdir}/gshadow
>>  fi
>> -local mem_exists="`grep 
>> "^$groupname:[^:]*:[^:]*:\([^,]*,\)*$username\(,[^,]*\)*"$rootdir/etc/group 
>> || true`"
>> +local mem_exists="`grep 
>> "^$groupname:[!:]*:[!:]*:\([!,]*,\)*$username\(,[!,]*\)*"$rootdir/etc/group 
>> || true`"
>>  if test "x$mem_exists" = "x"; then
>>  eval flock -x $rootdir${sysconfdir} -c \"$PSEUDO groupmems 
>> \$opts\" || true
>> -mem_exists="`grep 
>> "^$groupname:[^:]*:[^:]*:\([^,]*,\)*$username\(,[^,]*\)*"$rootdir/etc/group 
>> || true`"
>> ++   mem_exists="`grep 
>> "^$groupname:[!:]*:[!:]*:\([!,]*,\)*$username\(,[!,]*\)*"$rootdir/etc/group 
>> || true`"
>>  if test "x$mem_exists" = "x"; then
>>  bbfatal "${PN}: groupmems command did not succeed."
>>  fi
> The above change cannot be correct. Changing the expression "[^:]" 
> (which means "anything but :") to "[!:]" (which means "! or :") is 
> definitely not the same

Thanks for careful review. I really missed correcting thisfalse-positive report.

>> diff --git a/meta/recipes-devtools/guile/guile_2.0.12.bb
>> b/meta/recipes-devtools/guile/guile_2.0.12.bb
>> index d2fe511..3ada0f1 100644
>> --- a/meta/recipes-devtools/guile/guile_2.0.12.bb
>> +++ b/meta/recipes-devtools/guile/guile_2.0.12.bb
>> @@ -91,8 +91,10 @@ guile_cross_config() {
>>  then
>>  # Create guile-config returning target values instead of native 
>> values
>>  install -d ${SYSROOT_DESTDIR}${STAGING_BINDIR_CROSS}
>> -echo '#!'`which ${BUILD_SYS}-guile`$' \\\n--no-auto-compile -e 
>> main -s\n!#\n(define %guile-build-info '\'\( \
>> -> ${B}/guile-config.cross
>> +echo -n '#!' > ${B}/guile-config.cross
>> +echo -n `which x86_64-linux-guile` >> ${B}/guile-config.cross
>> +printf ' \\\n--no-auto-compile -e main -s\n!#\n(define 
>> %%guile-build-info ' >> ${B}/guile-config.cross
>> +echo "'(" >> ${B}/guile-config.cross
>>  sed -n -e 's:^[ \t]*{[ \t]*":  (:' \
>>  -e 's:",[ \t]*": . ":' \
>>  -e 's:" *}, *\\:"):' \
> I suggest to replace the entire guile_cross_config() function with this:
>
> guile_cross_config() {
>   # this is only for target recipe
>   [ "${PN}" = "${BPN}" ] || return 0
>
>   vars=$(sed -n -e 's:^[ \t]*{[ \t]*":  (:' \
> -e 's:",[ \t]*": . ":' \
> -e 's:" *}, *\\:"):' \
> -e 's:^.*cachedir.*$::' \
> -e '/^  (/p' \
>  < ${B}/libguile/libpath.h)
>
>   # Create guile-config returning target values instead of native values
>   install -d ${SYSROOT_DESTDIR}${bindir_crossscripts}
>   cat <${B}/guile-config.cross
> #!$(which ${BUILD_SYS}-guile) \\
> --no-auto-compile -e main -s
> !#
> (define %guile-build-info '(
> $vars
> ))
> EOF
>   cat ${B}/meta/guile-config >> ${B}/guile-config.cross
>   install ${B}/guile-config.cross 
> ${SYSROOT_DESTDIR}${bindir_crossscripts}/guile-config
> }
>
> That should make it a lot simpler to see what is going on.
> Please note that I have corrected the installation path as well.

Definitely simpler and clearer. I'll test and send v2.

Thanks,
Zhe

> //Peter
>
>

-- 
___
Openembedded-core mailing list

Re: [OE-core] [PATCH] bash2dash conversion

2016-09-19 Thread Burton, Ross 
On 19 September 2016 at 12:57, Peter Kjellerstedt <
peter.kjellerst...@axis.com> wrote:

> > ++mem_exists="`grep "^$groupname:[!:]*:[!:]*:\([!,
> ]*,\)*$username\(,[!,]*\)*"$rootdir/etc/group || true`"
> >   if test "x$mem_exists" = "x"; then
> >   bbfatal "${PN}: groupmems command did not succeed."
> >   fi
>
> The above change cannot be correct. Changing the expression "[^:]"
> (which means "anything but :") to "[!:]" (which means "! or :") is
> definitely not the same


My prediction is that this series is generated by running checkbashism over
the fragments, it fires a false-positive here (from memory because it
thinks it is processing a bash glob or something, not grep regex).

Note that I just submitted my verify-bashism script which has a whitelist
for things - such as use of command - which whilst not in POSIX are in fact
in bash/dash/ash so for all intents and purposes will work.

Ross
-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [OE-core] [PATCH] bash2dash conversion

2016-09-19 Thread Peter Kjellerstedt 
> -Original Message-
> From: openembedded-core-boun...@lists.openembedded.org
> [mailto:openembedded-core-boun...@lists.openembedded.org] On Behalf Of
> zhe...@windriver.com
> Sent: den 18 september 2016 04:52
> To: openembedded-core@lists.openembedded.org
> Subject: [OE-core] [PATCH] bash2dash conversion
 ^
Typically this is referred to as "Remove bashisms"...

> From: "Tim K. Chan" 
> 
> Change bash style to dash style
> 
> Signed-off-by: Tim K. Chan 
> [Adjust context]
> Signed-off-by: He Zhe 
> ---
>  meta/classes/systemd.bbclass|  4 ++--
>  meta/classes/update-rc.d.bbclass|  6 +++---
>  meta/classes/useradd_base.bbclass   |  4 ++--
>  meta/recipes-connectivity/resolvconf/resolvconf_1.79.bb |  2 +-
>  meta/recipes-core/glibc/glibc-package.inc   |  2 +-
>  meta/recipes-devtools/guile/guile_2.0.12.bb |  6 --
>  meta/recipes-devtools/rpm/rpm_5.4.16.bb | 13 +++--
>  7 files changed, 24 insertions(+), 13 deletions(-)

[cut]

> diff --git a/meta/classes/useradd_base.bbclass
> b/meta/classes/useradd_base.bbclass
> index f4dc713..35d5f06 100644
> --- a/meta/classes/useradd_base.bbclass
> +++ b/meta/classes/useradd_base.bbclass
> @@ -59,10 +59,10 @@ perform_groupmems () {
>   gshadow="no"
>   touch $rootdir${sysconfdir}/gshadow
>   fi
> - local mem_exists="`grep 
> "^$groupname:[^:]*:[^:]*:\([^,]*,\)*$username\(,[^,]*\)*"$rootdir/etc/group 
> || true`"
> + local mem_exists="`grep 
> "^$groupname:[!:]*:[!:]*:\([!,]*,\)*$username\(,[!,]*\)*"$rootdir/etc/group 
> || true`"
>   if test "x$mem_exists" = "x"; then
>   eval flock -x $rootdir${sysconfdir} -c \"$PSEUDO groupmems 
> \$opts\" || true
> - mem_exists="`grep 
> "^$groupname:[^:]*:[^:]*:\([^,]*,\)*$username\(,[^,]*\)*"$rootdir/etc/group 
> || true`"
> ++mem_exists="`grep 
> "^$groupname:[!:]*:[!:]*:\([!,]*,\)*$username\(,[!,]*\)*"$rootdir/etc/group 
> || true`"
>   if test "x$mem_exists" = "x"; then
>   bbfatal "${PN}: groupmems command did not succeed."
>   fi

The above change cannot be correct. Changing the expression "[^:]" 
(which means "anything but :") to "[!:]" (which means "! or :") is 
definitely not the same

> diff --git a/meta/recipes-devtools/guile/guile_2.0.12.bb
> b/meta/recipes-devtools/guile/guile_2.0.12.bb
> index d2fe511..3ada0f1 100644
> --- a/meta/recipes-devtools/guile/guile_2.0.12.bb
> +++ b/meta/recipes-devtools/guile/guile_2.0.12.bb
> @@ -91,8 +91,10 @@ guile_cross_config() { 
>   then
>   # Create guile-config returning target values instead of native 
> values
>   install -d ${SYSROOT_DESTDIR}${STAGING_BINDIR_CROSS}
> - echo '#!'`which ${BUILD_SYS}-guile`$' \\\n--no-auto-compile -e 
> main -s\n!#\n(define %guile-build-info '\'\( \
> - > ${B}/guile-config.cross
> + echo -n '#!' > ${B}/guile-config.cross
> + echo -n `which x86_64-linux-guile` >> ${B}/guile-config.cross
> + printf ' \\\n--no-auto-compile -e main -s\n!#\n(define 
> %%guile-build-info ' >> ${B}/guile-config.cross
> + echo "'(" >> ${B}/guile-config.cross
>   sed -n -e 's:^[ \t]*{[ \t]*":  (:' \
>   -e 's:",[ \t]*": . ":' \
>   -e 's:" *}, *\\:"):' \

I suggest to replace the entire guile_cross_config() function with this:

guile_cross_config() {
# this is only for target recipe
[ "${PN}" = "${BPN}" ] || return 0

vars=$(sed -n -e 's:^[ \t]*{[ \t]*":  (:' \
  -e 's:",[ \t]*": . ":' \
  -e 's:" *}, *\\:"):' \
  -e 's:^.*cachedir.*$::' \
  -e '/^  (/p' \
   < ${B}/libguile/libpath.h)

# Create guile-config returning target values instead of native values
install -d ${SYSROOT_DESTDIR}${bindir_crossscripts}
cat <${B}/guile-config.cross
#!$(which ${BUILD_SYS}-guile) \\
--no-auto-compile -e main -s
!#
(define %guile-build-info '(
$vars
))
EOF
cat ${B}/meta/guile-config >> ${B}/guile-config.cross
install ${B}/guile-config.cross 
${SYSROOT_DESTDIR}${bindir_crossscripts}/guile-config
}

That should make it a lot simpler to see what is going on.
Please note that I have corrected the installation path as well.

//Peter

-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core