Don't treat command termination due '-h' option as a fatal error

Signal early command termination due '-h' or '--help' option via
TerminatedByHelpException. This allows tests using
CLIGitCommand differentiate between unexpected command parsing errors
and expected command cancellation "on help" (which also allows
validation of expected/unexpected help messages).

Additional side-effect: jgit supports now git style of handling help
option: any unexpected command line options before help are reported as
errors, but after help ignored.

Bug: 484951
Change-Id: If45c41c0d32895ab6822a7ff9d851877dcef5771
Signed-off-by: Andrey Loskutov <loskutov@gmx.de>

2 files changed, 51 insertions, 2 deletions

diff --git a/org.eclipse.jgit.pgm.test/src/org/eclipse/jgit/pgm/CLIGitCommand.java b/org.eclipse.jgit.pgm.test/src/org/eclipse/jgit/pgm/CLIGitCommand.java
index bf15fed812..7d2cbca729 100644
--- a/org.eclipse.jgit.pgm.test/src/org/eclipse/jgit/pgm/CLIGitCommand.java
+++ b/org.eclipse.jgit.pgm.test/src/org/eclipse/jgit/pgm/CLIGitCommand.java
@@ -50,6 +50,7 @@ import java.util.List;
 
 import org.eclipse.jgit.internal.storage.file.FileRepository;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.pgm.TextBuiltin.TerminatedByHelpException;
 import org.eclipse.jgit.pgm.internal.CLIText;
 import org.eclipse.jgit.pgm.opt.CmdLineParser;
 import org.eclipse.jgit.pgm.opt.SubcommandHandler;
@@ -120,12 +121,15 @@ public class CLIGitCommand {
 		System.arraycopy(args, 1, argv, 0, args.length - 1);
 
 		CLIGitCommand bean = new CLIGitCommand();
-		final CmdLineParser clp = new CmdLineParser(bean);
+		final CmdLineParser clp = new TestCmdLineParser(bean);
 		clp.parseArgument(argv);
 
 		final TextBuiltin cmd = bean.getSubcommand();
 		ByteArrayOutputStream baos = new ByteArrayOutputStream();
 		cmd.outs = baos;
+		ByteArrayOutputStream errs = new ByteArrayOutputStream();
+		cmd.errs = errs;
+		boolean seenHelp = TextBuiltin.containsHelp(argv);
 		if (cmd.requiresRepository())
 			cmd.init(db, null);
 		else
@@ -133,9 +137,22 @@ public class CLIGitCommand {
 		try {
 			cmd.execute(bean.getArguments().toArray(
 					new String[bean.getArguments().size()]));
+		} catch (TerminatedByHelpException e) {
+			seenHelp = true;
+			// this is not a failure, command execution should just not happen
 		} finally {
-			if (cmd.outw != null)
+			if (cmd.outw != null) {
 				cmd.outw.flush();
+			}
+			if (cmd.errw != null) {
+				cmd.errw.flush();
+			}
+			if (seenHelp) {
+				return errs.toByteArray();
+			} else if (errs.size() > 0) {
+				// forward the errors to the standard err
+				System.err.print(errs.toString());
+			}
 		}
 		return baos.toByteArray();
 	}
@@ -195,4 +212,14 @@ public class CLIGitCommand {
 		return list.toArray(new String[list.size()]);
 	}
 
+	static class TestCmdLineParser extends CmdLineParser {
+		public TestCmdLineParser(Object bean) {
+			super(bean);
+		}
+
+		@Override
+		protected boolean containsHelp(String... args) {
+			return false;
+		}
+	}
 }
diff --git a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/DescribeTest.java b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/DescribeTest.java
index 6352a26524..1c8ba0c2f6 100644
--- a/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/DescribeTest.java
+++ b/org.eclipse.jgit.pgm.test/tst/org/eclipse/jgit/pgm/DescribeTest.java
@@ -43,6 +43,10 @@
 package org.eclipse.jgit.pgm;
 
 import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Arrays;
 
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.lib.CLIRepositoryTestCase;
@@ -103,4 +107,22 @@ public class DescribeTest extends CLIRepositoryTestCase {
 		assertArrayEquals(new String[] { "v1.0-0-g6fd41be", "" },
 				execute("git describe --long HEAD"));
 	}
+
+	@Test
+	public void testHelpArgumentBeforeUnknown() throws Exception {
+		String[] output = execute("git describe -h -XYZ");
+		String all = Arrays.toString(output);
+		assertTrue("Unexpected help output: " + all,
+				all.contains("jgit describe"));
+		assertFalse("Unexpected help output: " + all, all.contains("fatal"));
+	}
+
+	@Test
+	public void testHelpArgumentAfterUnknown() throws Exception {
+		String[] output = execute("git describe -XYZ -h");
+		String all = Arrays.toString(output);
+		assertTrue("Unexpected help output: " + all,
+				all.contains("jgit describe"));
+		assertTrue("Unexpected help output: " + all, all.contains("fatal"));
+	}
 }