[PATCH] scripts/tags.sh: don't rely on parsing `ls` for $ALLSOURCE_ARCHS generation

[PATCH] scripts/tags.sh: don't rely on parsing `ls` for $ALLSOURCE_ARCHS generation

[Joey Pabalinas]

[-- Attachment #1: Type: text/plain, Size: 990 bytes --]

Parsing `ls` is fragile at best and _will_ fail when $tree
contains spaces. Replace this with a glob-generated string
and directly assign it to $ALLSOURCE_ARCHS; use a subshell
so `cd` doesn't affect the current working directory.

Signed-off-by: Joey Pabalinas <joeypabalinas@gmail.com>

 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/scripts/tags.sh b/scripts/tags.sh
index 78e546ff689c2d5f40..b84acf8889fe836c60 100755
--- a/scripts/tags.sh
+++ b/scripts/tags.sh
@@ -29,14 +29,11 @@ fi
 ignore="$ignore ( -path ${tree}tools ) -prune -o"
 
 # Find all available archs
 find_all_archs()
 {
-	ALLSOURCE_ARCHS=""
-	for arch in `ls ${tree}arch`; do
-		ALLSOURCE_ARCHS="${ALLSOURCE_ARCHS} "${arch##\/}
-	done
+	ALLSOURCE_ARCHS="$( (cd "${tree}arch/" && echo *) )"
 }
 
 # Detect if ALLSOURCE_ARCHS is set. If not, we assume SRCARCH
 if [ "${ALLSOURCE_ARCHS}" = "" ]; then
 	ALLSOURCE_ARCHS=${SRCARCH}
-- 
2.17.0.rc1.35.g90bbd502d54fe92035.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] scripts/tags.sh: don't rely on parsing `ls` for $ALLSOURCE_ARCHS generation

[Joey Pabalinas]

[-- Attachment #1: Type: text/plain, Size: 301 bytes --]

On Tue, May 15, 2018 at 02:13:11PM -1000, Joey Pabalinas wrote:
> and directly assign it to $ALLSOURCE_ARCHS; use a subshell
> so `cd` doesn't affect the current working directory.

Whoops, turns out the inner `()` isn't needed, so going to
revise and send a v2.

-- 
Cheers,
Joey Pabalinas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] scripts/tags.sh: don't rely on parsing `ls` for $ALLSOURCE_ARCHS generation

[Masahiro Yamada]

2018-05-16 9:13 GMT+09:00 Joey Pabalinas <joeypabalinas@gmail.com>:
> Parsing `ls` is fragile at best and _will_ fail when $tree
> contains spaces. Replace this with a glob-generated string
> and directly assign it to $ALLSOURCE_ARCHS; use a subshell
> so `cd` doesn't affect the current working directory.
>
> Signed-off-by: Joey Pabalinas <joeypabalinas@gmail.com>
>
>  1 file changed, 1 insertion(+), 4 deletions(-)


Andrew picked it up, but this patch is *bad*

You missed arch/Kconfig.

$(cd "${tree}arch/" && echo *)
contains Kconfig, but it is not arch.





> diff --git a/scripts/tags.sh b/scripts/tags.sh
> index 78e546ff689c2d5f40..b84acf8889fe836c60 100755
> --- a/scripts/tags.sh
> +++ b/scripts/tags.sh
> @@ -29,14 +29,11 @@ fi
>  ignore="$ignore ( -path ${tree}tools ) -prune -o"
>
>  # Find all available archs
>  find_all_archs()
>  {
> -       ALLSOURCE_ARCHS=""
> -       for arch in `ls ${tree}arch`; do
> -               ALLSOURCE_ARCHS="${ALLSOURCE_ARCHS} "${arch##\/}
> -       done
> +       ALLSOURCE_ARCHS="$( (cd "${tree}arch/" && echo *) )"
>  }
>
>  # Detect if ALLSOURCE_ARCHS is set. If not, we assume SRCARCH
>  if [ "${ALLSOURCE_ARCHS}" = "" ]; then
>         ALLSOURCE_ARCHS=${SRCARCH}
> --
> 2.17.0.rc1.35.g90bbd502d54fe92035.dirty
>



-- 
Best Regards
Masahiro Yamada

Re: [PATCH] scripts/tags.sh: don't rely on parsing `ls` for $ALLSOURCE_ARCHS generation

[Joey Pabalinas]

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

On Fri, May 18, 2018 at 02:46:32PM +0900, Masahiro Yamada wrote:
> Andrew picked it up, but this patch is *bad*
> 
> You missed arch/Kconfig.
> 
> $(cd "${tree}arch/" && echo *)
> contains Kconfig, but it is not arch.

That was also something that I found a bit weird myself, but I had
assumed there was a good reason for keeping that. The original
command also returns a string containing Kconfig:

>  tree="$PWD/"
>  echo "$tree"
>  ALLSOURCE_ARCHS=""
>  for arch in `ls ${tree}arch`; do
>          ALLSOURCE_ARCHS="${ALLSOURCE_ARCHS} "${arch##\/}
>  done
>  echo "$ALLSOURCE_ARCHS"'

gives the same output as my command (albeit with an extra leading
space that shouldn't be important):

>  /store/code/projects/kernel/linux/
>   Kconfig alpha arc arm arm64 c6x h8300 hexagon ia64 m68k microblaze mips nds32 nios2 openrisc parisc powerpc riscv s390 sh sparc um unicore32 x86 xtensa

However, if there really is no reason for that being there, I
have no complaints against fixing it. I'll send a v3 in a bit.

-- 
Cheers,
Joey Pabalinas

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]