diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index cb6a6a1011a..674b090e97e 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -2066,6 +2066,9 @@ Release 2.8.0 - UNRELEASED HDFS-9187. Fix null pointer error in Globber when FS was not constructed via FileSystem#createFileSystem (cmccabe) + HDFS-9157. [OEV and OIV] : Unnecessary parsing for mandatory arguements if + '-h' option is specified as the only option (nijel via vinayakumarb) + Release 2.7.2 - UNRELEASED INCOMPATIBLE CHANGES diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineEditsViewer/OfflineEditsViewer.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineEditsViewer/OfflineEditsViewer.java index 54b8511526f..107881ff590 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineEditsViewer/OfflineEditsViewer.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineEditsViewer/OfflineEditsViewer.java @@ -39,7 +39,8 @@ import org.apache.commons.cli.PosixParser; @InterfaceAudience.Private @InterfaceStability.Unstable public class OfflineEditsViewer extends Configured implements Tool { - + private static final String HELP_OPT = "-h"; + private static final String HELP_LONGOPT = "--help"; private final static String defaultProcessor = "xml"; /** @@ -192,7 +193,12 @@ public class OfflineEditsViewer extends Configured implements Tool { Options options = buildOptions(); if(argv.length == 0) { printHelp(); - return -1; + return 0; + } + // print help and exit with zero exit code + if (argv.length == 1 && isHelpOption(argv[0])) { + printHelp(); + return 0; } CommandLineParser parser = new PosixParser(); CommandLine cmd; @@ -205,7 +211,9 @@ public class OfflineEditsViewer extends Configured implements Tool { return -1; } - if(cmd.hasOption("h")) { // print help and exit + if (cmd.hasOption("h")) { + // print help and exit with non zero exit code since + // it is not expected to give help and other options together. printHelp(); return -1; } @@ -237,4 +245,9 @@ public class OfflineEditsViewer extends Configured implements Tool { int res = ToolRunner.run(new OfflineEditsViewer(), argv); System.exit(res); } + + private static boolean isHelpOption(String arg) { + return arg.equalsIgnoreCase(HELP_OPT) || + arg.equalsIgnoreCase(HELP_LONGOPT); + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/OfflineImageViewerPB.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/OfflineImageViewerPB.java index 777acdf9b18..4afbdb8a934 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/OfflineImageViewerPB.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/OfflineImageViewerPB.java @@ -41,6 +41,8 @@ import org.apache.hadoop.net.NetUtils; */ @InterfaceAudience.Private public class OfflineImageViewerPB { + private static final String HELP_OPT = "-h"; + private static final String HELP_LONGOPT = "--help"; public static final Log LOG = LogFactory.getLog(OfflineImageViewerPB.class); private final static String usage = "Usage: bin/hdfs oiv [OPTIONS] -i INPUTFILE -o OUTPUTFILE\n" @@ -131,7 +133,11 @@ public class OfflineImageViewerPB { printUsage(); return 0; } - + // print help and exit with zero exit code + if (args.length == 1 && isHelpOption(args[0])) { + printUsage(); + return 0; + } CommandLineParser parser = new PosixParser(); CommandLine cmd; @@ -143,9 +149,11 @@ public class OfflineImageViewerPB { return -1; } - if (cmd.hasOption("h")) { // print help and exit + if (cmd.hasOption("h")) { + // print help and exit with non zero exit code since + // it is not expected to give help and other options together. printUsage(); - return 0; + return -1; } String inputFile = cmd.getOptionValue("i"); @@ -202,4 +210,9 @@ public class OfflineImageViewerPB { private static void printUsage() { System.out.println(usage); } + + private static boolean isHelpOption(String arg) { + return arg.equalsIgnoreCase(HELP_OPT) || + arg.equalsIgnoreCase(HELP_LONGOPT); + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineEditsViewer/TestOfflineEditsViewer.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineEditsViewer/TestOfflineEditsViewer.java index fbbbc292973..c5bc7210bc8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineEditsViewer/TestOfflineEditsViewer.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineEditsViewer/TestOfflineEditsViewer.java @@ -21,9 +21,11 @@ package org.apache.hadoop.hdfs.tools.offlineEditsViewer; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; +import java.io.PrintStream; import java.nio.ByteBuffer; import java.nio.channels.FileChannel; @@ -35,8 +37,10 @@ import org.apache.hadoop.hdfs.server.namenode.FSEditLogOpCodes; import org.apache.hadoop.hdfs.server.namenode.NameNodeLayoutVersion; import org.apache.hadoop.hdfs.server.namenode.OfflineEditsViewerHelper; import org.apache.hadoop.hdfs.tools.offlineEditsViewer.OfflineEditsViewer.Flags; +import org.apache.hadoop.io.IOUtils; import org.apache.hadoop.test.PathUtils; import org.junit.After; +import org.junit.Assert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -285,4 +289,23 @@ public class TestOfflineEditsViewer { return true; } + + @Test + public void testOfflineEditsViewerHelpMessage() throws Throwable { + final ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + final PrintStream out = new PrintStream(bytes); + final PrintStream oldOut = System.out; + try { + System.setOut(out); + int status = new OfflineEditsViewer().run(new String[] { "-h" }); + assertTrue("" + "Exit code returned for help option is incorrect", + status == 0); + Assert.assertFalse( + "Invalid Command error displayed when help option is passed.", bytes + .toString().contains("Error parsing command-line options")); + } finally { + System.setOut(oldOut); + IOUtils.closeStream(out); + } + } } diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java index e1beca50d48..467534aa363 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/offlineImageViewer/TestOfflineImageViewer.java @@ -67,6 +67,7 @@ import org.apache.hadoop.io.IOUtils; import org.apache.hadoop.net.NetUtils; import org.apache.hadoop.security.token.Token; import org.junit.AfterClass; +import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; @@ -350,6 +351,30 @@ public class TestOfflineImageViewer { status != 0); } + @Test + public void testOfflineImageViewerHelpMessage() throws Throwable { + final ByteArrayOutputStream bytes = new ByteArrayOutputStream(); + final PrintStream out = new PrintStream(bytes); + final PrintStream oldOut = System.out; + try { + System.setOut(out); + int status = OfflineImageViewerPB.run(new String[] { "-h" }); + assertTrue("Exit code returned for help option is incorrect", status == 0); + Assert.assertFalse( + "Invalid Command error displayed when help option is passed.", bytes + .toString().contains("Error parsing command-line options")); + status = + OfflineImageViewerPB.run(new String[] { "-h", "-i", + originalFsimage.getAbsolutePath(), "-o", "-", "-p", + "FileDistribution", "-maxSize", "512", "-step", "8" }); + Assert.assertTrue( + "Exit code returned for help with other option is incorrect", + status == -1); + } finally { + System.setOut(oldOut); + IOUtils.closeStream(out); + } + } private void testPBDelimitedWriter(String db) throws IOException, InterruptedException { final String DELIMITER = "\t";