You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-commits@hadoop.apache.org by to...@apache.org on 2011/06/06 22:53:37 UTC

svn commit: r1132764 - in /hadoop/common/trunk: ./ src/java/org/apache/hadoop/fs/ src/java/org/apache/hadoop/fs/shell/ src/test/core/org/apache/hadoop/cli/

Author: todd
Date: Mon Jun  6 20:53:37 2011
New Revision: 1132764

URL: http://svn.apache.org/viewvc?rev=1132764&view=rev
Log:
HADOOP-7353. Cleanup FsShell and prevent masking of RTE stack traces. Contributed by Daryn Sharp.

Modified:
    hadoop/common/trunk/CHANGES.txt
    hadoop/common/trunk/src/java/org/apache/hadoop/fs/FsShell.java
    hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Command.java
    hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CommandFactory.java
    hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Count.java
    hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Display.java
    hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/FsCommand.java
    hadoop/common/trunk/src/test/core/org/apache/hadoop/cli/testConf.xml

Modified: hadoop/common/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/CHANGES.txt?rev=1132764&r1=1132763&r2=1132764&view=diff
==============================================================================
--- hadoop/common/trunk/CHANGES.txt (original)
+++ hadoop/common/trunk/CHANGES.txt Mon Jun  6 20:53:37 2011
@@ -280,6 +280,9 @@ Trunk (unreleased changes)
 
     HADOOP-7341. Fix options parsing in CommandFormat (Daryn Sharp via todd)
 
+    HADOOP-7353. Cleanup FsShell and prevent masking of RTE stack traces.
+    (Daryn Sharp via todd)
+
 Release 0.22.0 - Unreleased
 
   INCOMPATIBLE CHANGES

Modified: hadoop/common/trunk/src/java/org/apache/hadoop/fs/FsShell.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/java/org/apache/hadoop/fs/FsShell.java?rev=1132764&r1=1132763&r2=1132764&view=diff
==============================================================================
--- hadoop/common/trunk/src/java/org/apache/hadoop/fs/FsShell.java (original)
+++ hadoop/common/trunk/src/java/org/apache/hadoop/fs/FsShell.java Mon Jun  6 20:53:37 2011
@@ -18,9 +18,10 @@
 package org.apache.hadoop.fs;
 
 import java.io.IOException;
+import java.io.PrintStream;
+import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
+import java.util.LinkedList;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -30,9 +31,6 @@ import org.apache.hadoop.conf.Configured
 import org.apache.hadoop.fs.shell.Command;
 import org.apache.hadoop.fs.shell.CommandFactory;
 import org.apache.hadoop.fs.shell.FsCommand;
-import org.apache.hadoop.fs.shell.PathExceptions.PathNotFoundException;
-import org.apache.hadoop.ipc.RPC;
-import org.apache.hadoop.util.StringUtils;
 import org.apache.hadoop.util.Tool;
 import org.apache.hadoop.util.ToolRunner;
 
@@ -46,23 +44,31 @@ public class FsShell extends Configured 
   private Trash trash;
   protected CommandFactory commandFactory;
 
+  private final String usagePrefix =
+    "Usage: hadoop fs [generic options]";
+
   /**
+   * Default ctor with no configuration.  Be sure to invoke
+   * {@link #setConf(Configuration)} with a valid configuration prior
+   * to running commands.
    */
   public FsShell() {
     this(null);
   }
 
+  /**
+   * Construct a FsShell with the given configuration.  Commands can be
+   * executed via {@link #run(String[])}
+   * @param conf the hadoop configuration
+   */
   public FsShell(Configuration conf) {
     super(conf);
-    fs = null;
-    trash = null;
-    commandFactory = new CommandFactory();
   }
   
   protected FileSystem getFS() throws IOException {
-    if(fs == null)
+    if (fs == null) {
       fs = FileSystem.get(getConf());
-    
+    }
     return fs;
   }
   
@@ -75,93 +81,145 @@ public class FsShell extends Configured 
   
   protected void init() throws IOException {
     getConf().setQuietMode(true);
+    if (commandFactory == null) {
+      commandFactory = new CommandFactory(getConf());
+      commandFactory.addObject(new Help(), "-help");
+      commandFactory.addObject(new Usage(), "-usage");
+      registerCommands(commandFactory);
+    }
   }
 
