[devscripts] 02/04: uscan: --verbose

[Osamu Aoki]

Hi,

On Sat, Sep 12, 2015 at 02:43:40PM -0400, James McCoy wrote:
> Thanks for enhancing the verbose functionality.  A couple comments
> below.
> 
> On Sat, Sep 12, 2015 at 04:03:38PM +0000, Osamu Aoki wrote:
> > This is an automated email from the git hooks/post-receive script.
> > 
> > osamu pushed a commit to branch master
> > in repository devscripts.
> > 
> > commit ec64b2f86b26da9040880cf595b3ce817c446c62
> > Author: Osamu Aoki <osamu at debian.org>
> > Date:   Sun Sep 6 17:01:24 2015 +0900
> > 
> >     uscan: --verbose
> >     
> >     [uscan] please offer debugging possibility for mangle rules
> >     https://bugs.debian.org/350454
> 
> I'm not sure how the below changes are addressing the request from that
> bug.  In fact, it looks like that was already addressed, but not closed,
> by cd2ff7ec.

As you mentioned in the next message, I should have swapped the order of
commit when rebasing commits.
 
> >      * start uupdate --verbose as needed
> >      * retain output of script if ---verbose
> >      * use dpkg::IPC as much to be consistent
> > ---
> >  scripts/uscan.pl | 16 +++++++++++++---
> >  1 file changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/scripts/uscan.pl b/scripts/uscan.pl
> > index 33f3ad4..a401544 100755
> > --- a/scripts/uscan.pl
> > +++ b/scripts/uscan.pl
> > @@ -1547,20 +1547,30 @@ EOF
> >  	if ($action =~ /^uupdate(\s|$)/) {
> >  	    push @cmd, "--no-symlink";
> >  	}
> > +	if ($verbose) {
> > +	    push @cmd, "--verbose";
> > +	}
> 
> The $verbose block should probably be inside the previous if, since we
> only know for sure that uupdate supports --verbose.  If someone isn't
> using uupdate as the action, then passing --verbose may cause it to
> fail.

Very true.

> >  	if ($watch_version > 1) {
> >  	    push @cmd, "--upstream-version", $newversion, $path;
> >  	} else {
> >  	    push @cmd, $path, $newversion;
> >  	}
> > +	my $msg;
> >  	my $actioncmd = join(" ", @cmd);
> >  	print "-- Executing user specified script\n     $actioncmd\n" if $verbose;
> >  	if ($dehs) {
> > -	    my $msg = "Executing user specified script: $actioncmd; output:\n";
> > -	    $msg .= `$actioncmd 2>&1`;
> > +	    spawn(exec => \@cmd,
> > +		error_to_string => \$msg,
> > +		wait_child => 1);
> 
> This needs to use nocheck => 1 to retain the previous behavior of not
> exiting the script if the user action errors.  It also needs to use the
> to_string key to capture the stdout in another variable.

Yes. nocheck => 1

Agh... for to_string.  If we capture to "another variable", then
concatenate later, then message order may be in the same order.

Hmmm... this may have been a bad idea.

> > +	    $msg = $msg . 
> > +		"Executing user specified script: $actioncmd; output:\n";
> 
> I think you wanted the literal string first, so the output continues to
> be after the "output:" part of the message.
> 
> >  	    dehs_msg($msg);
> >  	} else {
> > -	    system(@cmd);
> > +	    spawn(exec => \@cmd,
> > +		error_to_string => \$msg,
> > +		wait_child => 1);
> 
> Similar changes are needed here regarding nocheck and to_string.

Hmmm... adding nocheck is easy fix but fixing output sequence is not so
simple.  

Is it better to go back to `$actioncmd 2>&1` style for both?

Adding 2>&1 to @cmd does not work as tested (as expected).
Also using same $msg does not merge output.
Of course executing under another shell works but too complicated.
 
> > +	    print "$msg\n" if $verbose;

But add this for verbose output

Osamu