Re: [PATCH] Take care of errors reported from the server when upload command is started

["Shawn O. Pearce" <spearce@spearce.org>]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> @@ -44,4 +46,31 @@ class EclipseSshSessionFactory extends SshSessionFactory {
> +	@Override
> +	public OutputStream getErrorStream() {
...
> +					Activator.logError(s, new Throwable());

I'm not sure what value the Throwable gives us here; it may be some
call stack deep within JSch, isn't it?  Is it useful to include?

We may also want the log records in Eclipse to say which remote the
message came from, which means passing in the URIish as a parameter.

> @@ -77,6 +78,7 @@ class TransportGitSsh extends PackTransport {
>  	}
>  
>  	final SshSessionFactory sch;
> +	OutputStream errStream;

Could we avoid putting the error stream as an instance member
of the transport by instead using channel.getErrStream() in the
exception case below?
  
> @@ -179,7 +181,8 @@ class TransportGitSsh extends PackTransport {
>  			cmd.append(' ');
>  			sqAlways(cmd, path);
>  			channel.setCommand(cmd.toString());
> -			channel.setErrStream(System.err);
> +			errStream = SshSessionFactory.getInstance().getErrorStream();

Use sch rather than SshSessionFactory.getInstance().  We store it in the
transport so that once the transport instance is created it always goes
to the same SshSessionFactory for anything it needs, even if the caller
has changed the global SshSessionFactory away on us.

> @@ -198,7 +201,12 @@ class TransportGitSsh extends PackTransport {
>  			try {
>  				session = openSession();
>  				channel = exec(session, getOptionUploadPack());
> -				init(channel.getInputStream(), channel.getOutputStream());
> +
> +				if (channel.isConnected())
> +					init(channel.getInputStream(), channel.getOutputStream());
> +				else
> +					throw new TransportException(errStream.toString());

I think you can say channel.getErrStream() here and not need the
instance member.

-- 
Shawn.