On Thu, May 24, 2018 at 11:18 AM, Jani Nikula <[email protected]> wrote: > On Thu, 24 May 2018, Arkadiusz Hiler <[email protected]> wrote: >> This subcommand runs 'make W=1' on the provided range. >> >> Signed-off-by: Arkadiusz Hiler <[email protected]> >> --- >> 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. > >> + >> + 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. > >> + 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. > >> + echo "$make_output" > > Please use echoerr for error messages throughout. > >> + >> + # error will probably propagate, let's cut the noise >> + echo > > I'm not fond of superfluous whitespace in output. > >> + echo "Quitting early. Please fix and check the other commits as >> well." >> + return 1
This kind of early exit isn't really user friendly. I think you want at least a warn_or_fail so that users can see everything with -d. But even better would be if you do the same I've done for the commit checking when pulling/pushing in commit c0c4dc1c924bd1d94315601026a2f9facd8a7f73 Author: Daniel Vetter <[email protected]> Date: Wed May 2 15:08:24 2018 +0200 dim: check all commits in dim apply-pull and push-branch Cheers, Daniel >> + 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? Underline should be the same > length as the heading. > >> +Runs incremental make W=1 on 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. >> + >> checker >> ------- >> Run sparse on drm/i915. > > -- > Jani Nikula, Intel Open Source Graphics Center > _______________________________________________ > dim-tools mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/dim-tools -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dim-tools mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dim-tools