+  protected void registerCommands(CommandFactory factory) {
+    // TODO: DFSAdmin subclasses FsShell so need to protect the command
+    // registration.  This class should morph into a base class for
+    // commands, and then this method can be abstract
+    if (this.getClass().equals(FsShell.class)) {
+      factory.registerCommands(FsCommand.class);
+    }
+  }
+  
   /**
    * Returns the Trash object associated with this shell.
+   * @return Path to the trash
+   * @throws IOException upon error
    */
   public Path getCurrentTrashDir() throws IOException {
     return getTrash().getCurrentTrashDir();
   }
 
+  // NOTE: Usage/Help are inner classes to allow access to outer methods
+  // that access commandFactory
+  
   /**
-   * Return an abbreviated English-language desc of the byte length
-   * @deprecated Consider using {@link org.apache.hadoop.util.StringUtils#byteDesc} instead.
+   *  Display help for commands with their short usage and long description
    */
-  @Deprecated
-  public static String byteDesc(long len) {
-    return StringUtils.byteDesc(len);
-  }
+   protected class Usage extends FsCommand {
+    public static final String NAME = "usage";
+    public static final String USAGE = "[cmd ...]";
+    public static final String DESCRIPTION =
+      "Displays the usage for given command or all commands if none\n" +
+      "is specified.";
+    
+    @Override
+    protected void processRawArguments(LinkedList<String> args) {
+      if (args.isEmpty()) {
+        printUsage(System.out);
+      } else {
+        for (String arg : args) printUsage(System.out, arg);
+      }
+    }
+  } 
 
   /**
-   * @deprecated Consider using {@link org.apache.hadoop.util.StringUtils#limitDecimalTo2} instead.
+   * Displays short usage of commands sans the long description
    */
