changeset 367:061e5f3a6bad

Make plugin compatible with mercurial 2.1 Hg pull command in 2.1 returns a non-zero exit code when no new changes were pulled. We detected that as a error.
author Dmitry Neverov <dmitry.neverov@jetbrains.com>
date Fri, 03 Feb 2012 13:23:18 +0400
parents a75f2b73b1d8
children 0b2e9154d26e
files mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/ArchiveCommand.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/BaseCommand.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CatCommand.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CloneCommand.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CommandExecutionSettings.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CommandExecutionSettingsBuilder.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CommandResult.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CommandUtil.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/IdentifyCommand.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/PullCommand.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/PushCommand.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/UnknownFileException.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/UnknownRevisionException.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/UpdateCommand.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/VcsRootCommand.java mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/VersionCommand.java mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CatCommandTest.java mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CommandResultTest.java
diffstat 18 files changed, 326 insertions(+), 128 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/ArchiveCommand.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/ArchiveCommand.java	Fri Feb 03 13:23:18 2012 +0400
@@ -20,6 +20,8 @@
 
 import java.io.File;
 
+import static jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CommandExecutionSettingsBuilder.with;
+
 public class ArchiveCommand extends VcsRootCommand {
   private File myDestDir;
   private String myToId;
@@ -45,8 +47,7 @@
     setRevision(cli);
     setDestination(cli);
 
-    CommandResult res = runCommand(cli);
-    failIfNotEmptyStdErr(res);
+    runCommand(cli, with().failureWhenStderrNotEmpty());
     deleteHgArchival();
   }
 
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/BaseCommand.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/BaseCommand.java	Fri Feb 03 13:23:18 2012 +0400
@@ -18,11 +18,9 @@
 import com.intellij.execution.configurations.GeneralCommandLine;
 import com.intellij.openapi.util.SystemInfo;
 import jetbrains.buildServer.util.StringUtil;
-import jetbrains.buildServer.vcs.VcsException;
 import org.jetbrains.annotations.NotNull;
 
 import java.io.File;
