Re: [PATCH v4 1/2] scripts: Support compiled source, improved precise

[Masahiro Yamada <masahiroy@kernel.org>]

On Sat, May 2, 2020 at 2:27 PM xujialu <xujialu@vimux.org> wrote:
>
> Original 'COMPILED_SOURCE=1 make cscope' collects nearly 30000 files
> include too many unused files, this patch precisely collects source
> files from *.cmd, in this case just 3000 files include dts and dtsi.
>
> Usage:
>   1) COMPILED_SOURCE=1                           make {cscope,gtags}
>   2) COMPILED_SOURCE=1 KBUILD_ABS_SRCTREE=1      make {cscope,gtags}
>   3) COMPILED_SOURCE=1              ./scripts/tags.sh {cscope,gtags}
>   4) COMPILED_SOURCE=1 ABSPWD=$PWD/ ./scripts/tags.sh {cscope,gtags}
>
> Signed-off-by: xujialu <xujialu@vimux.org>
> ---
>  scripts/tags.sh | 36 ++++++++++++++++++++++--------------
>  1 file changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/scripts/tags.sh b/scripts/tags.sh
> index 4e18ae5282a6..941a5c61d343 100755
> --- a/scripts/tags.sh
> +++ b/scripts/tags.sh
> @@ -89,22 +89,30 @@ all_sources()
>         find_other_sources '*.[chS]'
>  }
>
> +# COMPILED_SOURCE=1                           make {cscope,gtags}
> +# COMPILED_SOURCE=1 KBUILD_ABS_SRCTREE=1      make {cscope,gtags}
> +# COMPILED_SOURCE=1              ./scripts/tags.sh {cscope,gtags}
> +# COMPILED_SOURCE=1 ABSPWD=$PWD/ ./scripts/tags.sh {cscope,gtags}


These comment are misleading since this sounds like
is is only usef for cscope, gtags.


Please do not introduce a new variable ABSPWD, which is unneeded.
This is a rare use-case, but if you want to run this script directly,
you must set the variables described at line 9 properly.




> +xtags_juggle_list()
> +{
> +       SRCTREE=$(realpath ${tree}.)
> +
> +       cd $(dirname $(find -name .config -print -quit).)


Why is this needed?

You are already in objtree
when this script is being run.

If you handle the objects built with O= option,
you need to do 'make O=... gtags'.

> +
> +       realpath -e --relative-to=${SRCTREE} $(find -name "*.cmd" -exec \
> +               grep -Poh '(?(?=^source_.* \K).*|(?=^  \K\S).*(?= \\))' {} \+ |
> +               awk '!a[$0]++') include/generated/autoconf.h |
> +       sed -e "/\.\./d" -e "s,^,${ABSPWD}${tree},"
> +}

Why is --relative-to=${SRCTREE} needed?

You are dropping ${SRCTREE} and adding ${ABSPWD}${tree}.
I do not understand what this is doing back-and-forth.



Lastly, the file order is currently carefully crafted
but this patch would make it random-ordered.

I am afraid the following commit would be broken.




commit f81b1be40c44b33b9706d64c117edd29e627ad12 (HEAD)
Author: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Date:   Mon Feb 8 00:25:59 2010 +0100

    tags: include headers before source files

    Currently looking up a structure definition in TAGS / tags takes one to
    one of multiple "static struct X" definitions in arch sources, which makes
    it for many structs practically impossible to get to the required header.
    This patch changes the order of sources being tagged to first scan
    architecture includes, then the top-level include/ directory, and only
    then the rest. It also takes into account, that many architectures have
    more than one include directory, i.e., not only arch/$ARCH/include, but
    also arch/$ARCH/mach-X/include etc.

    Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
    Reviewed-by: WANG Cong <xiyou.wangcong@gmail.com>
    [mmarek@suse.cz: fix 'var+=text' bashism]
    Signed-off-by: Michal Marek <mmarek@suse.cz>

> +
>  all_compiled_sources()
>  {
> -       for i in $(all_sources); do
> -               case "$i" in
> -                       *.[cS])
> -                               j=${i/\.[cS]/\.o}
> -                               j="${j#$tree}"
> -                               if [ -e $j ]; then
> -                                       echo $i
> -                               fi
> -                               ;;
> -                       *)
> -                               echo $i
> -                               ;;
> -               esac
> -       done
> +       # Consider 'git ls-files' features:
> +       #   1) sort and uniq target files
> +       #   2) limit target files by index
> +       # git ls-files $(xtags_juggle_list)


How is this related to this ?






> +
> +       xtags_juggle_list | sort -u
>  }
>
>  all_target_sources()
> --
> 2.20.1
>


--
Best Regards
Masahiro Yamada