Re: [PATCH] scripts/tags.sh: don't rely on parsing `ls` for $ALLSOURCE_ARCHS generation
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
signature.asc
Description: PGP signature
Re: [PATCH] scripts/tags.sh: don't rely on parsing `ls` for $ALLSOURCE_ARCHS generation
2018-05-16 9:13 GMT+09:00 Joey Pabalinas :
> 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
>
> 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
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 signature.asc Description: PGP signature