-import java.util.Collections;
 
 /**
  * @author pavel
@@ -95,18 +93,4 @@
   private void setupHg(GeneralCommandLine cli) {
     cli.setExePath(myHgPath);
   }
-
-  protected CommandResult runCommand(@NotNull GeneralCommandLine cli) throws VcsException {
-    return CommandUtil.runCommand(cli, Collections.<String>emptySet());
-  }
-
-  protected CommandResult runCommand(@NotNull GeneralCommandLine cli, int executionTimeout) throws VcsException {
-    return CommandUtil.runCommand(cli, executionTimeout, Collections.<String>emptySet());
-  }
-
-  protected void failIfNotEmptyStdErr(@NotNull CommandResult res) throws VcsException {
-    if (!StringUtil.isEmpty(res.getStderr())) {
-      CommandUtil.commandFailed(res);
-    }
-  }
 }
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CatCommand.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CatCommand.java	Fri Feb 03 13:23:18 2012 +0400
@@ -27,6 +27,7 @@
 import java.util.Queue;
 
 import static com.intellij.openapi.util.io.FileUtil.delete;
+import static jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CommandExecutionSettingsBuilder.with;
 
 public class CatCommand extends VcsRootCommand {
   private String myRevId;
@@ -86,7 +87,7 @@
         cmdSize += path.length();
       } while (cmdSize < MAX_CMD_LEN && !paths.isEmpty());
 
-      runCommand(cli, checkFailure);
+      runCommand(cli, with().checkForFailure(checkFailure));
     }
   }
 
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CloneCommand.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CloneCommand.java	Fri Feb 03 13:23:18 2012 +0400
@@ -21,6 +21,8 @@
 
 import java.io.File;
 
+import static jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CommandExecutionSettingsBuilder.with;
+
 public class CloneCommand extends VcsRootCommand {
   private String myToId;
   private boolean myUpdateWorkingDir = true;
@@ -74,6 +76,6 @@
     cli.addParameter(myRepository);
     cli.addParameter(myWorkingDir.getName());
 
-    runCommand(cli, 24*3600); // some repositories are quite large, we set timeout to 24 hours
+    runCommand(cli, with().timeout(24 * 3600)); // some repositories are quite large, we set timeout to 24 hours
   }
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CommandExecutionSettings.java	Fri Feb 03 13:23:18 2012 +0400
@@ -0,0 +1,40 @@
+package jetbrains.buildServer.buildTriggers.vcs.mercurial.command;
+
+import org.jetbrains.annotations.NotNull;
+
+import java.util.Set;
+
+/**
+ * @author dmitry.neverov
+ */
+public class CommandExecutionSettings {
+
+  private final int myTimeout;
+  private final Set<String> myPrivateData;
+  private final boolean myCheckForFailure;
+  private final boolean myFailWhenStderrNotEmpty;
+
+  CommandExecutionSettings(int timeout, @NotNull Set<String> privateData, boolean checkForFailure, boolean failWhenStderrNotEmpty) {
+    myTimeout = timeout;
+    myPrivateData = privateData;
+    myCheckForFailure = checkForFailure;
+    myFailWhenStderrNotEmpty = failWhenStderrNotEmpty;
+  }
+
+  public int timeout() {
+    return myTimeout;
+  }
+
+  @NotNull
+  public Set<String> privateData() {
+    return myPrivateData;
+  }
+
+  public boolean shouldCheckForFailure() {
+    return myCheckForFailure;
+  }
+
+  public boolean shouldFailWithNonEmptyStderr() {
+    return myFailWhenStderrNotEmpty;
+  }
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CommandExecutionSettingsBuilder.java	Fri Feb 03 13:23:18 2012 +0400
@@ -0,0 +1,47 @@
+package jetbrains.buildServer.buildTriggers.vcs.mercurial.command;
+
+import org.jetbrains.annotations.NotNull;
+
+import java.util.Collections;
+import java.util.Set;
+
+/**
+ * @author dmitry.neverov
+ */
+public class CommandExecutionSettingsBuilder {
+
+  private static final int DEFAULT_COMMAND_TIMEOUT_SEC = 3600;
+
+  private int myTimeout = DEFAULT_COMMAND_TIMEOUT_SEC;
+  private Set<String> myPrivateData = Collections.emptySet();
+  private boolean myCheckForFailure = true;
+  private boolean myFailWhenStderrNotEmpty = false;
+
+  public static CommandExecutionSettingsBuilder with() {
+    return new CommandExecutionSettingsBuilder();
+  }
+
+  public CommandExecutionSettings build() {
+    return new CommandExecutionSettings(myTimeout, myPrivateData, myCheckForFailure, myFailWhenStderrNotEmpty);
+  }
+
+  public CommandExecutionSettingsBuilder timeout(int timeout) {
+    myTimeout = timeout;
+    return this;
+  }
+
+  public CommandExecutionSettingsBuilder privateData(@NotNull Set<String> privateData) {
+    myPrivateData = privateData;
+    return this;
+  }
+
+  public CommandExecutionSettingsBuilder checkForFailure(boolean checkForFailure) {
+    myCheckForFailure = checkForFailure;
+    return this;
+  }
+
+  public CommandExecutionSettingsBuilder failureWhenStderrNotEmpty() {
+    myFailWhenStderrNotEmpty = true;
+    return this;
+  }
+}
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CommandResult.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CommandResult.java	Fri Feb 03 13:23:18 2012 +0400
@@ -1,6 +1,8 @@
 package jetbrains.buildServer.buildTriggers.vcs.mercurial.command;
 
+import com.intellij.openapi.diagnostic.Logger;
 import jetbrains.buildServer.ExecResult;
+import jetbrains.buildServer.util.StringUtil;
 import jetbrains.buildServer.vcs.VcsException;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -8,24 +10,27 @@
 import java.util.Collections;
 import java.util.Set;
 
+import static com.intellij.openapi.util.text.StringUtil.isEmpty;
 import static jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CommandUtil.removePrivateData;
 
 /**
- * Decorator for ExecResult that filters out private data from stdout and strerr.
+ * Mercurial command result. Filters out private data from stdout and detects errors.
  *
  * @author dmitry.neverov
  */
 public class CommandResult {
 
+  private final Logger myLogger;
   private final String myCommand;
   private final ExecResult myDelegate;
   private final Set<String> myPrivateData;
 
-  public CommandResult(@NotNull final String command, @NotNull final ExecResult execResult) {
-    this(command, execResult, Collections.<String>emptySet());
+  public CommandResult(@NotNull Logger logger, @NotNull String command, @NotNull ExecResult execResult) {
+    this(logger, command, execResult, Collections.<String>emptySet());
   }
 
-  public CommandResult(@NotNull final String command, @NotNull final ExecResult execResult, @NotNull final Set<String> privateData) {
+  public CommandResult(@NotNull Logger logger, @NotNull String command, @NotNull ExecResult execResult, @NotNull Set<String> privateData) {
+    myLogger = logger;
     myCommand = command;
     myDelegate = execResult;
     myPrivateData = privateData;
@@ -36,36 +41,80 @@
     return removePrivateData(myDelegate.getStdout(), myPrivateData);
   }
 
+  public void checkCommandFailed() throws VcsException {
+    checkFailure(false);
+  }
+
+  public void checkFailure(boolean failWhenStderrIsNonEmpty) throws VcsException {
+    rethrowDetectedError();
+    if (isFailure())
+      logAndThrowError();
+    String stderr = getStderr();
+    if (!isEmpty(stderr)) {
+      if (failWhenStderrIsNonEmpty)
+        logAndThrowError();
+      else
+        logStderr(stderr);
+    }
+  }
+
+  private void logAndThrowError() throws VcsException {
+    String message = createCommandLogMessage();
+    myLogger.warn(message);
+    if (hasImportantException())
+      myLogger.error("Error during executing '" + getCommand() + "'", getException());
+    throw new VcsException(message);
+  }
+
+  private void logStderr(String stderr) {
+    myLogger.warn("Error output produced by: " + getCommand());
+    myLogger.warn(stderr);
+  }
+
   @NotNull
-  public String getStderr() {
+  private String getStderr() {
     return removePrivateData(myDelegate.getStderr(), myPrivateData);
   }
 
   @Nullable
-  public Throwable getException() {
+  private Throwable getException() {
     return myDelegate.getException();
   }
 
-  public int getExitCode() {
-    return myDelegate.getExitCode();
+  private boolean isFailure() {
+    //A non-zero exit code is not an error:
+    //http://mercurial.selenic.com/bts/issue186
+    //http://mercurial.selenic.com/bts/issue2189
+    //E.g. pull command in hg 2.1 exits with 1 if no new changes were pulled
+    return getException() != null;
+  }
+
+  private boolean shouldDetectErrors() {
+    return isFailure() || myDelegate.getExitCode() != 0;
   }
 
   @NotNull
-  public String getCommand() {
-    return myCommand;
+  private String getCommand() {
+    return removePrivateData(myCommand, myPrivateData);
   }
 
-  public boolean isFailure() {
-    return getExitCode() != 0 || getException() != null;
-  }
-
-  public boolean hasImportantException() {
+  private boolean hasImportantException() {
     Throwable exception = getException();
     return exception instanceof NullPointerException;
   }
 
+  private String createCommandLogMessage() {
+    String stderr = getStderr();
+    String stdout = getStdout();
+    String exceptionMessage = getExceptionMessage();
+    return "'" + getCommand() + "' command failed.\n" +
+            (!StringUtil.isEmpty(stdout) ? "stdout: " + stdout + "\n" : "") +
+            (!StringUtil.isEmpty(stderr) ? "stderr: " + stderr + "\n" : "") +
+            (exceptionMessage != null ? "exception: " + exceptionMessage : "");
+  }
+
   @Nullable
-  public String getExceptionMessage() {
+  private String getExceptionMessage() {
     Throwable exception = getException();
     if (exception == null)
       return null;
@@ -75,12 +124,13 @@
     return message;
   }
 
-  public void rethrowDetectedError() throws VcsException {
-    if (!isFailure())
+  private void rethrowDetectedError() throws VcsException {
+    if (!shouldDetectErrors())
       return;
-    String stderr = getStderr();
+    String stderr = getStderr().trim();
     checkUnrelatedRepository(stderr);
     checkUnknownRevision(stderr);
+    checkFileNotUnderTheRoot(stderr);
   }
 
   private void checkUnrelatedRepository(@NotNull final String stderr) throws UnrelatedRepositoryException {
@@ -99,4 +149,16 @@
     }
   }
 
+  private void checkFileNotUnderTheRoot(@NotNull final String stderr) throws VcsException {
+    final String prefix = "abort: ";
+    int idx = stderr.indexOf("abort: ");
+    if (idx != -1) {
+      int startIdx = idx + prefix.length();
+      int endIdx = stderr.indexOf(" not under root");
+      if (endIdx != -1) {
+        String path = stderr.substring(startIdx, endIdx);
+        throw new UnknownFileException(path);
+      }
+    }
+  }
 }
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CommandUtil.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CommandUtil.java	Fri Feb 03 13:23:18 2012 +0400
@@ -19,71 +19,34 @@
 import jetbrains.buildServer.ExecResult;
 import jetbrains.buildServer.SimpleCommandLineProcessRunner;
 import jetbrains.buildServer.log.Loggers;
-import jetbrains.buildServer.util.StringUtil;
 import jetbrains.buildServer.vcs.VcsException;
 import org.jetbrains.annotations.NotNull;
 
-import java.util.Collections;
 import java.util.Set;
 
+import static jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CommandExecutionSettingsBuilder.with;
+
 public class CommandUtil {
-  private static final int DEFAULT_COMMAND_TIMEOUT_SEC = 3600;
 
-  public static void checkCommandFailed(@NotNull final CommandResult res) throws VcsException {
-    res.rethrowDetectedError();
-    if (res.isFailure())
-      commandFailed(res);
-    if (res.getStderr().length() > 0) {
-      Loggers.VCS.warn("Error output produced by: " + res.getCommand());
-      Loggers.VCS.warn(res.getStderr());
-    }
-  }
-
-  public static void commandFailed(@NotNull final CommandResult res) throws VcsException {
-    final String message = createCommandLogMessage(res);
-    Loggers.VCS.warn(message);
-    if (res.hasImportantException())
-      Loggers.VCS.error("Error during executing '" + res.getCommand() + "'", res.getException());
-    throw new VcsException(message);
+  public static CommandResult runCommand(@NotNull GeneralCommandLine cli) throws VcsException {
+    return runCommand(cli, with());
   }
 
-  private static String createCommandLogMessage(@NotNull final CommandResult res) {
-    String stderr = res.getStderr();
-    String stdout = res.getStdout();
-    String exceptionMessage = res.getExceptionMessage();
-    return "'" + res.getCommand() + "' command failed.\n" +
-            (!StringUtil.isEmpty(stdout) ? "stdout: " + stdout + "\n" : "") +
-            (!StringUtil.isEmpty(stderr) ? "stderr: " + stderr + "\n" : "") +
-            (exceptionMessage != null ? "exception: " + exceptionMessage : "");
-  }
-
-  public static CommandResult runCommand(@NotNull GeneralCommandLine cli) throws VcsException {
-    return runCommand(cli, DEFAULT_COMMAND_TIMEOUT_SEC, Collections.<String>emptySet());
+  public static CommandResult runCommand(@NotNull GeneralCommandLine cli, @NotNull CommandExecutionSettingsBuilder executionSettingsBuilder) throws VcsException {
+    return runCommand(cli, executionSettingsBuilder.build());
   }
 
-  public static CommandResult runCommand(@NotNull GeneralCommandLine cli, @NotNull Set<String> privateData) throws VcsException {
-    return runCommand(cli, DEFAULT_COMMAND_TIMEOUT_SEC, privateData);
-  }
-
-  public static CommandResult runCommand(@NotNull GeneralCommandLine cli, final int executionTimeout, @NotNull Set<String> privateData) throws VcsException {
-    return runCommand(cli, executionTimeout, privateData, true);
-  }
-
-  public static CommandResult runCommand(@NotNull GeneralCommandLine cli, @NotNull Set<String> privateData, final boolean checkFailure) throws VcsException {
-    return runCommand(cli, DEFAULT_COMMAND_TIMEOUT_SEC, privateData, checkFailure);
-  }
-
-  public static CommandResult runCommand(@NotNull GeneralCommandLine cli, final int executionTimeout, @NotNull Set<String> privateData, final boolean checkFailure) throws VcsException {
-    final String cmdStr = removePrivateData(cli.getCommandLineString(), privateData);
-    Loggers.VCS.debug("Run command: " + cmdStr);
-    CommandResult res = run(cli, executionTimeout, cmdStr,privateData);
-    if (checkFailure)
-      CommandUtil.checkCommandFailed(res);
-    Loggers.VCS.debug("Command " + cmdStr + " output:\n" + res.getStdout());
+  private static CommandResult runCommand(@NotNull GeneralCommandLine cli, @NotNull CommandExecutionSettings executionSettings) throws VcsException {
+    final String command = removePrivateData(cli.getCommandLineString(), executionSettings.privateData());
+    Loggers.VCS.debug("Run command: " + command);
+    CommandResult res = run(cli, executionSettings.timeout(), command, executionSettings.privateData());
+    if (executionSettings.shouldCheckForFailure() || executionSettings.shouldFailWithNonEmptyStderr())
+      res.checkFailure(executionSettings.shouldFailWithNonEmptyStderr());
+    Loggers.VCS.debug("Command " + command + " output:\n" + res.getStdout());
     return res;
   }
 
-  private static CommandResult run(@NotNull final GeneralCommandLine cli, final int executionTimeout, @NotNull final String cmdStr, @NotNull final Set<String> privateData) {
+  private static CommandResult run(@NotNull final GeneralCommandLine cli, final int executionTimeout, @NotNull final String command, @NotNull final Set<String> privateData) {
     final long start = System.currentTimeMillis();
     ExecResult res = SimpleCommandLineProcessRunner.runCommand(cli, null, new SimpleCommandLineProcessRunner.RunCommandEventsAdapter() {
       @Override
@@ -93,10 +56,10 @@
       @Override
       public void onProcessFinished(Process ps) {
         long duration = System.currentTimeMillis() - start;
-        Loggers.VCS.debug("Command " + cmdStr + " took " + duration + "ms");
+        Loggers.VCS.debug("Command " + command + " took " + duration + "ms");
       }
     });
-    return new CommandResult(cmdStr, res, privateData);
+    return new CommandResult(Loggers.VCS, command, res, privateData);
   }
 
   public static String removePrivateData(final String str, final Set<String> privateData) {
@@ -105,7 +68,6 @@
       if (data == null || data.length() == 0) continue;
       result = result.replace(data, "******");
     }
-
     return result;
   }
 }
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/IdentifyCommand.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/IdentifyCommand.java	Fri Feb 03 13:23:18 2012 +0400
@@ -21,6 +21,8 @@
 
 import java.io.File;
 
+import static jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CommandExecutionSettingsBuilder.with;
+
 /**
  * @author Pavel.Sher
  *         Date: 16.07.2008
@@ -62,8 +64,7 @@
       cli.addParameter("--rev");
       cli.addParameter(myRevisionNumber.toString());
     }
-    CommandResult res = runCommand(cli);
-    failIfNotEmptyStdErr(res);
+    CommandResult res = runCommand(cli, with().failureWhenStderrNotEmpty());
     String output = res.getStdout().trim();
     return output.contains(" ") ? output.substring(0, output.indexOf(" ")) : output;
   }
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/PullCommand.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/PullCommand.java	Fri Feb 03 13:23:18 2012 +0400
@@ -22,6 +22,7 @@
 import java.io.File;
 
 import static com.intellij.openapi.util.io.FileUtil.delete;
+import static jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CommandExecutionSettingsBuilder.with;
 
 /**
  * @author Pavel.Sher
@@ -45,8 +46,7 @@
     GeneralCommandLine cli = createCommandLine();
     cli.addParameter("pull");
     cli.addParameter(myPullUrl);
-    CommandResult result = CommandUtil.runCommand(cli, timeout, getPrivateData(), false);
-    CommandUtil.checkCommandFailed(result);
+    runCommand(cli, with().timeout(timeout));
   }
 
   private void ensureRepositoryIsNotLocked() {
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/PushCommand.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/PushCommand.java	Fri Feb 03 13:23:18 2012 +0400
@@ -21,6 +21,8 @@
 
 import java.io.File;
 
+import static jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CommandExecutionSettingsBuilder.with;
+
 /**
  * @author pavel
  */
@@ -42,7 +44,6 @@
       cli.addParameter("-f");
     }
     cli.addParameter(getSettings().getRepositoryUrlWithCredentials());
-    CommandResult res = runCommand(cli);
-    failIfNotEmptyStdErr(res);
+    runCommand(cli, with().failureWhenStderrNotEmpty());
   }
 }
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/UnknownFileException.java	Fri Feb 03 13:23:18 2012 +0400
@@ -0,0 +1,22 @@
+package jetbrains.buildServer.buildTriggers.vcs.mercurial.command;
+
+import jetbrains.buildServer.vcs.VcsException;
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * @author dmitry.neverov
+ */
+public class UnknownFileException extends VcsException {
+
+  private final String myPath;
+
+  public UnknownFileException(@NotNull String path) {
+    super("Unknown file " + path);
+    myPath = path;
+  }
+
+  public String getPath() {
+    return myPath;
+  }
+
+}
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/UnknownRevisionException.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/UnknownRevisionException.java	Fri Feb 03 13:23:18 2012 +0400
@@ -11,6 +11,7 @@
   private final String myRevision;
 
   public UnknownRevisionException(@NotNull final String revision) {
+    super("Unknown revision " + revision);
     myRevision = revision;
   }
 
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/UpdateCommand.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/UpdateCommand.java	Fri Feb 03 13:23:18 2012 +0400
@@ -22,6 +22,7 @@
 import java.io.File;
 
 import static com.intellij.openapi.util.io.FileUtil.delete;
+import static jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CommandExecutionSettingsBuilder.with;
 
 public class UpdateCommand extends VcsRootCommand {
 
@@ -48,7 +49,7 @@
     } else {
       cli.addParameter(getSettings().getBranchName());
     }
-    runCommand(cli, UPDATE_TIMEOUT_SECONDS);
+    runCommand(cli, with().timeout(UPDATE_TIMEOUT_SECONDS));
   }
 
   private void ensureWorkingDirIsNotLocked() {
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/VcsRootCommand.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/VcsRootCommand.java	Fri Feb 03 13:23:18 2012 +0400
@@ -8,6 +8,8 @@
 import java.util.Collections;
 import java.util.Set;
 
+import static jetbrains.buildServer.buildTriggers.vcs.mercurial.command.CommandExecutionSettingsBuilder.with;
+
 /**
  * @author dmitry.neverov
  */
@@ -15,7 +17,6 @@
 
   private final Settings mySettings;
 
-
   public VcsRootCommand(@NotNull final Settings settings, @NotNull final File workDir) {
     super(settings.getHgCommandPath(), workDir);
     mySettings = settings;
@@ -26,26 +27,15 @@
     return mySettings;
   }
 
-
-  public Set<String> getPrivateData() {
-    return Collections.singleton(mySettings.getPassword());
-  }
-
-
   protected CommandResult runCommand(@NotNull GeneralCommandLine cli) throws VcsException {
-    return CommandUtil.runCommand(cli, getPrivateData());
+    return CommandUtil.runCommand(cli, with());
   }
 
-
-  protected CommandResult runCommand(@NotNull GeneralCommandLine cli, int executionTimeout) throws VcsException {
-    return CommandUtil.runCommand(cli, executionTimeout, getPrivateData());
+  protected CommandResult runCommand(@NotNull GeneralCommandLine cli, @NotNull CommandExecutionSettingsBuilder with) throws VcsException {
+    return CommandUtil.runCommand(cli, with.privateData(getPrivateData()));
   }
 
-  protected CommandResult runCommand(@NotNull GeneralCommandLine cli, int executionTimeout, boolean checkFailure) throws VcsException {
-    return CommandUtil.runCommand(cli, executionTimeout, getPrivateData(), checkFailure);
-  }
-
-  protected CommandResult runCommand(@NotNull GeneralCommandLine cli, boolean checkFailure) throws VcsException {
-    return CommandUtil.runCommand(cli, getPrivateData(), checkFailure);
+  private Set<String> getPrivateData() {
+    return Collections.singleton(mySettings.getPassword());
   }
 }
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/VersionCommand.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/VersionCommand.java	Fri Feb 03 13:23:18 2012 +0400
@@ -26,7 +26,7 @@
     GeneralCommandLine cli = createCommandLine();
     cli.addParameter("version");
     cli.addParameter("--quiet");
-    CommandResult result = runCommand(cli);
+    CommandResult result = CommandUtil.runCommand(cli);
     return HgVersion.parse(result.getStdout());
   }
 
--- a/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CatCommandTest.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CatCommandTest.java	Fri Feb 03 13:23:18 2012 +0400
@@ -23,14 +23,27 @@
   public void command_should_not_leave_garbage_in_temp_dir() throws IOException, VcsException {
     cleanCatResultDirs();
     setRepository("mercurial-tests/testData/rep1", true);
+    final String nonExisting = "/non/existing/path";
     try {
-      runCat(asList("/non/existing/path"));
+      runCat(asList(nonExisting));
       fail("exception should be thrown for non-existing path");
-    } catch (VcsException e) {
+    } catch (UnknownFileException e) {
+      assertEquals(nonExisting, e.getPath());
       checkTempDirDoesNotContainCatResults();
     }
   }
 
+  public void should_not_throw_exception_if_not_asked_to() throws IOException, VcsException {
+    cleanCatResultDirs();
+    setRepository("mercurial-tests/testData/rep1", true);
+    runCommand(new CommandExecutor<File>() {
+      public File execute(@NotNull final Settings settings, @NotNull final File workingDir) throws VcsException {
+        CatCommand cat = new CatCommand(settings, workingDir);
+        return cat.execute(asList("/non/existing/path"), false);
+      }
+    });
+  }
+
   private void cleanCatResultDirs() {
     for (File f : getCatResultDirs())
       delete(f);
--- a/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CommandResultTest.java	Fri Feb 03 13:23:18 2012 +0400
+++ b/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/CommandResultTest.java	Fri Feb 03 13:23:18 2012 +0400
@@ -1,14 +1,20 @@
 package jetbrains.buildServer.buildTriggers.vcs.mercurial.command;
 
+import com.intellij.openapi.diagnostic.Logger;
 import jetbrains.buildServer.ExecResult;
 import jetbrains.buildServer.StreamGobbler;
 import jetbrains.buildServer.vcs.VcsException;
+import org.apache.log4j.Level;
+import org.jetbrains.annotations.NonNls;
 import org.jetbrains.annotations.NotNull;
+import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 import java.io.ByteArrayInputStream;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
+import java.util.List;
 
 import static org.testng.AssertJUnit.*;
 
@@ -18,18 +24,25 @@
 @Test
 public class CommandResultTest {
 
+  private RecordingLogger myLogger;
+
+  @BeforeMethod
+  public void setUp() {
+    myLogger = new RecordingLogger();
+  }
+
   public void output_should_not_contain_private_data() {
     String password = "pass";
     CommandResult commandResult = commandResultFor(execResult().withStdout(password).withStderr(password), password);
     assertFalse(commandResult.getStdout().contains(password));
-    assertFalse(commandResult.getStderr().contains(password));
+    myLogger.assertLogMessagesDontContain(password);
   }
 
   @Test(expectedExceptions = UnrelatedRepositoryException.class)
   public void should_detect_unrelated_repository_error() throws VcsException {
     String unrelatedRepositoryStderr = "abort: repository is unrelated\n";
     CommandResult commandResult = commandResultFor(execResult().withStderr(unrelatedRepositoryStderr).withExitCode(255));
-    commandResult.rethrowDetectedError();
+    commandResult.checkCommandFailed();
   }
 
   @Test
@@ -38,21 +51,22 @@
     String unknownRevisionError = "abort: unknown revision '" + unknownRevision + "'\n";
     CommandResult commandResult = commandResultFor(execResult().withStderr(unknownRevisionError).withExitCode(255));
     try {
-      commandResult.rethrowDetectedError();
+      commandResult.checkCommandFailed();
       fail("unknown exception should be thrown");
     } catch (UnknownRevisionException e) {
       assertEquals(unknownRevision, e.getRevision());
     }
   }
 
-  public void should_detect_failure_when_delegate_has_exception() {
+  @Test(expectedExceptions = VcsException.class)
+  public void should_detect_failure_when_delegate_has_exception() throws VcsException {
     CommandResult commandResult = commandResultFor(execResult().withException(new RuntimeException()));
-    assertTrue(commandResult.isFailure());
+    commandResult.checkCommandFailed();
   }
 
-  public void should_detect_failure_with_non_zero_exit_code() {
+  public void should_detect_failure_with_non_zero_exit_code() throws VcsException {
     CommandResult commandResult = commandResultFor(execResult().withExitCode(1));
-    assertTrue(commandResult.isFailure());
+    commandResult.checkCommandFailed();
   }
 
 
@@ -61,7 +75,7 @@
   }
 
   CommandResult commandResultFor(ExecResultBuilder builder, String... privateData) {
-    return new CommandResult("", builder.build(), new HashSet<String>(Arrays.asList(privateData)));
+    return new CommandResult(myLogger, "", builder.build(), new HashSet<String>(Arrays.asList(privateData)));
   }
 
   private class ExecResultBuilder {
@@ -109,4 +123,60 @@
       return gobbler;
     }
   }
+
+  private class RecordingLogger extends Logger {
+
+    private List<String> myMessages = new ArrayList<String>();
+
+    public void assertLogMessagesDontContain(@NotNull String... strs) {
+      for (String s : strs) {
+        for (String message : myMessages)
+          assertFalse("'" + s + "' was logged", message.contains(s));
+      }
+    }
+
+    @Override
+    public boolean isDebugEnabled() {
+      return true;
+    }
+
+    @Override
+    public void debug(@NonNls String s) {
+      myMessages.add(s);
+    }
+
+    @Override
+    public void debug(Throwable throwable) {
+      myMessages.add(throwable.getMessage());
+    }
+
+    @Override
+    public void debug(@NonNls String s, Throwable throwable) {
+      myMessages.add(s);
+    }
+
+    @Override
+    public void error(@NonNls String s, Throwable throwable, @NonNls String... strings) {
+      myMessages.add(s);
+    }
+
+    @Override
+    public void info(@NonNls String s) {
+      myMessages.add(s);
+    }
+
+    @Override
+    public void info(@NonNls String s, Throwable throwable) {
+      myMessages.add(s);
+    }
+
+    @Override
+    public void warn(@NonNls String s, Throwable throwable) {
+      myMessages.add(s);
+    }
+
+    @Override
+    public void setLevel(Level level) {
+    }
+  }
 }