Re: [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 14 Mar 2018, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Mar 13, 2018 at 4:07 PM, Jani Nikula <jani.nikula@intel.com> wrote:
>> On Tue, 13 Mar 2018, Daniel Vetter <daniel@ffwll.ch> wrote:
>>> On Tue, Mar 13, 2018 at 01:30:09PM +0200, Jani Nikula wrote:
>>>> To reduce noise on CI checkpatch reports, we want to silence some
>>>> checkpatch warnings. Different branches may end up having different
>>>> rules, and users may want to get unfiltered results, so introduce
>>>> checkpatch profiles. Add some placeholder profiles to be filled later
>>>> on.
>>>>
>>>> The idea is that CI would run 'dim checkpatch HEAD^ drm-intel' (or some
>>>> other commit range-ish as the case may be).
>>>>
>>>> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>>>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>> ---
>>>>  dim     | 46 +++++++++++++++++++++++++++++++++++++++++-----
>>>>  dim.rst |  9 +++++++--
>>>>  2 files changed, 48 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/dim b/dim
>>>> index 81e2bc1511ac..4ba1c7ff490a 100755
>>>> --- a/dim
>>>> +++ b/dim
>>>> @@ -836,7 +836,7 @@ function apply_patch #patch_file
>>>>              rv=1
>>>>      fi
>>>>
>>>> -    if ! checkpatch_commit HEAD; then
>>>> +    if ! checkpatch_commit HEAD branch; then
>>>>              rv=1
>>>>      fi
>>>>      if ! check_maintainer $branch HEAD; then
>>>> @@ -1358,12 +1358,47 @@ function check_maintainer
>>>>  }
>>>>
>>>>  # $1 is the git sha1 to check
>>>> +# $2 is the checkpatch profile
>>>>  function checkpatch_commit
>>>>  {
>>>> -    local commit rv checkpatch_options
>>>> +    local commit rv checkpatch_options profile profile_options
>>>>
>>>>      commit=$1
>>>> -    checkpatch_options="-q --emacs --strict --show-types -"
>>>> +    profile=${2:-default}
>>>> +
>>>> +    # special branch profile maps branches to profiles
>>>> +    if [[ "$profile" = "branch" ]]; then
>>>> +            case "$(git_current_branch)" in
>>>> +                    drm-intel-next-queued|drm-intel-next-fixes|drm-intel-fixes)
>>>> +                            profile=drm-intel
>>>> +                            ;;
>>>> +                    drm-misc-next|drm-misc-next-fixes|drm-misc-fixes)
>>>> +                            profile=drm-misc
>>>> +                            ;;
>>>
>>> Use branch_to_repo instead, if that doesn't come up with anything, then
>>> default?
>>
>> This command is supposed to work without configuration (a developer
>> command)...
>
> Duh :-/
>
> I guess I'm ok as-is, but feels a bit silly.

Thanks, pushed all three. I don't deny it feels a bit silly, but it's
nothing that can't be fixed later.

BR,
Jani.



> -Daniel
>
>>
>> BR,
>> Jani.
>>
>>
>>>
>>> With that little bit of polished applied, on patches 1&2:
>>>
>>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> +                    *)
>>>> +                            profile=default
>>>> +                            ;;
>>>> +            esac
>>>> +    fi
>>>> +
>>>> +    # map profiles to checkpatch options
>>>> +    case "$profile" in
>>>> +            default)
>>>> +                    profile_options=""
>>>> +                    ;;
>>>> +            drm-misc)
>>>> +                    profile_options=""
>>>> +                    ;;
>>>> +            drm-intel)
>>>> +                    profile_options=""
>>>> +                    ;;
>>>> +            *)
>>>> +                    echoerr "Unknown checkpatch profile $profile"
>>>> +                    profile_options=""
>>>> +                    ;;
>>>> +    esac
>>>> +
>>>> +    checkpatch_options="-q --emacs --strict --show-types $profile_options -"
>>>>
>>>>      git --no-pager log --oneline -1 $commit
>>>>      if ! git show --pretty=email $commit |\
>>>> @@ -1430,12 +1465,13 @@ function dim_extract_next_fixes
>>>>  dim_alias_cp=checkpatch
>>>>  function dim_checkpatch
>>>>  {
>>>> -    local range rv
>>>> +    local range profile rv
>>>>
>>>>      range=$(rangeish "${1:-}")
>>>> +    profile=${2:-}
>>>>
>>>>      for commit in $(git rev-list --reverse $range); do
>>>> -            if ! checkpatch_commit $commit; then
>>>> +            if ! checkpatch_commit $commit $profile; then
>>>>                      rv=1
>>>>              fi
>>>>      done
>>>> diff --git a/dim.rst b/dim.rst
>>>> index e2c5fd6e6d0a..cc930959e497 100644
>>>> --- a/dim.rst
>>>> +++ b/dim.rst
>>>> @@ -130,13 +130,18 @@ fixes *commit-ish*
>>>>  Print the Fixes: and Cc: lines for the supplied *commit-ish* in the linux kernel
>>>>  CodingStyle approved format.
>>>>
>>>> -checkpatch [*commit-ish* [.. *commit-ish*]]
>>>> --------------------------------------------
>>>> +checkpatch [*commit-ish* [.. *commit-ish*]] [*profile*]
>>>> +-------------------------------------------------------
>>>>  Runs the given commit range commit-ish..commit-ish through the check tools.
>>>>
>>>>  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.
>>>>
>>>> +If profile is given, uses specific options for checkpatch error
>>>> +filtering. Current profiles are "default", "branch", "drm-intel", and
>>>> +"drm-misc". The "branch" profile maps the current git branch to the appropriate
>>>> +profile, or if the branch is not known, to "default".
>>>> +
>>>>  sparse [*commit-ish* [.. *commit-ish*]]
>>>>  ---------------------------------------
>>>>  Run sparse on the files changed by the given commit range.
>>>> --
>>>> 2.11.0
>>>>
>>>> _______________________________________________
>>>> dim-tools mailing list
>>>> dim-tools@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dim-tools
>>
>> --
>> Jani Nikula, Intel Open Source Technology Center

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx