Re: [PATCH] dim: switch to dry-run in warn_or_fail

[Daniel Vetter]

On Fri, May 04, 2018 at 03:54:07PM +0300, Jani Nikula wrote:
> On Fri, 04 May 2018, Daniel Vetter  wrote:
> > When merging a pull requests there's potentially a long list of
> > problematic patches. By switching to dry-run mode we can dump them
> > all.
> >
> > The upside here is that without this patch the workflow when
> > processing a pull is:
> >
> > $ dim apply-pull ...
> > -> dim refuses, only reports first problem
> > $ dim -d apply-pull
> > -> you see all the issues, make up your mind to merge or not
> > $ dim -f apply-pull
> >
> > With this patch we have one step less:
> >
> > $ dim apply-pull
> > -> refuses pull, but with the auto-switch to dry-run reports
> > everything and you can immediately make up your mind to merge or not
> > $ dim -f apply-pull
> 
> Downside is that warn_or_fail now *never* fails, and you have to make
> absolutely sure dry run actually is a dry run *everywhere*. So far it's
> been best effort.
> 
> At least dim rebuild-tip does not handle this cleanly.
> 
> I'd just make checkpatch_commit_push do logging and return an exit code,
> which the callers can handle as fatally as they want. I.e. there's the
> loop to go through all the commits, handle exit codes from
> checkpatch_commit_push just like apply_patch handles exit codes, then
> loop through all the commits, and after that, do the warn_or_fail if rv
> is non-zero.

Yeah, that's a safer way to implement the same idea. I'll do that.
-Daniel

> 
> BR,
> Jani.
> 
> 
> >
> > Cc: Dave Airlie 
> > Signed-off-by: Daniel Vetter 
> > ---
> >  dim | 13 +
> >  1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/dim b/dim
> > index d8288a342352..499ffcfdd807 100755
> > --- a/dim
> > +++ b/dim
> > @@ -144,6 +144,12 @@ function echoerr
> > echo "$dim: $*" >&2
> >  }
> >  
> > +function enable_dry_run
> > +{
> > +   DRY_RUN=--dry-run
> > +   DRY="echo"
> > +}
> > +
> >  function warn_or_fail
> >  {
> > if [[ $FORCE ]] ; then
> > @@ -151,8 +157,8 @@ function warn_or_fail
> > elif [[ $DRY ]] ; then
> > echoerr "WARNING: $1, but continuing dry-run"
> > else
> > -   echoerr "ERROR: $1, aborting"
> > -   exit 1
> > +   echoerr "ERROR: $1, switching to dry-run mode"
> > +   enable_dry_run
> > fi
> >  }
> >  
> > @@ -2261,8 +2267,7 @@ HELP=
> >  while getopts hdfis opt; do
> > case "$opt" in
> > d)
> > -   DRY_RUN=--dry-run
> > -   DRY="echo"
> > +   enable_dry_run
> > ;;
> > f)
> > FORCE=1
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dim-tools mailing list
dim-tools@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dim-tools

Re: [PATCH] dim: switch to dry-run in warn_or_fail

[Jani Nikula]

On Fri, 04 May 2018, Daniel Vetter  wrote:
> When merging a pull requests there's potentially a long list of
> problematic patches. By switching to dry-run mode we can dump them
> all.
>
> The upside here is that without this patch the workflow when
> processing a pull is:
>
> $ dim apply-pull ...
> -> dim refuses, only reports first problem
> $ dim -d apply-pull
> -> you see all the issues, make up your mind to merge or not
> $ dim -f apply-pull
>
> With this patch we have one step less:
>
> $ dim apply-pull
> -> refuses pull, but with the auto-switch to dry-run reports
> everything and you can immediately make up your mind to merge or not
> $ dim -f apply-pull

Downside is that warn_or_fail now *never* fails, and you have to make
absolutely sure dry run actually is a dry run *everywhere*. So far it's
been best effort.

