[RFC] dim: Add warncheck subcommand

[Arkadiusz Hiler]

On Thu, May 24, 2018 at 12:18:43PM +0300, Jani Nikula wrote:
> On Thu, 24 May 2018, Arkadiusz Hiler <arkadiusz.hiler at intel.com> wrote:
> > This subcommand runs 'make W=1' on the provided range.
> >
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler at intel.com>
> > ---
> >  dim     | 38 ++++++++++++++++++++++++++++++++++++++
> >  dim.rst |  7 +++++++
> >  2 files changed, 45 insertions(+)
> >
> > diff --git a/dim b/dim
> > index ed26033f5aba..988e83e691c6 100755
> > --- a/dim
> > +++ b/dim
> > @@ -1506,6 +1506,44 @@ function dim_sparse
> >  	return $rv
> >  }
> >  
> > +function dim_warncheck
> > +{
> > +    _restore_head_on_exit
> 
> Please use tabs for indentation throughout.

Yeah, sorry, different machine and the lack of my locak .vimrc.local

> > +
> > +	local range make_output make_rc commits
> > +
> > +	range=$(rangeish "${1:-}")
> > +    commits=( $(git rev-list --reverse $range) )
> > +
> > +    git checkout --detach  ${commits[0]}~ > /dev/null 2>&1
> > +
> > +    if ! make -j$(nproc) W=1 > /dev/null 2>&1; then
> > +        echo "The base for the provided range does not build cleanly."
> > +        echo "Commit: $(git log -n1 --format='%s' HEAD)"
> > +        echo "Bailing out to cut on the noise."
> > +        return 1
> > +    fi
> > +
> > +	for commit in "${commits[@]}"; do
> 
> How about doing
> 
> 	baseline="${commits[0]}~"
> 
> 	for commit in $baseline "${commits[@]}"; do
> 
> and handling the baseline in the same loop? In the error case, you can
> check if [[ "$commit" = "$baseline" ]] and output a different error
> message.

I thought about that, but I want to commit the baseline with -j$(nproc)
to save on time, and that adds even more branching. I decided against
that, as this seems easier to follow.

> > +        git checkout --detach $commit >/dev/null 2>&1
> > +
> > +        set +e
> > +        make_output="$(make W=1 2>&1)"
> > +        make_rc=$?
> > +        set -e
> > +
> > +        if [ "$make_rc" -ne 0 ]; then
> > +            echo "Commit: $(git log -n1 --format='%s' $commit)"
> 
> We have dim_cite function to pretty format commits.

k

> > +            echo "$make_output"
> 
> Please use echoerr for error messages throughout.

k

> > +
> > +            # error will probably propagate, let's cut the noise
> > +            echo
> 
> I'm not fond of superfluous whitespace in output.

Question of taste tbh. I really do not like the output without that
visual separation of the make output and summary. But whatever, I'll get
rid of it in v2.

> > +            echo "Quitting early. Please fix and check the other commits as well."
> > +            return 1
> > +        fi
> > +	done
> > +}
> > +
> >  function dim_checker
> >  {
> >  	rm -f drivers/gpu/drm/i915/*.o drivers/gpu/drm/i915/*.ko
> > diff --git a/dim.rst b/dim.rst
> > index e2c5fd6e6d0a..c73af992243f 100644
> > --- a/dim.rst
> > +++ b/dim.rst
> > @@ -144,6 +144,13 @@ Run sparse on the files changed by the given commit range.
> >  If no commit-ish is passed, defaults to HEAD^..HEAD. If one commit-ish is passed
> >  instead of a range, the range commit-ish..HEAD is used.
> >  
> > +warncheck [*commit-ish* [.. *commit-ish*]]
> > +---------------------------------------
> 
> Doesn't make check complain about this?

What do you mean by 'make check'?

> Underline should be the same  length as the heading.

k

-- 
Cheers,
Arek