-  @Deprecated
-  public static synchronized String limitDecimalTo2(double d) {
-    return StringUtils.limitDecimalTo2(d);
+  protected class Help extends FsCommand {
+    public static final String NAME = "help";
+    public static final String USAGE = "[cmd ...]";
+    public static final String DESCRIPTION =
+      "Displays help for given command or all commands if none\n" +
+      "is specified.";
+    
+    @Override
+    protected void processRawArguments(LinkedList<String> args) {
+      if (args.isEmpty()) {
+        printHelp(System.out);
+      } else {
+        for (String arg : args) printHelp(System.out, arg);
+      }
+    }
   }
 
-  private void printHelp(String cmd) {
-    String summary = "hadoop fs is the command to execute fs commands. " +
-      "The full syntax is: \n\n" +
-      "hadoop fs [-fs <local | file system URI>] [-conf <configuration file>]\n\t" +
-      "[-D <property=value>]\n\t" +
-      "[-report]";
-
-    String conf ="-conf <configuration file>:  Specify an application configuration file.";
- 
-    String D = "-D <property=value>:  Use value for given property.";
+  /*
+   * The following are helper methods for getInfo().  They are defined
+   * outside of the scope of the Help/Usage class because the run() method
+   * needs to invoke them too. 
+   */
+
+  // print all usages
+  private void printUsage(PrintStream out) {
+    printInfo(out, null, false);
+  }
   
-    String fs = "-fs [local | <file system URI>]: \tSpecify the file system to use.\n" + 
-      "\t\tIf not specified, the current configuration is used, \n" +
-      "\t\ttaken from the following, in increasing precedence: \n" + 
-      "\t\t\tcore-default.xml inside the hadoop jar file \n" +
-      "\t\t\tcore-site.xml in $HADOOP_CONF_DIR \n" +
-      "\t\t'local' means use the local file system as your DFS. \n" +
-      "\t\t<file system URI> specifies a particular file system to \n" +
-      "\t\tcontact. This argument is optional but if used must appear\n" +
-      "\t\tappear first on the command line.  Exactly one additional\n" +
-      "\t\targument must be specified. \n";
-
-    String help = "-help [cmd]: \tDisplays help for given command or all commands if none\n" +
-      "\t\tis specified.\n";
-
-    Command instance = commandFactory.getInstance("-" + cmd);
-    if (instance != null) {
-      printHelp(instance);
-    } else if ("fs".equals(cmd)) {
-      System.out.println(fs);
-    } else if ("conf".equals(cmd)) {
-      System.out.println(conf);
-    } else if ("D".equals(cmd)) {
-      System.out.println(D);
-    } else if ("help".equals(cmd)) {
-      System.out.println(help);
+  // print one usage
+  private void printUsage(PrintStream out, String cmd) {
+    printInfo(out, cmd, false);
+  }
+
+  // print all helps
+  private void printHelp(PrintStream out) {
+    printInfo(out, null, true);
+  }
+
+  // print one help
+  private void printHelp(PrintStream out, String cmd) {
+    printInfo(out, cmd, true);
+  }
+
+  private void printInfo(PrintStream out, String cmd, boolean showHelp) {
+    if (cmd != null) {
+      // display help or usage for one command
+      Command instance = commandFactory.getInstance("-" + cmd);
+      if (instance == null) {
+        throw new UnknownCommandException(cmd);
+      }
+      if (showHelp) {
+        printInstanceHelp(out, instance);
+      } else {
+        printInstanceUsage(out, instance);
+      }
     } else {
-      System.out.println(summary);
-      for (String thisCmdName : commandFactory.getNames()) {
-        instance = commandFactory.getInstance(thisCmdName);
+      // display help or usage for all commands 
+      out.println(usagePrefix);
+      
+      // display list of short usages
+      ArrayList<Command> instances = new ArrayList<Command>();
+      for (String name : commandFactory.getNames()) {
+        Command instance = commandFactory.getInstance(name);
         if (!instance.isDeprecated()) {
           System.out.println("\t[" + instance.getUsage() + "]");
+          instances.add(instance);
         }
       }
-      System.out.println("\t[-help [cmd]]\n");
-      
-      System.out.println(fs);
-
-      for (String thisCmdName : commandFactory.getNames()) {
-        instance = commandFactory.getInstance(thisCmdName);
-        if (!instance.isDeprecated()) {
-          printHelp(instance);
+      // display long descriptions for each command
+      if (showHelp) {
+        for (Command instance : instances) {
+          out.println();
+          printInstanceHelp(out, instance);
         }
       }
-      System.out.println(help);
-    }        
+      out.println();
+      ToolRunner.printGenericCommandUsage(out);
+    }
+  }
+
+  private void printInstanceUsage(PrintStream out, Command instance) {
+    out.println(usagePrefix + " " + instance.getUsage());
   }
 
   // TODO: will eventually auto-wrap the text, but this matches the expected
   // output for the hdfs tests...
-  private void printHelp(Command instance) {
+  private void printInstanceHelp(PrintStream out, Command instance) {
     boolean firstLine = true;
     for (String line : instance.getDescription().split("\n")) {
       String prefix;
@@ -174,120 +232,51 @@ public class FsShell extends Configured 
       System.out.println(prefix + line);
     }    
   }
-  
-  /**
-   * Displays format of commands.
-   * 
-   */
-  private void printUsage(String cmd) {
-    String prefix = "Usage: java " + FsShell.class.getSimpleName();
-
-    Command instance = commandFactory.getInstance(cmd);
-    if (instance != null) {
-      System.err.println(prefix + " [" + instance.getUsage() + "]");
-    } else if ("-fs".equals(cmd)) {
-      System.err.println("Usage: java FsShell" + 
-                         " [-fs <local | file system URI>]");
-    } else if ("-conf".equals(cmd)) {
-      System.err.println("Usage: java FsShell" + 
-                         " [-conf <configuration file>]");
-    } else if ("-D".equals(cmd)) {
-      System.err.println("Usage: java FsShell" + 
-                         " [-D <[property=value>]");
-    } else {
-      System.err.println("Usage: java FsShell");
-      for (String name : commandFactory.getNames()) {
-        instance = commandFactory.getInstance(name);
-        if (!instance.isDeprecated()) {
-          System.err.println("           [" + instance.getUsage() + "]");
-        }
-      }
-      System.err.println("           [-help [cmd]]");
-      System.err.println();
-      ToolRunner.printGenericCommandUsage(System.err);
-    }
-  }
 
   /**
    * run
    */
   public int run(String argv[]) throws Exception {
-    // TODO: This isn't the best place, but this class is being abused with
-    // subclasses which of course override this method.  There really needs
-    // to be a better base class for all commands
-    commandFactory.setConf(getConf());
-    commandFactory.registerCommands(FsCommand.class);
-    
-    if (argv.length < 1) {
-      printUsage(""); 
-      return -1;
-    }
-
-    int exitCode = -1;
-    int i = 0;
-    String cmd = argv[i++];
     // initialize FsShell
-    try {
-      init();
-    } catch (RPC.VersionMismatch v) {
-      LOG.debug("Version mismatch", v);
-      System.err.println("Version Mismatch between client and server" +
-                         "... command aborted.");
-      return exitCode;
-    } catch (IOException e) {
-      LOG.debug("Error", e);
-      System.err.println("Bad connection to FS. Command aborted. Exception: " +
-          e.getLocalizedMessage());
-      return exitCode;
-    }
+    init();
 
-    try {
-      Command instance = commandFactory.getInstance(cmd);
-      if (instance != null) {
-        exitCode = instance.run(Arrays.copyOfRange(argv, i, argv.length));
-      } else if ("-help".equals(cmd)) {
-        if (i < argv.length) {
-          printHelp(argv[i]);
-        } else {
-          printHelp("");
+    int exitCode = -1;
+    if (argv.length < 1) {
+      printUsage(System.err);
+    } else {
+      String cmd = argv[0];
+      Command instance = null;
+      try {
+        instance = commandFactory.getInstance(cmd);
+        if (instance == null) {
+          throw new UnknownCommandException();
         }
-      } else {
-        System.err.println(cmd + ": Unknown command");
-        printUsage("");
-      }
-    } catch (Exception e) {
-      exitCode = 1;
-      LOG.debug("Error", e);
-      displayError(cmd, e);
-      if (e instanceof IllegalArgumentException) {
-        exitCode = -1;
-        printUsage(cmd);
+        exitCode = instance.run(Arrays.copyOfRange(argv, 1, argv.length));
+      } catch (IllegalArgumentException e) {
+        displayError(cmd, e.getLocalizedMessage());
+        if (instance != null) {
+          printInstanceUsage(System.err, instance);
+        }
+      } catch (Exception e) {
+        // instance.run catches IOE, so something is REALLY wrong if here
+        LOG.debug("Error", e);
+        displayError(cmd, "Fatal internal error");
+        e.printStackTrace(System.err);
       }
     }
     return exitCode;
   }
-
-  // TODO: this is a quick workaround to accelerate the integration of
-  // redesigned commands.  this will be removed this once all commands are
-  // converted.  this change will avoid having to change the hdfs tests
-  // every time a command is converted to use path-based exceptions
-  private static Pattern[] fnfPatterns = {
-    Pattern.compile("File (.*) does not exist\\."),
-    Pattern.compile("File does not exist: (.*)"),
-    Pattern.compile("`(.*)': specified destination directory doest not exist")
-  };
-  private void displayError(String cmd, Exception e) {
-    String message = e.getLocalizedMessage().split("\n")[0];
-    for (Pattern pattern : fnfPatterns) {
-      Matcher matcher = pattern.matcher(message);
-      if (matcher.matches()) {
-        message = new PathNotFoundException(matcher.group(1)).getMessage();
-        break;
-      }
+  
+  private void displayError(String cmd, String message) {
+    for (String line : message.split("\n")) {
+      System.err.println(cmd.substring(1) + ": " + line);
     }
-    System.err.println(cmd.substring(1) + ": " + message);  
   }
   
+  /**
+   *  Performs any necessary cleanup
+   * @throws IOException upon error
+   */
   public void close() throws IOException {
     if (fs != null) {
       fs.close();
@@ -297,9 +286,11 @@ public class FsShell extends Configured 
 
   /**
    * main() has some simple utility methods
+   * @param argv the command and its arguments
+   * @throws Exception upon error
    */
   public static void main(String argv[]) throws Exception {
-    FsShell shell = new FsShell();
+    FsShell shell = newShellInstance();
     int res;
     try {
       res = ToolRunner.run(shell, argv);
@@ -308,4 +299,26 @@ public class FsShell extends Configured 
     }
     System.exit(res);
   }
+
+  // TODO: this should be abstract in a base class
+  protected static FsShell newShellInstance() {
+    return new FsShell();
+  }
+  
+  /**
+   * The default ctor signals that the command being executed does not exist,
+   * while other ctor signals that a specific command does not exist.  The
+   * latter is used by commands that process other commands, ex. -usage/-help
+   */
+  @SuppressWarnings("serial")
+  static class UnknownCommandException extends IllegalArgumentException {
+    private final String cmd;    
+    UnknownCommandException() { this(null); }
+    UnknownCommandException(String cmd) { this.cmd = cmd; }
+    
+    @Override
+    public String getMessage() {
+      return ((cmd != null) ? "`"+cmd+"': " : "") + "Unknown command";
+    }
+  }
 }

Modified: hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Command.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Command.java?rev=1132764&r1=1132763&r2=1132764&view=diff
==============================================================================
--- hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Command.java (original)
+++ hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Command.java Mon Jun  6 20:53:37 2011
@@ -42,6 +42,13 @@ import org.apache.hadoop.util.StringUtil
 @InterfaceStability.Evolving
 
 abstract public class Command extends Configured {
+  /** default name of the command */
+  public static String NAME;
+  /** the command's usage switches and arguments format */
+  public static String USAGE;
+  /** the command's long description */
+  public static String DESCRIPTION;
+    
   protected String[] args;
   protected String name;
   protected int exitCode = 0;
@@ -70,14 +77,6 @@ abstract public class Command extends Co
   /** @return the command's name excluding the leading character - */
   abstract public String getCommandName();
   
-  /**
-   * Name the command
-   * @param cmdName as invoked
-   */
-  public void setCommandName(String cmdName) {
-    name = cmdName;
-  }
-  
   protected void setRecursive(boolean flag) {
     recursive = flag;
   }
@@ -120,14 +119,16 @@ abstract public class Command extends Co
    * expand arguments, and then process each argument.
    * <pre>
    * run
-   * \-> {@link #processOptions(LinkedList)}
-   * \-> {@link #expandArguments(LinkedList)} -> {@link #expandArgument(String)}*
-   * \-> {@link #processArguments(LinkedList)}
-   *     \-> {@link #processArgument(PathData)}*
-   *         \-> {@link #processPathArgument(PathData)}
-   *             \-> {@link #processPaths(PathData, PathData...)}
-   *                 \-> {@link #processPath(PathData)}*
-   *         \-> {@link #processNonexistentPath(PathData)}
+   * |-> {@link #processOptions(LinkedList)}
+   * \-> {@link #processRawArguments(LinkedList)}
+   *      |-> {@link #expandArguments(LinkedList)}
+   *      |   \-> {@link #expandArgument(String)}*
+   *      \-> {@link #processArguments(LinkedList)}
+   *          |-> {@link #processArgument(PathData)}*
+   *          |   |-> {@link #processPathArgument(PathData)}
+   *          |   \-> {@link #processPaths(PathData, PathData...)}
+   *          |        \-> {@link #processPath(PathData)}*
+   *          \-> {@link #processNonexistentPath(PathData)}
    * </pre>
    * Most commands will chose to implement just
    * {@link #processOptions(LinkedList)} and {@link #processPath(PathData)}
@@ -144,7 +145,7 @@ abstract public class Command extends Co
             "DEPRECATED: Please use '"+ getReplacementCommand() + "' instead.");
       }
       processOptions(args);
-      processArguments(expandArguments(args));
+      processRawArguments(args);
     } catch (IOException e) {
       displayError(e);
     }
@@ -171,6 +172,19 @@ abstract public class Command extends Co
   protected void processOptions(LinkedList<String> args) throws IOException {}
 
   /**
+   * Allows commands that don't use paths to handle the raw arguments.
+   * Default behavior is to expand the arguments via
+   * {@link #expandArguments(LinkedList)} and pass the resulting list to
+   * {@link #processArguments(LinkedList)} 
+   * @param args the list of argument strings
+   * @throws IOException
+   */
+  protected void processRawArguments(LinkedList<String> args)
+  throws IOException {
+    processArguments(expandArguments(args));
+  }
+
+  /**
    *  Expands a list of arguments into {@link PathData} objects.  The default
    *  behavior is to call {@link #expandArgument(String)} on each element
    *  which by default globs the argument.  The loop catches IOExceptions,
@@ -353,7 +367,26 @@ abstract public class Command extends Co
    * @param message warning message to display
    */
   public void displayWarning(String message) {
-    err.println(getCommandName() + ": " + message);
+    err.println(getName() + ": " + message);
+  }
+  
+  /**
+   * The name of the command.  Will first try to use the assigned name
+   * else fallback to the command's preferred name
+   * @return name of the command
+   */
+  public String getName() {
+    return (name == null)
+      ? getCommandField("NAME")
+      : name.startsWith("-") ? name.substring(1) : name; // this is a historical method
+  }
+
+  /**
+   * Define the name of the command.
+   * @param name as invoked
+   */
+  public void setName(String name) {
+    this.name = name;
   }
   
   /**
@@ -361,7 +394,7 @@ abstract public class Command extends Co
    * @return "name options"
    */
   public String getUsage() {
-    String cmd = "-" + getCommandName();
+    String cmd = "-" + getName();
     String usage = isDeprecated() ? "" : getCommandField("USAGE");
     return usage.isEmpty() ? cmd : cmd + " " + usage; 
   }
@@ -400,9 +433,10 @@ abstract public class Command extends Co
   private String getCommandField(String field) {
     String value;
     try {
-      value = (String)this.getClass().getField(field).get(null);
+      value = this.getClass().getField(field).get(this).toString();
     } catch (Exception e) {
-      throw new RuntimeException(StringUtils.stringifyException(e));
+      throw new RuntimeException(
+          "failed to get " + this.getClass().getSimpleName()+"."+field, e);
     }
     return value;
   }

Modified: hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CommandFactory.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CommandFactory.java?rev=1132764&r1=1132763&r2=1132764&view=diff
==============================================================================
--- hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CommandFactory.java (original)
+++ hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/CommandFactory.java Mon Jun  6 20:53:37 2011
@@ -19,7 +19,8 @@
 package org.apache.hadoop.fs.shell;
 
 import java.util.Arrays;
-import java.util.Hashtable;
+import java.util.HashMap;
+import java.util.Map;
 
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
@@ -35,8 +36,11 @@ import org.apache.hadoop.util.StringUtil
 @InterfaceStability.Unstable
 
 public class CommandFactory extends Configured implements Configurable {
-  private Hashtable<String, Class<? extends Command>> classMap =
-    new Hashtable<String, Class<? extends Command>>();
+  private Map<String, Class<? extends Command>> classMap =
+    new HashMap<String, Class<? extends Command>>();
+
+  private Map<String, Command> objectMap =
+    new HashMap<String, Command>();
 
   /** Factory constructor for commands */
   public CommandFactory() {
@@ -79,16 +83,22 @@ public class CommandFactory extends Conf
   }
   
   /**
-   * Returns the class implementing the given command.  The
-   * class must have been registered via
-   * {@link #addClass(Class, String...)}
-   * @param cmd name of the command
-   * @return instance of the requested command
+   * Register the given object as handling the given list of command
+   * names.  Avoid calling this method and use
+   * {@link #addClass(Class, String...)} whenever possible to avoid
+   * startup overhead from excessive command object instantiations.  This
+   * method is intended only for handling nested non-static classes that
+   * are re-usable.  Namely -help/-usage.
+   * @param cmdObject the object implementing the command names
+   * @param names one or more command names that will invoke this class
    */
-  protected Class<? extends Command> getClass(String cmd) {
-    return classMap.get(cmd);
+  public void addObject(Command cmdObject, String ... names) {
+    for (String name : names) {
+      objectMap.put(name, cmdObject);
+      classMap.put(name, null); // just so it shows up in the list of commands
+    }
   }
-  
+
   /**
    * Returns an instance of the class implementing the given command.  The
    * class must have been registered via
@@ -109,11 +119,13 @@ public class CommandFactory extends Conf
   public Command getInstance(String cmdName, Configuration conf) {
     if (conf == null) throw new NullPointerException("configuration is null");
     
-    Command instance = null;
-    Class<? extends Command> cmdClass = getClass(cmdName);
-    if (cmdClass != null) {
-      instance = ReflectionUtils.newInstance(cmdClass, conf);
-      instance.setCommandName(cmdName);
+    Command instance = objectMap.get(cmdName);
+    if (instance == null) {
+      Class<? extends Command> cmdClass = classMap.get(cmdName);
+      if (cmdClass != null) {
+        instance = ReflectionUtils.newInstance(cmdClass, conf);
+        instance.setName(cmdName);
+      }
     }
     return instance;
   }

Modified: hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Count.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Count.java?rev=1132764&r1=1132763&r2=1132764&view=diff
==============================================================================
--- hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Count.java (original)
+++ hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Count.java Mon Jun  6 20:53:37 2011
@@ -54,9 +54,7 @@ public class Count extends FsCommand {
   private boolean showQuotas;
 
   /** Constructor */
-  public Count() {
-    setCommandName(NAME);
-  }
+  public Count() {}
   
   /** Constructor
    * @deprecated invoke via {@link FsShell}
@@ -67,7 +65,6 @@ public class Count extends FsCommand {
   @Deprecated
   public Count(String[] cmd, int pos, Configuration conf) {
     super(conf);
-    setCommandName(NAME);
     this.args = Arrays.copyOfRange(cmd, pos, cmd.length);
   }
 

Modified: hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Display.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Display.java?rev=1132764&r1=1132763&r2=1132764&view=diff
==============================================================================
--- hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Display.java (original)
+++ hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/Display.java Mon Jun  6 20:53:37 2011
@@ -100,7 +100,7 @@ class Display extends FsCommand {
    */ 
   public static class Text extends Cat {
     public static final String NAME = "text";
-    public static final String SHORT_USAGE = Cat.USAGE;
+    public static final String USAGE = Cat.USAGE;
     public static final String DESCRIPTION =
       "Takes a source file and outputs the file in text format.\n" +
       "The allowed formats are zip and TextRecordInputStream.";

Modified: hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/FsCommand.java
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/FsCommand.java?rev=1132764&r1=1132763&r2=1132764&view=diff
==============================================================================
--- hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/FsCommand.java (original)
+++ hadoop/common/trunk/src/java/org/apache/hadoop/fs/shell/FsCommand.java Mon Jun  6 20:53:37 2011
@@ -65,8 +65,10 @@ abstract public class FsCommand extends 
     super(conf);
   }
 
-  public String getCommandName() {
-    return name.startsWith("-") ? name.substring(1) : name; 
+  // historical abstract method in Command
+  @Override
+  public String getCommandName() { 
+    return getName(); 
   }
   
   // abstract method that normally is invoked by runall() which is

Modified: hadoop/common/trunk/src/test/core/org/apache/hadoop/cli/testConf.xml
URL: http://svn.apache.org/viewvc/hadoop/common/trunk/src/test/core/org/apache/hadoop/cli/testConf.xml?rev=1132764&r1=1132763&r2=1132764&view=diff
==============================================================================
--- hadoop/common/trunk/src/test/core/org/apache/hadoop/cli/testConf.xml (original)
+++ hadoop/common/trunk/src/test/core/org/apache/hadoop/cli/testConf.xml Mon Jun  6 20:53:37 2011
@@ -39,7 +39,7 @@
       <comparators>
         <comparator>
           <type>SubstringComparator</type>
-          <expected-output>hadoop fs is the command to execute fs commands. The full syntax is</expected-output>
+          <expected-output>Usage: hadoop fs [generic options]</expected-output>
         </comparator>
       </comparators>
     </test>
@@ -730,7 +730,7 @@
       <comparators>
         <comparator>
           <type>RegexpComparator</type>
-          <expected-output>^-help \[cmd\]:( |\t)*Displays help for given command or all commands if none( )*</expected-output>
+          <expected-output>^-help \[cmd ...\]:( |\t)*Displays help for given command or all commands if none( )*</expected-output>
         </comparator>
         <comparator>
           <type>RegexpComparator</type>