At least dim rebuild-tip does not handle this cleanly.

I'd just make checkpatch_commit_push do logging and return an exit code,
which the callers can handle as fatally as they want. I.e. there's the
loop to go through all the commits, handle exit codes from
checkpatch_commit_push just like apply_patch handles exit codes, then
loop through all the commits, and after that, do the warn_or_fail if rv
is non-zero.

BR,
Jani.


>
> Cc: Dave Airlie 
> Signed-off-by: Daniel Vetter 
> ---
>  dim | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/dim b/dim
> index d8288a342352..499ffcfdd807 100755
> --- a/dim
> +++ b/dim
> @@ -144,6 +144,12 @@ function echoerr
>   echo "$dim: $*" >&2
>  }
>  
> +function enable_dry_run
> +{
> + DRY_RUN=--dry-run
> + DRY="echo"
> +}
> +
>  function warn_or_fail
>  {
>   if [[ $FORCE ]] ; then
> @@ -151,8 +157,8 @@ function warn_or_fail
>   elif [[ $DRY ]] ; then
>   echoerr "WARNING: $1, but continuing dry-run"
>   else
> - echoerr "ERROR: $1, aborting"
> - exit 1
> + echoerr "ERROR: $1, switching to dry-run mode"
> + enable_dry_run
>   fi
>  }
>  
> @@ -2261,8 +2267,7 @@ HELP=
>  while getopts hdfis opt; do
>   case "$opt" in
>   d)
> - DRY_RUN=--dry-run
> - DRY="echo"
> + enable_dry_run
>   ;;
>   f)
>   FORCE=1

-- 
Jani Nikula, Intel Open Source Technology Center
___
dim-tools mailing list
dim-tools@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dim-tools

Re: [PATCH] dim: switch to dry-run in warn_or_fail

[Dave Airlie]

Yeah this seems useful after doing my first -f pull today.

Acked-by: Dave Airlie 

On 4 May 2018 at 15:21, Daniel Vetter  wrote:
> When merging a pull requests there's potentially a long list of
> problematic patches. By switching to dry-run mode we can dump them
> all.
>
> The upside here is that without this patch the workflow when
> processing a pull is:
>
> $ dim apply-pull ...
> -> dim refuses, only reports first problem
> $ dim -d apply-pull
> -> you see all the issues, make up your mind to merge or not
> $ dim -f apply-pull
>
> With this patch we have one step less:
>
> $ dim apply-pull
> -> refuses pull, but with the auto-switch to dry-run reports
> everything and you can immediately make up your mind to merge or not
> $ dim -f apply-pull
>
> Cc: Dave Airlie 
> Signed-off-by: Daniel Vetter 
> ---
>  dim | 13 +
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/dim b/dim
> index d8288a342352..499ffcfdd807 100755
> --- a/dim
> +++ b/dim
> @@ -144,6 +144,12 @@ function echoerr
> echo "$dim: $*" >&2
>  }
>
> +function enable_dry_run
> +{
> +   DRY_RUN=--dry-run
> +   DRY="echo"
> +}
> +
>  function warn_or_fail
>  {
> if [[ $FORCE ]] ; then
> @@ -151,8 +157,8 @@ function warn_or_fail
> elif [[ $DRY ]] ; then
> echoerr "WARNING: $1, but continuing dry-run"
> else
> -   echoerr "ERROR: $1, aborting"
> -   exit 1
> +   echoerr "ERROR: $1, switching to dry-run mode"
> +   enable_dry_run
> fi
>  }
>
> @@ -2261,8 +2267,7 @@ HELP=
>  while getopts hdfis opt; do
> case "$opt" in
> d)
> -   DRY_RUN=--dry-run
> -   DRY="echo"
> +   enable_dry_run
> ;;
> f)
> FORCE=1
> --
> 2.17.0
>
___
dim-tools mailing list
dim-tools@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dim-tools