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

[Arkadiusz Hiler]

On Mon, Dec 11, 2017 at 10:38:35AM +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
> incrementally, on every file that was changed.
> 
> 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.
> 
> v2:
>     Clearer wording on remap-log check.
>     Use the incremental build.
>     Reset ref on exit.
> 
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> ---
>  dim | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 53 insertions(+), 4 deletions(-)
> 
> diff --git a/dim b/dim
> index ee958be231ab..44b4daa5fabe 100755
> --- a/dim
> +++ b/dim
> @@ -1468,15 +1468,64 @@ function dim_checkpatch
>  	return $rv
>  }
>  
> +function _restore_head_on_exit
> +{
> +    local original_ref
> +
> +    original_ref="$(git rev-parse --abbrev-ref HEAD)"
> +
> +    if [ "$original_ref" == "HEAD" ]; then
> +        original_ref="$(git rev-parse HEAD)"
> +    fi
> +
> +    trap "git checkout -q $original_ref" EXIT
> +}

^ Here we set a trap that resets the tree to the original state.

Daniel suggested using separate worktree instead. I am not very fond of
that approach though.

It would make sense if I had full dim prefix set up, so we can create,
let's say, $DIM_PREFIX/sparse-check-tree, and reuse it.

But we don't want the whole prefix for CI. We just want to run couple of
check in $PWD (many trees), without the setup overhead.

Other way to address that is to provide path to the worktree as a
parameter (ugly) or mktemp -d (bad, you pay penalty of constant
recreation and recompilation of the worktree).

Any idea on how we can juggle the two?

- Arek

> +
>  function dim_sparse
>  {
> -	local range
> +	local range rv sr prev_sr prev_remapped diff_result remap_log commits
>  
>  	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 [ ! -e  $remap_log ]; then
> +        echo "$remap_log is not compailed, please run make in maintainer-tools dir!"
> +        exit 1
> +    fi
> +
> +    _restore_head_on_exit
> +
> +    # make the initial reference build
> +    commits=( $(git rev-list --reverse $range) )
> +    git checkout --detach  ${commits[0]}~ > /dev/null 2>&1
> +    make -j8 drivers/gpu/drm/ > /dev/null 2>&1
> +
> +	for commit in ${commits[@]}; do
> +        touch --no-create $(git diff --name-only $commit~...$commit)
> +        prev_sr="$(make C=1 -j$(nproc) drivers/gpu/drm/ 2>&1 1>/dev/null | sort)"
> +
> +        git checkout --detach $commit >/dev/null 2>&1
> +        sr="$(make C=1 -j$(nproc) drivers/gpu/drm/ 2>&1 1>/dev/null | sort)"
> +
> +        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