{"thread":{"id":"14079","subject":"[jgit PATCH] Paper bag fix quoting for SSH transport commands","startedAt":"2008-06-22T01:36:40Z","lastAt":"2008-06-23T02:30:54Z","messageCount":8,"participants":["Shawn O. Pearce","Robin Rosenberg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"80584","messageId":"20080622013640.GA18629@spearce.org","threadId":"14079","inReplyTo":null,"subject":"[jgit PATCH] Paper bag fix quoting for SSH transport commands","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-06-22T01:36:40Z","receivedAt":"2008-06-22T01:36:40Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Not all Git-over-SSH servers run a Bourne shell on the remote side\nto evaluate the command we are sending.  Some servers run git-shell,\nwhich will fail to execute git-upload-pack if we feed it a quoted\nstring for the name git-upload-pack.\n\nTesting concludes that git-shell requires the command name to never\nbe quoted, and the argument name to always be single quoted.  As\nthis is a long-standing behavior in the wild jgit needs to conform,\nas git-shell and all git-shell work-a-likes such as gitosis may be\nfollowing the same convention.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n\n If there are no arguments I'll push this into the public tree.\n It seems right on the surface, and is necessary to use jgit against\n repo.or.cz, and probably many other sites like it.\n\n .../spearce/jgit/transport/TransportGitSsh.java    |   22 ++++++++++++++++++-\n 1 files changed, 20 insertions(+), 2 deletions(-)\n\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java\nindex d31c525..8944df7 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java\n@@ -88,6 +88,24 @@ class TransportGitSsh extends PackTransport {\n \t\treturn new SshFetchConnection();\n \t}\n \n+\tprivate static void sqMinimal(final StringBuilder cmd, final String val) {\n+\t\tif (val.matches(\"^[a-zA-Z0-9._/-]*$\")) {\n+\t\t\t// If the string matches only generally safe characters\n+\t\t\t// that the shell is not going to evaluate specially we\n+\t\t\t// should leave the string unquoted. Not all systems\n+\t\t\t// actually run a shell and over-quoting confuses them\n+\t\t\t// when it comes to the command name.\n+\t\t\t//\n+\t\t\tcmd.append(val);\n+\t\t} else {\n+\t\t\tsq(cmd, val);\n+\t\t}\n+\t}\n+\n+\tprivate static void sqAlways(final StringBuilder cmd, final String val) {\n+\t\tsq(cmd, val);\n+\t}\n+\n \tprivate static void sq(final StringBuilder cmd, final String val) {\n \t\tint i = 0;\n \n@@ -157,9 +175,9 @@ class TransportGitSsh extends PackTransport {\n \t\t\t\tpath = (uri.getPath().substring(1));\n \n \t\t\tfinal StringBuilder cmd = new StringBuilder();\n-\t\t\tsq(cmd, exe);\n+\t\t\tsqMinimal(cmd, exe);\n \t\t\tcmd.append(' ');\n-\t\t\tsq(cmd, path);\n+\t\t\tsqAlways(cmd, path);\n \t\t\tchannel.setCommand(cmd.toString());\n \t\t\tchannel.setErrStream(System.err);\n \t\t\tchannel.connect();\n-- \n1.5.6.74.g8a5e\n"},{"id":"80633","messageId":"1214156797-29186-1-git-send-email-robin.rosenberg@dewire.com","threadId":"14079","inReplyTo":"20080622013640.GA18629@spearce.org","subject":"[PATCH] Take care of errors reported from the server when upload command is started","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-06-22T17:46:35Z","receivedAt":"2008-06-22T17:46:35Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"When JSch cannot launch the command our SSH Channel will close immediately\nin connect, resulting in a NullPointerException and a closed System.err\nsilencing an errors.\n\nUsing System.err for errors also means we get no feedback when using from\nEclipse.\n---\n .../spearce/egit/ui/EclipseSshSessionFactory.java  |   29 ++++++++++++++++++++\n .../jgit/transport/DefaultSshSessionFactory.java   |    6 ++++\n .../spearce/jgit/transport/SshSessionFactory.java  |   10 +++++++\n .../spearce/jgit/transport/TransportGitSsh.java    |   12 +++++++-\n 4 files changed, 55 insertions(+), 2 deletions(-)\n\ndiff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java\nindex 5005494..144d47d 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/EclipseSshSessionFactory.java\n@@ -8,6 +8,8 @@\n  *******************************************************************************/\n package org.spearce.egit.ui;\n \n+import java.io.IOException;\n+import java.io.OutputStream;\n import java.security.AccessController;\n import java.security.PrivilegedAction;\n \n@@ -44,4 +46,31 @@ class EclipseSshSessionFactory extends SshSessionFactory {\n \t\t\t}\n \t\t});\n \t}\n+\n+\t@Override\n+\tpublic OutputStream getErrorStream() {\n+\t\treturn new OutputStream() {\n+\n+\t\t\tStringBuilder all = new StringBuilder();\n+\t\t\tStringBuilder sb = new StringBuilder();\n+\n+\t\t\tpublic String toString() {\n+\t\t\t\treturn all.toString();\n+\t\t\t}\n+\n+\t\t\t@Override\n+\t\t\tpublic void write(int b) throws IOException {\n+\t\t\t\tif (b == '\\r')\n+\t\t\t\t\treturn;\n+\t\t\t\tsb.append((char) b);\n+\t\t\t\tif (b == '\\n') {\n+\t\t\t\t\tString s = sb.toString();\n+\t\t\t\t\tall.append(s);\n+\t\t\t\t\tsb = new StringBuilder();\n+\t\t\t\t\tActivator.logError(s, new Throwable());\n+\t\t\t\t}\n+\t\t\t}\n+\t\t};\n+\t}\n+\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java b/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java\nindex 8a59904..5924a04 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/DefaultSshSessionFactory.java\n@@ -46,6 +46,7 @@ import java.io.File;\n import java.io.FileInputStream;\n import java.io.FileNotFoundException;\n import java.io.IOException;\n+import java.io.OutputStream;\n import java.security.AccessController;\n import java.security.PrivilegedAction;\n \n@@ -249,4 +250,9 @@ class DefaultSshSessionFactory extends SshSessionFactory {\n \t\t\treturn null; // cancel\n \t\t}\n \t}\n+\n+\t@Override\n+\tpublic OutputStream getErrorStream() {\n+\t\treturn System.err;\n+\t}\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/SshSessionFactory.java b/org.spearce.jgit/src/org/spearce/jgit/transport/SshSessionFactory.java\nindex 7f8136d..4c9eae0 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/SshSessionFactory.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/SshSessionFactory.java\n@@ -38,6 +38,8 @@\n \n package org.spearce.jgit.transport;\n \n+import java.io.OutputStream;\n+\n import com.jcraft.jsch.JSchException;\n import com.jcraft.jsch.Session;\n \n@@ -121,4 +123,12 @@ public abstract class SshSessionFactory {\n \t\tif (session.isConnected())\n \t\t\tsession.disconnect();\n \t}\n+\n+\t/**\n+\t * Find or create an OutputStream for Ssh to use. For a command line client\n+\t * this is probably System.err.\n+\t * \n+\t * @return an OutputStream to receive the SSH error stream.\n+\t */\n+\tpublic abstract OutputStream getErrorStream();\n }\ndiff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java\nindex 8944df7..82723a3 100644\n--- a/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java\n+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/TransportGitSsh.java\n@@ -39,6 +39,7 @@\n package org.spearce.jgit.transport;\n \n import java.io.IOException;\n+import java.io.OutputStream;\n import java.net.ConnectException;\n import java.net.UnknownHostException;\n \n@@ -77,6 +78,7 @@ class TransportGitSsh extends PackTransport {\n \t}\n \n \tfinal SshSessionFactory sch;\n+\tOutputStream errStream;\n \n \tTransportGitSsh(final Repository local, final URIish uri) {\n \t\tsuper(local, uri);\n@@ -179,7 +181,8 @@ class TransportGitSsh extends PackTransport {\n \t\t\tcmd.append(' ');\n \t\t\tsqAlways(cmd, path);\n \t\t\tchannel.setCommand(cmd.toString());\n-\t\t\tchannel.setErrStream(System.err);\n+\t\t\terrStream = SshSessionFactory.getInstance().getErrorStream();\n+\t\t\tchannel.setErrStream(errStream, true); \n \t\t\tchannel.connect();\n \t\t\treturn channel;\n \t\t} catch (JSchException je) {\n@@ -198,7 +201,12 @@ class TransportGitSsh extends PackTransport {\n \t\t\ttry {\n \t\t\t\tsession = openSession();\n \t\t\t\tchannel = exec(session, getOptionUploadPack());\n-\t\t\t\tinit(channel.getInputStream(), channel.getOutputStream());\n+\n+\t\t\t\tif (channel.isConnected())\n+\t\t\t\t\tinit(channel.getInputStream(), channel.getOutputStream());\n+\t\t\t\telse\n+\t\t\t\t\tthrow new TransportException(errStream.toString());\n+\t\t\t\t\t\n \t\t\t} catch (TransportException err) {\n \t\t\t\tclose();\n \t\t\t\tthrow err;\n-- \n1.5.5.1.178.g1f811\n"},{"id":"80634","messageId":"1214156797-29186-2-git-send-email-robin.rosenberg@dewire.com","threadId":"14079","inReplyTo":"1214156797-29186-1-git-send-email-robin.rosenberg@dewire.com","subject":"[PATCH] Clone: Handle cancel in clone dialog specially","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-06-22T17:46:36Z","receivedAt":"2008-06-22T17:46:36Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n .../src/org/spearce/egit/ui/UIText.java            |    3 +++\n .../egit/ui/internal/clone/SourceBranchPage.java   |   18 ++++++++++++------\n .../src/org/spearce/egit/ui/uitext.properties      |    2 ++\n 3 files changed, 17 insertions(+), 6 deletions(-)\n\ndiff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java\nindex 9ccf606..4adb99c 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/UIText.java\n@@ -122,6 +122,9 @@ public class UIText extends NLS {\n \tpublic static String SourceBranchPage_cannotListBranches;\n \n \t/** */\n+\tpublic static String SourceBranchPage_remoteListingCancelled;\n+\n+\t/** */\n \tpublic static String CloneDestinationPage_title;\n \n \t/** */\ndiff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java\nindex b2f1a18..615be32 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java\n@@ -20,6 +20,7 @@ import java.util.List;\n import org.eclipse.core.runtime.IProgressMonitor;\n import org.eclipse.core.runtime.IStatus;\n import org.eclipse.core.runtime.NullProgressMonitor;\n+import org.eclipse.core.runtime.OperationCanceledException;\n import org.eclipse.core.runtime.Status;\n import org.eclipse.jface.dialogs.ErrorDialog;\n import org.eclipse.jface.operation.IRunnableWithProgress;\n@@ -269,12 +270,17 @@ class SourceBranchPage extends WizardPage {\n \t\t\t});\n \t\t} catch (InvocationTargetException e) {\n \t\t\tThrowable why = e.getCause();\n-\t\t\tErrorDialog.openError(getShell(),\n-\t\t\t\t\tUIText.SourceBranchPage_transportError,\n-\t\t\t\t\tUIText.SourceBranchPage_cannotListBranches, new Status(\n-\t\t\t\t\t\t\tIStatus.ERROR, Activator.getPluginId(), 0, why\n-\t\t\t\t\t\t\t\t\t.getMessage(), why.getCause()));\n-\t\t\ttransportError(why.getMessage());\n+\t\t\tif ((why instanceof OperationCanceledException)) {\n+\t\t\t\ttransportError(UIText.SourceBranchPage_remoteListingCancelled);\n+\t\t\t\treturn;\n+\t\t\t} else {\n+\t\t\t\tErrorDialog.openError(getShell(),\n+\t\t\t\t\t\tUIText.SourceBranchPage_transportError,\n+\t\t\t\t\t\tUIText.SourceBranchPage_cannotListBranches, new Status(\n+\t\t\t\t\t\t\t\tIStatus.ERROR, Activator.getPluginId(), 0, why\n+\t\t\t\t\t\t\t\t\t\t.getMessage(), why.getCause()));\n+\t\t\t\ttransportError(why.getMessage());\n+\t\t\t}\n \t\t\treturn;\n \t\t} catch (InterruptedException e) {\n \t\t\ttransportError(UIText.SourceBranchPage_interrupted);\ndiff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties\nindex 18f8c28..9516aa0 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/uitext.properties\n@@ -55,6 +55,8 @@ SourceBranchPage_errorBranchRequired=At least one branch must be selected.\n SourceBranchPage_transportError=Transport Error\n SourceBranchPage_cannotListBranches=Cannot list the available branches.\n SourceBranchPage_interrupted=Connection attempt interrupted.\n+SourceBranchPage_remoteListingCancelled=Operation cancelled\n+\n \n CloneDestinationPage_title=Local Destination\n CloneDestinationPage_description=Configure the local storage location for {0}.\n-- \n1.5.5.1.178.g1f811\n"},{"id":"80632","messageId":"1214156797-29186-3-git-send-email-robin.rosenberg@dewire.com","threadId":"14079","inReplyTo":"1214156797-29186-2-git-send-email-robin.rosenberg@dewire.com","subject":"[PATCH] Clone: If url is changed was changed, forget the old value","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-06-22T17:46:37Z","receivedAt":"2008-06-22T17:46:37Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"This fixes a bug found by some monkey testing where the dialog\nget stuck in a state where changing connection parameters have\nno effect.\n\nSigned-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n .../egit/ui/internal/clone/SourceBranchPage.java   |    4 +++-\n 1 files changed, 3 insertions(+), 1 deletions(-)\n\ndiff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java\nindex 615be32..b704aaa 100644\n--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java\n+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/clone/SourceBranchPage.java\n@@ -75,8 +75,10 @@ class SourceBranchPage extends WizardPage {\n \t\tsetImageDescriptor(UIIcons.WIZBAN_IMPORT_REPO);\n \t\tsourcePage.addURIishChangeListener(new URIishChangeListener() {\n \t\t\tpublic void uriishChanged(final URIish newURI) {\n-\t\t\t\tif (newURI == null || !newURI.equals(validated))\n+\t\t\t\tif (newURI == null || !newURI.equals(validated)) {\n+\t\t\t\t\tvalidated = null;\n \t\t\t\t\tsetPageComplete(false);\n+\t\t\t\t}\n \t\t\t}\n \t\t});\n \t\tbranchChangeListeners = new ArrayList<BranchChangeListener>(3);\n-- \n1.5.5.1.178.g1f811\n"},{"id":"80629","messageId":"200806221954.08919.robin.rosenberg.lists@dewire.com","threadId":"14079","inReplyTo":"20080622013640.GA18629@spearce.org","subject":"Re: [jgit PATCH] Paper bag fix quoting for SSH transport commands","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2008-06-22T17:54:08Z","receivedAt":"2008-06-22T17:54:08Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"söndagen den 22 juni 2008 03.36.40 skrev Shawn O. Pearce:\n> Not all Git-over-SSH servers run a Bourne shell on the remote side\n> to evaluate the command we are sending.  Some servers run git-shell,\n> which will fail to execute git-upload-pack if we feed it a quoted\n> string for the name git-upload-pack.\n> \n> Testing concludes that git-shell requires the command name to never\n> be quoted, and the argument name to always be single quoted.  As\n> this is a long-standing behavior in the wild jgit needs to conform,\n> as git-shell and all git-shell work-a-likes such as gitosis may be\n> following the same convention.\n> \n> Signed-off-by: Shawn O. Pearce <spearce@spearce.org>\n> ---\n> \n>  If there are no arguments I'll push this into the public tree.\n>  It seems right on the surface, and is necessary to use jgit against\n>  repo.or.cz, and probably many other sites like it.\n\nSeems ok and works here. Error handling still has a paperbagish feel. See\nfollow up patches.\n\nMaybe we should have a patch for git too so it will actually work with spaces in file names. What do people on Windows do? (those that actually get an SSH server up and running and sleep well overe it on that platform).\n\nAs for pushing and signing. One way is for you (Shawn) and me is to sign-off and push each other's patches. I pushed this one.\n\n-- robin\n"},{"id":"80648","messageId":"20080622221545.GF11793@spearce.org","threadId":"14079","inReplyTo":"200806221954.08919.robin.rosenberg.lists@dewire.com","subject":"Re: [jgit PATCH] Paper bag fix quoting for SSH transport commands","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-06-22T22:15:45Z","receivedAt":"2008-06-22T22:15:45Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:\n> söndagen den 22 juni 2008 03.36.40 skrev Shawn O. Pearce:\n> > \n> > Testing concludes that git-shell requires the command name to never\n> > be quoted, and the argument name to always be single quoted.  As\n> > this is a long-standing behavior in the wild jgit needs to conform,\n> > as git-shell and all git-shell work-a-likes such as gitosis may be\n> > following the same convention.\n> \n> Seems ok and works here. Error handling still has a paperbagish feel. See\n> follow up patches.\n\nWell, I wasn't trying to clean up error handling, I was just trying\nto make `jgit fetch` work against repo.or.cz.  But I agree, the\nerror handling and feedback in the transport code could be improved.\n\n> Maybe we should have a patch for git too so it will actually work\n> with spaces in file names. What do people on Windows do? (those that\n> actually get an SSH server up and running and sleep well overe it\n> on that platform).\n\nThe issue only happens with the command name, which most people\nuse just git-upload-pack/git-receive-pack as they have git in their\n$PATH on the remote side, or are running git-shell.  I suspect that\nWindows users just don't run Git over SSH with paths that contain\nspaces to access git-upload-pack remotely.\n\nI'd patch Git to use the same rule as jgit and only quote the command\nwhen really necessary, but its not high on my list of things to do\nthis month.  :-)\n\n> As for pushing and signing. One way is for you (Shawn) and me is\n> to sign-off and push each other's patches. I pushed this one.\n\nGiven that repo.or.cz doesn't show reflogs, I take it this is only\na way to make sure at least someone has reviewed the patch before\nit goes into the main tree, since we both have write access?  I\ncan live with that.\n\n-- \nShawn.\n"},{"id":"80653","messageId":"20080622230159.GG11793@spearce.org","threadId":"14079","inReplyTo":"1214156797-29186-1-git-send-email-robin.rosenberg@dewire.com","subject":"Re: [PATCH] Take care of errors reported from the server when upload command is started","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2008-06-22T23:01:59Z","receivedAt":"2008-06-22T23:01:59Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Robin Rosenberg <robin.rosenberg@dewire.com> wrote:\n> @@ -44,4 +46,31 @@ class EclipseSshSessionFactory extends SshSessionFactory {\n> +\t@Override\n> +\tpublic OutputStream getErrorStream() {\n...\n> +\t\t\t\t\tActivator.logError(s, new Throwable());\n\nI'm not sure what value the Throwable gives us here; it may be some\ncall stack deep within JSch, isn't it?  Is it useful to include?\n\nWe may also want the log records in Eclipse to say which remote the\nmessage came from, which means passing in the URIish as a parameter.\n\n> @@ -77,6 +78,7 @@ class TransportGitSsh extends PackTransport {\n>  \t}\n>  \n>  \tfinal SshSessionFactory sch;\n> +\tOutputStream errStream;\n\nCould we avoid putting the error stream as an instance member\nof the transport by instead using channel.getErrStream() in the\nexception case below?\n  \n> @@ -179,7 +181,8 @@ class TransportGitSsh extends PackTransport {\n>  \t\t\tcmd.append(' ');\n>  \t\t\tsqAlways(cmd, path);\n>  \t\t\tchannel.setCommand(cmd.toString());\n> -\t\t\tchannel.setErrStream(System.err);\n> +\t\t\terrStream = SshSessionFactory.getInstance().getErrorStream();\n\nUse sch rather than SshSessionFactory.getInstance().  We store it in the\ntransport so that once the transport instance is created it always goes\nto the same SshSessionFactory for anything it needs, even if the caller\nhas changed the global SshSessionFactory away on us.\n\n> @@ -198,7 +201,12 @@ class TransportGitSsh extends PackTransport {\n>  \t\t\ttry {\n>  \t\t\t\tsession = openSession();\n>  \t\t\t\tchannel = exec(session, getOptionUploadPack());\n> -\t\t\t\tinit(channel.getInputStream(), channel.getOutputStream());\n> +\n> +\t\t\t\tif (channel.isConnected())\n> +\t\t\t\t\tinit(channel.getInputStream(), channel.getOutputStream());\n> +\t\t\t\telse\n> +\t\t\t\t\tthrow new TransportException(errStream.toString());\n\nI think you can say channel.getErrStream() here and not need the\ninstance member.\n\n-- \nShawn.\n"},{"id":"80670","messageId":"200806230430.54639.robin.rosenberg.lists@dewire.com","threadId":"14079","inReplyTo":"20080622221545.GF11793@spearce.org","subject":"Re: [jgit PATCH] Paper bag fix quoting for SSH transport commands","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2008-06-23T02:30:54Z","receivedAt":"2008-06-23T02:30:54Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"måndagen den 23 juni 2008 00.15.45 skrev Shawn O. Pearce:\n> Robin Rosenberg <robin.rosenberg.lists@dewire.com> wrote:\n> > As for pushing and signing. One way is for you (Shawn) and me is\n> > to sign-off and push each other's patches. I pushed this one.\n> \n> Given that repo.or.cz doesn't show reflogs, I take it this is only\n> a way to make sure at least someone has reviewed the patch before\n> it goes into the main tree, since we both have write access?  I\n> can live with that.\nWe'll try that (unless we need pushing desperately).\n\n-- robin\n"}]}