From 99ebad8e757e90f6e036fc213d99f82dec7b80d7 Mon Sep 17 00:00:00 2001 From: Tsz-wo Sze Date: Thu, 21 Apr 2011 16:05:30 +0000 Subject: [PATCH] HADOOP-7233. Refactor ls to conform to new FsCommand class. Contributed by Daryn Sharp git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1095761 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 3 + src/java/org/apache/hadoop/fs/FsShell.java | 148 +++-------------- .../org/apache/hadoop/fs/shell/Command.java | 30 +++- .../org/apache/hadoop/fs/shell/Count.java | 21 ++- .../org/apache/hadoop/fs/shell/FsCommand.java | 3 +- src/java/org/apache/hadoop/fs/shell/Ls.java | 155 ++++++++++++++++++ .../core/org/apache/hadoop/cli/testConf.xml | 6 +- 7 files changed, 224 insertions(+), 142 deletions(-) create mode 100644 src/java/org/apache/hadoop/fs/shell/Ls.java diff --git a/CHANGES.txt b/CHANGES.txt index 1450c116f2..7628fbf435 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -96,6 +96,9 @@ Trunk (unreleased changes) HADOOP-7230. Move "fs -help" shell command tests from HDFS to COMMOM; see also HDFS-1844. (Daryn Sharp via szetszwo) + HADOOP-7233. Refactor ls to conform to new FsCommand class. (Daryn Sharp + via szetszwo) + OPTIMIZATIONS BUG FIXES diff --git a/src/java/org/apache/hadoop/fs/FsShell.java b/src/java/org/apache/hadoop/fs/FsShell.java index 6c70dc7ff8..ea163f5c36 100644 --- a/src/java/org/apache/hadoop/fs/FsShell.java +++ b/src/java/org/apache/hadoop/fs/FsShell.java @@ -613,87 +613,6 @@ private void setFileReplication(Path file, FileSystem srcFs, short newRep, List< } } - - /** - * Get a listing of all files in that match the file pattern srcf. - * @param srcf a file pattern specifying source files - * @param recursive if need to list files in subdirs - * @throws IOException - * @see org.apache.hadoop.fs.FileSystem#globStatus(Path) - */ - private int ls(String srcf, boolean recursive) throws IOException { - Path srcPath = new Path(srcf); - FileSystem srcFs = srcPath.getFileSystem(this.getConf()); - FileStatus[] srcs = srcFs.globStatus(srcPath); - if (srcs==null || srcs.length==0) { - throw new FileNotFoundException("Cannot access " + srcf + - ": No such file or directory."); - } - - boolean printHeader = (srcs.length == 1) ? true: false; - int numOfErrors = 0; - for(int i=0; isrc - * ideally we should provide "-l" option, that lists like "ls -l". - */ - private int ls(FileStatus src, FileSystem srcFs, boolean recursive, - boolean printHeader) throws IOException { - final String cmd = recursive? "lsr": "ls"; - final FileStatus[] items = shellListStatus(cmd, srcFs, src); - if (items == null) { - return 1; - } else { - int numOfErrors = 0; - if (!recursive && printHeader) { - if (items.length != 0) { - System.out.println("Found " + items.length + " items"); - } - } - - int maxReplication = 3, maxLen = 10, maxOwner = 0,maxGroup = 0; - - for(int i = 0; i < items.length; i++) { - FileStatus stat = items[i]; - int replication = String.valueOf(stat.getReplication()).length(); - int len = String.valueOf(stat.getLen()).length(); - int owner = String.valueOf(stat.getOwner()).length(); - int group = String.valueOf(stat.getGroup()).length(); - - if (replication > maxReplication) maxReplication = replication; - if (len > maxLen) maxLen = len; - if (owner > maxOwner) maxOwner = owner; - if (group > maxGroup) maxGroup = group; - } - - for (int i = 0; i < items.length; i++) { - FileStatus stat = items[i]; - Path cur = stat.getPath(); - String mdate = dateForm.format(new Date(stat.getModificationTime())); - - System.out.print((stat.isDirectory() ? "d" : "-") + - stat.getPermission() + " "); - System.out.printf("%"+ maxReplication + - "s ", (stat.isFile() ? stat.getReplication() : "-")); - if (maxOwner > 0) - System.out.printf("%-"+ maxOwner + "s ", stat.getOwner()); - if (maxGroup > 0) - System.out.printf("%-"+ maxGroup + "s ", stat.getGroup()); - System.out.printf("%"+ maxLen + "d ", stat.getLen()); - System.out.print(mdate + " "); - System.out.println(cur.toUri().getPath()); - if (recursive && stat.isDirectory()) { - numOfErrors += ls(stat,srcFs, recursive, printHeader); - } - } - return numOfErrors; - } - } - /** * Show the size of a partition in the filesystem that contains * the specified path. @@ -1400,7 +1319,7 @@ 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 ] [-conf ]\n\t" + - "[-D ] [-ls ] [-lsr ] [-df []] [-du [-s] [-h] ]\n\t" + + "[-D ] [-df []] [-du [-s] [-h] ]\n\t" + "[-dus ] [-mv ] [-cp ] [-rm [-skipTrash] ]\n\t" + "[-rmr [-skipTrash] ] [-put ... ] [-copyFromLocal ... ]\n\t" + "[-moveFromLocal ... ] [" + @@ -1429,21 +1348,6 @@ private void printHelp(String cmd) { "\t\tappear first on the command line. Exactly one additional\n" + "\t\targument must be specified. \n"; - - String ls = "-ls : \tList the contents that match the specified file pattern. If\n" + - "\t\tpath is not specified, the contents of /user/\n" + - "\t\twill be listed. Directory entries are of the form \n" + - "\t\t\tdirName (full path) \n" + - "\t\tand file entries are of the form \n" + - "\t\t\tfileName(full path) size \n" + - "\t\twhere n is the number of replicas specified for the file \n" + - "\t\tand size is the size of the file, in bytes.\n"; - - String lsr = "-lsr : \tRecursively list the contents that match the specified\n" + - "\t\tfile pattern. Behaves very similarly to hadoop fs -ls,\n" + - "\t\texcept that the data is shown for all the entries in the\n" + - "\t\tsubtree.\n"; - String df = "-df []: \tShows the capacity, free and used space of the filesystem.\n"+ "\t\tIf the filesystem has multiple partitions, and no path to a particular partition\n"+ "\t\tis specified, then the status of the root partitions will be shown.\n"; @@ -1573,17 +1477,13 @@ private void printHelp(String cmd) { Command instance = commandFactory.getInstance("-" + cmd); if (instance != null) { - System.out.println(instance.getDescription()); + 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 ("ls".equals(cmd)) { - System.out.println(ls); - } else if ("lsr".equals(cmd)) { - System.out.println(lsr); } else if ("df".equals(cmd)) { System.out.println(df); } else if ("du".equals(cmd)) { @@ -1644,13 +1544,11 @@ private void printHelp(String cmd) { System.out.println(summary); for (String thisCmdName : commandFactory.getNames()) { instance = commandFactory.getInstance(thisCmdName); - System.out.println(instance.getUsage()); + System.out.println("\t[" + instance.getUsage() + "]"); } System.out.println("\t[-help [cmd]]\n"); System.out.println(fs); - System.out.println(ls); - System.out.println(lsr); System.out.println(df); System.out.println(du); System.out.println(dus); @@ -1678,14 +1576,29 @@ private void printHelp(String cmd) { System.out.println(chgrp); for (String thisCmdName : commandFactory.getNames()) { - instance = commandFactory.getInstance(thisCmdName); - System.out.println(instance.getDescription()); + printHelp(commandFactory.getInstance(thisCmdName)); } System.out.println(help); } } + // TODO: will eventually auto-wrap the text, but this matches the expected + // output for the hdfs tests... + private void printHelp(Command instance) { + boolean firstLine = true; + for (String line : instance.getDescription().split("\n")) { + String prefix; + if (firstLine) { + prefix = instance.getUsage() + ":\t"; + firstLine = false; + } else { + prefix = "\t\t"; + } + System.out.println(prefix + line); + } + } + /** * Apply operation specified by 'cmd' on all parameters * starting from argv[startindex]. @@ -1720,10 +1633,6 @@ private int doall(String cmd, String argv[], int startindex) { delete(argv[i], true, rmSkipTrash); } else if ("-df".equals(cmd)) { df(argv[i]); - } else if ("-ls".equals(cmd)) { - exitCode = ls(argv[i], false); - } else if ("-lsr".equals(cmd)) { - exitCode = ls(argv[i], true); } else if ("-touchz".equals(cmd)) { touchz(argv[i]); } else if ("-text".equals(cmd)) { @@ -1781,8 +1690,7 @@ private void printUsage(String cmd) { } else if ("-D".equals(cmd)) { System.err.println("Usage: java FsShell" + " [-D <[property=value>]"); - } else if ("-ls".equals(cmd) || "-lsr".equals(cmd) || - "-du".equals(cmd) || "-dus".equals(cmd) || + } else if ("-du".equals(cmd) || "-dus".equals(cmd) || "-touchz".equals(cmd) || "-mkdir".equals(cmd) || "-text".equals(cmd)) { System.err.println("Usage: java FsShell" + @@ -1822,8 +1730,6 @@ private void printUsage(String cmd) { System.err.println("Usage: java FsShell [" + TAIL_USAGE + "]"); } else { System.err.println("Usage: java FsShell"); - System.err.println(" [-ls ]"); - System.err.println(" [-lsr ]"); System.err.println(" [-df []]"); System.err.println(" [-du [-s] [-h] ]"); System.err.println(" [-dus ]"); @@ -1965,18 +1871,6 @@ public int run(String argv[]) throws Exception { "-chown".equals(cmd) || "-chgrp".equals(cmd)) { exitCode = FsShellPermissions.changePermissions(cmd, argv, i, this); - } else if ("-ls".equals(cmd)) { - if (i < argv.length) { - exitCode = doall(cmd, argv, i); - } else { - exitCode = ls(Path.CUR_DIR, false); - } - } else if ("-lsr".equals(cmd)) { - if (i < argv.length) { - exitCode = doall(cmd, argv, i); - } else { - exitCode = ls(Path.CUR_DIR, true); - } } else if ("-mv".equals(cmd)) { exitCode = rename(argv, getConf()); } else if ("-cp".equals(cmd)) { diff --git a/src/java/org/apache/hadoop/fs/shell/Command.java b/src/java/org/apache/hadoop/fs/shell/Command.java index 0c0b18dac3..225496ccb7 100644 --- a/src/java/org/apache/hadoop/fs/shell/Command.java +++ b/src/java/org/apache/hadoop/fs/shell/Command.java @@ -79,6 +79,14 @@ public void setCommandName(String cmdName) { name = cmdName; } + protected void setRecursive(boolean flag) { + recursive = flag; + } + + protected boolean isRecursive() { + return recursive; + } + /** * Execute the command on the input path * @@ -138,7 +146,8 @@ public int run(String...argv) { displayError(e); } - return (numErrors == 0) ? exitCode : 1; + // TODO: -1 should be reserved for syntax error, 1 should be failure + return (numErrors == 0) ? exitCode : -1; } /** @@ -244,7 +253,18 @@ protected void processPathArgument(PathData item) throws IOException { protected void processNonexistentPathArgument(PathData item) throws IOException { // TODO: this should be more posix-like: ex. "No such file or directory" - throw new FileNotFoundException("Can not find listing for " + item); + throw new FileNotFoundException(getFnfText(item.path)); + } + + /** + * TODO: A crutch until the text is standardized across commands... + * Eventually an exception that takes the path as an argument will + * replace custom text + * @param path the thing that doesn't exist + * @returns String in printf format + */ + protected String getFnfText(Path path) { + throw new RuntimeException(path + ": No such file or directory"); } /** @@ -309,7 +329,7 @@ protected List expandGlob(String pattern) throws IOException { if (stats != null && stats.length == 0) { // glob failed to match // TODO: this should be more posix-like: ex. "No such file or directory" - throw new FileNotFoundException("Can not find listing for " + pattern); + throw new FileNotFoundException(getFnfText(path)); } List items = new LinkedList(); @@ -371,7 +391,9 @@ public void displayWarning(String message) { * @return "name options" */ public String getUsage() { - return getCommandField("USAGE"); + String cmd = "-" + getCommandName(); + String usage = getCommandField("USAGE"); + return usage.isEmpty() ? cmd : cmd + " " + usage; } /** diff --git a/src/java/org/apache/hadoop/fs/shell/Count.java b/src/java/org/apache/hadoop/fs/shell/Count.java index b365a2263e..c63bf1d552 100644 --- a/src/java/org/apache/hadoop/fs/shell/Count.java +++ b/src/java/org/apache/hadoop/fs/shell/Count.java @@ -26,6 +26,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.ContentSummary; import org.apache.hadoop.fs.FsShell; +import org.apache.hadoop.fs.Path; /** * Count the number of directories, files, bytes, quota, and remaining quota. @@ -43,13 +44,13 @@ public static void registerCommands(CommandFactory factory) { } public static final String NAME = "count"; - public static final String USAGE = "-" + NAME + " [-q] ..."; - public static final String DESCRIPTION = CommandUtils.formatDescription(USAGE, - "Count the number of directories, files and bytes under the paths", - "that match the specified file pattern. The output columns are:", - "DIR_COUNT FILE_COUNT CONTENT_SIZE FILE_NAME or", - "QUOTA REMAINING_QUATA SPACE_QUOTA REMAINING_SPACE_QUOTA ", - " DIR_COUNT FILE_COUNT CONTENT_SIZE FILE_NAME"); + public static final String USAGE = "[-q] ..."; + public static final String DESCRIPTION = + "Count the number of directories, files and bytes under the paths\n" + + "that match the specified file pattern. The output columns are:\n" + + "DIR_COUNT FILE_COUNT CONTENT_SIZE FILE_NAME or\n" + + "QUOTA REMAINING_QUATA SPACE_QUOTA REMAINING_SPACE_QUOTA \n" + + " DIR_COUNT FILE_COUNT CONTENT_SIZE FILE_NAME"; private boolean showQuotas; @@ -86,4 +87,10 @@ protected void processPath(PathData src) throws IOException { ContentSummary summary = src.fs.getContentSummary(src.path); out.println(summary.toString(showQuotas) + src.path); } + + // TODO: remove when the error is commonized... + @Override + protected String getFnfText(Path path) { + return "Can not find listing for " + path; + } } diff --git a/src/java/org/apache/hadoop/fs/shell/FsCommand.java b/src/java/org/apache/hadoop/fs/shell/FsCommand.java index db7a7a7596..82b1376491 100644 --- a/src/java/org/apache/hadoop/fs/shell/FsCommand.java +++ b/src/java/org/apache/hadoop/fs/shell/FsCommand.java @@ -42,7 +42,8 @@ abstract public class FsCommand extends Command { * @param factory where to register the class */ public static void registerCommands(CommandFactory factory) { - Count.registerCommands(factory); + factory.registerCommands(Count.class); + factory.registerCommands(Ls.class); } protected FsCommand() {} diff --git a/src/java/org/apache/hadoop/fs/shell/Ls.java b/src/java/org/apache/hadoop/fs/shell/Ls.java new file mode 100644 index 0000000000..e0627edce7 --- /dev/null +++ b/src/java/org/apache/hadoop/fs/shell/Ls.java @@ -0,0 +1,155 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.fs.shell; + +import java.io.IOException; +import java.text.SimpleDateFormat; +import java.util.Date; +import java.util.LinkedList; + +import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.classification.InterfaceStability; +import org.apache.hadoop.fs.FileStatus; +import org.apache.hadoop.fs.Path; + +/** + * Get a listing of all files in that match the file patterns. + */ +@InterfaceAudience.Private +@InterfaceStability.Unstable + +class Ls extends FsCommand { + public static void registerCommands(CommandFactory factory) { + factory.addClass(Ls.class, "-ls"); + factory.addClass(Lsr.class, "-lsr"); + } + + public static final String NAME = "ls"; + public static final String USAGE = "[ ...]"; + public static final String DESCRIPTION = + "List the contents that match the specified file pattern. If\n" + + "path is not specified, the contents of /user/\n" + + "will be listed. Directory entries are of the form \n" + + "\tdirName (full path) \n" + + "and file entries are of the form \n" + + "\tfileName(full path) size \n" + + "where n is the number of replicas specified for the file \n" + + "and size is the size of the file, in bytes."; + + protected static final SimpleDateFormat dateFormat = + new SimpleDateFormat("yyyy-MM-dd HH:mm"); + + protected int maxRepl = 3, maxLen = 10, maxOwner = 0, maxGroup = 0; + protected String lineFormat; + + @Override + protected void processOptions(LinkedList args) + throws IOException { + CommandFormat cf = new CommandFormat(null, 0, Integer.MAX_VALUE, "R"); + cf.parse(args); + setRecursive(cf.getOpt("R")); + if (args.isEmpty()) args.add(Path.CUR_DIR); + } + + @Override + protected void processPaths(PathData parent, PathData ... items) + throws IOException { + // implicitly recurse once for cmdline directories + if (parent == null && items[0].stat.isDirectory()) { + recursePath(items[0]); + return; + } + + if (!isRecursive() && items.length != 0) { + out.println("Found " + items.length + " items"); + } + adjustColumnWidths(items); + super.processPaths(parent, items); + } + + @Override + protected void processPath(PathData item) throws IOException { + FileStatus stat = item.stat; + String line = String.format(lineFormat, + (stat.isDirectory() ? "d" : "-"), + stat.getPermission(), + (stat.isFile() ? stat.getReplication() : "-"), + stat.getOwner(), + stat.getGroup(), + stat.getLen(), + dateFormat.format(new Date(stat.getModificationTime())), + item.path.toUri().getPath() + ); + out.println(line); + } + + /** + * Compute column widths and rebuild the format string + * @param items to find the max field width for each column + */ + private void adjustColumnWidths(PathData items[]) { + for (PathData item : items) { + FileStatus stat = item.stat; + maxRepl = maxLength(maxRepl, stat.getReplication()); + maxLen = maxLength(maxLen, stat.getLen()); + maxOwner = maxLength(maxOwner, stat.getOwner()); + maxGroup = maxLength(maxGroup, stat.getGroup()); + } + + StringBuilder fmt = new StringBuilder(); + fmt.append("%s%s "); // permission string + fmt.append("%" + maxRepl + "s "); + fmt.append("%-" + maxOwner + "s "); + fmt.append("%-" + maxGroup + "s "); + fmt.append("%" + maxLen + "d "); + fmt.append("%s %s"); // mod time & path + lineFormat = fmt.toString(); + } + + private int maxLength(int n, Object value) { + return Math.max(n, (value != null) ? String.valueOf(value).length() : 0); + } + + // TODO: remove when the error is commonized... + @Override + protected String getFnfText(Path path) { + return "Cannot access " + path.toUri() + ": No such file or directory."; + } + + /** + * Get a recursive listing of all files in that match the file patterns. + * Same as "-ls -R" + */ + public static class Lsr extends Ls { + public static final String NAME = "lsr"; + public static final String USAGE = Ls.USAGE; + public static final String DESCRIPTION = + "Recursively list the contents that match the specified\n" + + "file pattern. Behaves very similarly to hadoop fs -ls,\n" + + "except that the data is shown for all the entries in the\n" + + "subtree."; + + @Override + protected void processOptions(LinkedList args) + throws IOException { + args.addFirst("-R"); + super.processOptions(args); + } + } +} diff --git a/src/test/core/org/apache/hadoop/cli/testConf.xml b/src/test/core/org/apache/hadoop/cli/testConf.xml index b14469dbcd..5603ca8ff3 100644 --- a/src/test/core/org/apache/hadoop/cli/testConf.xml +++ b/src/test/core/org/apache/hadoop/cli/testConf.xml @@ -54,7 +54,7 @@ RegexpComparator - ^-ls <path>:( |\t)*List the contents that match the specified file pattern. If( )* + ^-ls \[<path> \.\.\.\]:( |\t)*List the contents that match the specified file pattern. If( )* RegexpComparator @@ -101,7 +101,7 @@ RegexpComparator - ^-lsr <path>:( |\t)*Recursively list the contents that match the specified( )* + ^-lsr \[<path> \.\.\.\]:( |\t)*Recursively list the contents that match the specified( )* RegexpComparator @@ -222,7 +222,7 @@ RegexpComparator - ^-count \[-q\] <path> \.\.\.: Count the number of directories, files and bytes under the paths( )* + ^-count \[-q\] <path> \.\.\.:( |\t)*Count the number of directories, files and bytes under the paths( )* RegexpComparator