[PATCH 2/2] dim: Make dim sparse check each commit in the range-ish

[Arkadiusz Hiler]

On Mon, Nov 27, 2017 at 04:39:57PM +0200, Arkadiusz Hiler wrote:
> Previously 'dim sparse' just touched all the files affected by the range
> and run sparse against that. Since this may show issues from included
> headers and sparse-compliance does not seem to be strongly enforced the
> output is rather unhelpful.
> 
> With this change the repo is checkout at each commit, sparse is run on
> the whole drivers/gpu/drm/ directory for each step.
> 
> Each result is compared to it's parent's result using remap-log to
> adjust for potential line numbering changes.
> 
> Only the difference is reported.
> 
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> ---
>  dim | 40 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/dim b/dim
> index ee958be231ab..c586883363e8 100755
> --- a/dim
> +++ b/dim
> @@ -1398,6 +1398,13 @@ function checkpatch_commit
>  	return $rv
>  }
>  
> +# $1 is the git sha1 to check
> +function sparse_commit
> +{
> +    git checkout --detach $1 >/dev/null 2>&1
> +    make C=2 -j$(nproc) drivers/gpu/drm/ 2>&1 1>/dev/null | sort
> +}

<ivyl> danvet: also do we want to have the sparse checks just for i915.ko,
       whole i915/ or whole drm/ ?
<danvet> ivyl, all that's changed?
<danvet> for dim sparse $commit-ish
<danvet> well $commit|range-ish

The sequence would look something like that:

------------------------------------------------------------
git checkout ${commits[0]}~
make

for commit in commits:
    # get reference from parent just for the changed files
    touch --no-create $(git diff --name-only $commit)
    prev_sr="$(make C=1)"

    # now get the state after the commit is applied
    git checkout $commit
    sr="$(make C=1)"

    # and compare
    prev_remapped="$(remap_log ... prev_sr)"
    diff <(echo $prev_remapped) <(echo $sr)
------------------------------------------------------------

For N patches that means 2*N spare runs, while the current
implementation (seen below) is N+1 but checks the whole drm dir.

It does not look that bad, but I have to check how much longer does it
take.

BTW, excerpt from the kernel's Docs:
Do a kernel make with "make C=1" to run sparse on all the C files that
get recompiled, or use "make C=2" to run sparse on the files whether
they need to be recompiled or not.  The latter is a fast way to check
the whole tree if you have already built it.

>  # turn $1 in to a git commit range
>  function rangeish()
>  {
> @@ -1470,13 +1477,38 @@ function dim_checkpatch
>  
>  function dim_sparse
>  {
> -	local range
> +	local range rv sr prev_sr prev_remapped diff_result remap_log
>  
>  	range=$(rangeish "${1:-}")
> +    remap_log=$DIM_PREFIX/maintainer-tools/remap-log
>  
> -	make $DIM_MAKE_OPTIONS
> -	touch --no-create $(git diff --name-only $range) $(git diff --name-only)
> -	make C=1
> +    if [ ! -x  $remap_log ]; then
> +        echo "$remap_log is not an executable, please run make in maintainer-tools dir!"
> +        exit 1
> +    fi

https://github.com/zsh-users/zsh/blob/master/Functions/VCS_Info/Backends/VCS_INFO_get_data_git#L73
We can use something like that to get the current ref here (whether it
is a branch or detached head or anything else).

trap "git checkout $original_ref" 0

to restore the HEAD to it's original state after we are done here.

> +	for commit in $(git rev-list --reverse $range); do
> +        if [ -z "$prev_sr" ]; then prev_sr="$(sparse_commit $commit~)"; fi
> +        sr="$(sparse_commit $commit)"
> +        prev_remapped="$(echo "$prev_sr" | $remap_log <(git diff HEAD~ | $remap_log))"
> +        diff_result="$(diff -u <(echo "$prev_remapped") <(echo "$sr") || true)"
> +
> +        echo "Commit: $(git log -n1 --format='%s' $commit)"
> +        if [ -n "$diff_result" ]; then
> +            echo "$diff_result" | egrep '^[+-]' | egrep -v '^[+-]{3}'
> +        else
> +            echo "Okay!"
> +        fi
> +        echo
> +
> +        if (echo "$diff_result" | grep -q '^+'); then
> +            rv=1
> +        fi
> +
> +        prev_sr="$sr"
> +	done
> +
> +	return $rv
>  }
>  
>  function dim_checker
> -- 
> 2.13.6
> 
> _______________________________________________
> dim-tools mailing list
> dim-tools at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dim-tools