* [DIM PATCH 1/3] dim: cleanup checkpatch_commit @ 2018-03-13 11:30 Jani Nikula 2018-03-13 11:30 ` [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options Jani Nikula 2018-03-13 11:30 ` [DIM PATCH 3/3] dim: loosen some drm-intel checkpatch rules Jani Nikula 0 siblings, 2 replies; 9+ messages in thread From: Jani Nikula @ 2018-03-13 11:30 UTC (permalink / raw) To: intel-gfx, dim-tools; +Cc: jani.nikula, Rodrigo Vivi Remove some old cruft. Pass checkpatch parameters via a variable. No functional changes. 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 | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dim b/dim index 45aec403a44e..81e2bc1511ac 100755 --- a/dim +++ b/dim @@ -1360,13 +1360,14 @@ function check_maintainer # $1 is the git sha1 to check function checkpatch_commit { - local commit cmd bug_lines non_i915_files rv + local commit rv checkpatch_options commit=$1 - cmd="git show --pretty=email $commit" + checkpatch_options="-q --emacs --strict --show-types -" git --no-pager log --oneline -1 $commit - if ! $cmd | scripts/checkpatch.pl -q --emacs --strict --show-types -; then + if ! git show --pretty=email $commit |\ + scripts/checkpatch.pl $checkpatch_options; then rv=1 fi -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options 2018-03-13 11:30 [DIM PATCH 1/3] dim: cleanup checkpatch_commit Jani Nikula @ 2018-03-13 11:30 ` Jani Nikula 2018-03-13 11:48 ` Daniel Vetter 2018-03-13 11:30 ` [DIM PATCH 3/3] dim: loosen some drm-intel checkpatch rules Jani Nikula 1 sibling, 1 reply; 9+ messages in thread From: Jani Nikula @ 2018-03-13 11:30 UTC (permalink / raw) To: intel-gfx, dim-tools; +Cc: jani.nikula, Rodrigo Vivi 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 + ;; + *) + 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options 2018-03-13 11:30 ` [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options Jani Nikula @ 2018-03-13 11:48 ` Daniel Vetter 2018-03-13 15:07 ` Jani Nikula 0 siblings, 1 reply; 9+ messages in thread From: Daniel Vetter @ 2018-03-13 11:48 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx, dim-tools, Rodrigo Vivi 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? 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 -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options 2018-03-13 11:48 ` Daniel Vetter @ 2018-03-13 15:07 ` Jani Nikula 2018-03-14 6:29 ` Daniel Vetter 0 siblings, 1 reply; 9+ messages in thread From: Jani Nikula @ 2018-03-13 15:07 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx, dim-tools, Rodrigo Vivi 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)... 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options 2018-03-13 15:07 ` Jani Nikula @ 2018-03-14 6:29 ` Daniel Vetter 2018-03-14 8:57 ` Jani Nikula 0 siblings, 1 reply; 9+ messages in thread From: Daniel Vetter @ 2018-03-14 6:29 UTC (permalink / raw) To: Jani Nikula Cc: intel-gfx, DRM maintainer tools announcements, discussion, and development, Rodrigo Vivi 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. -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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options 2018-03-14 6:29 ` Daniel Vetter @ 2018-03-14 8:57 ` Jani Nikula 0 siblings, 0 replies; 9+ messages in thread From: Jani Nikula @ 2018-03-14 8:57 UTC (permalink / raw) To: Daniel Vetter Cc: intel-gfx, DRM maintainer tools announcements, discussion, and development, Rodrigo Vivi 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [DIM PATCH 3/3] dim: loosen some drm-intel checkpatch rules 2018-03-13 11:30 [DIM PATCH 1/3] dim: cleanup checkpatch_commit Jani Nikula 2018-03-13 11:30 ` [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options Jani Nikula @ 2018-03-13 11:30 ` Jani Nikula 2018-03-13 11:55 ` Daniel Vetter 1 sibling, 1 reply; 9+ messages in thread From: Jani Nikula @ 2018-03-13 11:30 UTC (permalink / raw) To: intel-gfx, dim-tools; +Cc: jani.nikula, Rodrigo Vivi Set max line length to 100. I don't want to silence the LONG_LINE warning altogether, and I'd still prefer to keep lines under 80 characters, but I also don't want to see all the noise, and nor do I want to see silly code trying to arbitrarily squeeze under 80 when it doesn't make sense. 100 is a nice arbitrary round number... I hope review catches silly stuff regardless. Fingers crossed. BIT_MACRO. We have (1 << N) all over the place. I hope to switch to BIT() macro eventually, but this documents current use. PREFER_KERNEL_TYPES. We also have uint(8|16|32|64)_t all over the place. I also hope to move towards kernel types, but this documents current use. SPLIT_STRING, LONG_LINE_STRING. Don't nag about strings split to many lines, but also don't nag about strings not split. There's plenty more that could be tweaked, but let's start with something to improve the S/N ratio of the automated CI checkpatch reports. Now that we have --show-types included in the output, we can more easily discuss the ignores on a case-by-case basis. 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 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dim b/dim index 4ba1c7ff490a..9fa6d9cd855b 100755 --- a/dim +++ b/dim @@ -1390,7 +1390,7 @@ function checkpatch_commit profile_options="" ;; drm-intel) - profile_options="" + profile_options="--max-line-length=100 --ignore=BIT_MACRO,PREFER_KERNEL_TYPES,SPLIT_STRING,LONG_LINE_STRING" ;; *) echoerr "Unknown checkpatch profile $profile" -- 2.11.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [DIM PATCH 3/3] dim: loosen some drm-intel checkpatch rules 2018-03-13 11:30 ` [DIM PATCH 3/3] dim: loosen some drm-intel checkpatch rules Jani Nikula @ 2018-03-13 11:55 ` Daniel Vetter 2018-03-14 9:03 ` Jani Nikula 0 siblings, 1 reply; 9+ messages in thread From: Daniel Vetter @ 2018-03-13 11:55 UTC (permalink / raw) To: Jani Nikula; +Cc: intel-gfx, dim-tools, Rodrigo Vivi On Tue, Mar 13, 2018 at 01:30:10PM +0200, Jani Nikula wrote: > Set max line length to 100. I don't want to silence the LONG_LINE > warning altogether, and I'd still prefer to keep lines under 80 > characters, but I also don't want to see all the noise, and nor do I > want to see silly code trying to arbitrarily squeeze under 80 when it > doesn't make sense. 100 is a nice arbitrary round number... I hope > review catches silly stuff regardless. Fingers crossed. > > BIT_MACRO. We have (1 << N) all over the place. I hope to switch to > BIT() macro eventually, but this documents current use. > > PREFER_KERNEL_TYPES. We also have uint(8|16|32|64)_t all over the > place. I also hope to move towards kernel types, but this documents > current use. > > SPLIT_STRING, LONG_LINE_STRING. Don't nag about strings split to many > lines, but also don't nag about strings not split. > > There's plenty more that could be tweaked, but let's start with > something to improve the S/N ratio of the automated CI checkpatch > reports. Now that we have --show-types included in the output, we can > more easily discuss the ignores on a case-by-case basis. > > 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 | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/dim b/dim > index 4ba1c7ff490a..9fa6d9cd855b 100755 > --- a/dim > +++ b/dim > @@ -1390,7 +1390,7 @@ function checkpatch_commit > profile_options="" > ;; > drm-intel) > - profile_options="" > + profile_options="--max-line-length=100 --ignore=BIT_MACRO,PREFER_KERNEL_TYPES,SPLIT_STRING,LONG_LINE_STRING" I've scrolled a bit through checkpatch complaines with this, and I think it looks a lot more reasonable. There's also a huge pile of stuff that we should probably have fixed when the patches landed, so CI'ing this looks good. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> Aside: Should we encourage checkpatch patches to clean this up, with the note that they must use the drm-intel profile and the caveat that we might want to add more stuff to our ignore list instead of taking the patches? -Daniel > ;; > *) > echoerr "Unknown checkpatch profile $profile" > -- > 2.11.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [DIM PATCH 3/3] dim: loosen some drm-intel checkpatch rules 2018-03-13 11:55 ` Daniel Vetter @ 2018-03-14 9:03 ` Jani Nikula 0 siblings, 0 replies; 9+ messages in thread From: Jani Nikula @ 2018-03-14 9:03 UTC (permalink / raw) To: Daniel Vetter; +Cc: intel-gfx, dim-tools, Rodrigo Vivi On Tue, 13 Mar 2018, Daniel Vetter <daniel@ffwll.ch> wrote: > On Tue, Mar 13, 2018 at 01:30:10PM +0200, Jani Nikula wrote: >> Set max line length to 100. I don't want to silence the LONG_LINE >> warning altogether, and I'd still prefer to keep lines under 80 >> characters, but I also don't want to see all the noise, and nor do I >> want to see silly code trying to arbitrarily squeeze under 80 when it >> doesn't make sense. 100 is a nice arbitrary round number... I hope >> review catches silly stuff regardless. Fingers crossed. >> >> BIT_MACRO. We have (1 << N) all over the place. I hope to switch to >> BIT() macro eventually, but this documents current use. >> >> PREFER_KERNEL_TYPES. We also have uint(8|16|32|64)_t all over the >> place. I also hope to move towards kernel types, but this documents >> current use. >> >> SPLIT_STRING, LONG_LINE_STRING. Don't nag about strings split to many >> lines, but also don't nag about strings not split. >> >> There's plenty more that could be tweaked, but let's start with >> something to improve the S/N ratio of the automated CI checkpatch >> reports. Now that we have --show-types included in the output, we can >> more easily discuss the ignores on a case-by-case basis. >> >> 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 | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/dim b/dim >> index 4ba1c7ff490a..9fa6d9cd855b 100755 >> --- a/dim >> +++ b/dim >> @@ -1390,7 +1390,7 @@ function checkpatch_commit >> profile_options="" >> ;; >> drm-intel) >> - profile_options="" >> + profile_options="--max-line-length=100 --ignore=BIT_MACRO,PREFER_KERNEL_TYPES,SPLIT_STRING,LONG_LINE_STRING" > > I've scrolled a bit through checkpatch complaines with this, and I think > it looks a lot more reasonable. There's also a huge pile of stuff that we > should probably have fixed when the patches landed, so CI'ing this looks > good. > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Aside: Should we encourage checkpatch patches to clean this up, with the > note that they must use the drm-intel profile and the caveat that we might > want to add more stuff to our ignore list instead of taking the patches? Overall I just think the checkpatch patches crop up enough without encouragement. IMO let's see how this rolls in CI first and proceed from there. The two warnings that I do think would most benefit from mass change are BIT_MACRO and PREFER_KERNEL_TYPES. People tend to look around them and cargo cult. A little bit of git grep on the C99 types seems to back this up; their use multiplies and prospers in some files, while is neglible in others. BR, Jani. -- Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-03-14 9:03 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-13 11:30 [DIM PATCH 1/3] dim: cleanup checkpatch_commit Jani Nikula 2018-03-13 11:30 ` [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options Jani Nikula 2018-03-13 11:48 ` Daniel Vetter 2018-03-13 15:07 ` Jani Nikula 2018-03-14 6:29 ` Daniel Vetter 2018-03-14 8:57 ` Jani Nikula 2018-03-13 11:30 ` [DIM PATCH 3/3] dim: loosen some drm-intel checkpatch rules Jani Nikula 2018-03-13 11:55 ` Daniel Vetter 2018-03-14 9:03 ` Jani Nikula
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox