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

[James McCoy]

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.

>      * 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.

>  	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.

> +	    $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.

> +	    print "$msg\n" if $verbose;
>  	}
>      }

Cheers,
-- 
James
GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy <jamessan at debian.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 949 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/devscripts-devel/attachments/20150912/b419ba6e/attachment.sig>