Author: dhruba
Date: Thu Oct 4 17:05:04 2007
New Revision: 582033
URL: http://svn.apache.org/viewvc?rev=582033&view=rev
Log:
HADOOP-1961. The -get option to dfs-shell works when a single filename
is specified. (Raghu Angadi via dhruba)
Modified:
lucene/hadoop/trunk/CHANGES.txt
lucene/hadoop/trunk/src/java/org/apache/hadoop/fs/FsShell.java
lucene/hadoop/trunk/src/test/org/apache/hadoop/dfs/TestDFSShell.java
Modified: lucene/hadoop/trunk/CHANGES.txt
URL:
http://svn.apache.org/viewvc/lucene/hadoop/trunk/CHANGES.txt?rev=582033&r1=582032&r2=582033&view=diff
==============================================================================
--- lucene/hadoop/trunk/CHANGES.txt (original)
+++ lucene/hadoop/trunk/CHANGES.txt Thu Oct 4 17:05:04 2007
@@ -378,6 +378,9 @@
block and that source Datanode had failed.
(Raghu Angadi via dhruba)
+ HADOOP-1961. The -get option to dfs-shell works when a single filename
+ is specified. (Raghu Angadi via dhruba)
+
Release 0.14.1 - 2007-09-04
BUG FIXES
Modified: lucene/hadoop/trunk/src/java/org/apache/hadoop/fs/FsShell.java
URL:
http://svn.apache.org/viewvc/lucene/hadoop/trunk/src/java/org/apache/hadoop/fs/FsShell.java?rev=582033&r1=582032&r2=582033&view=diff
==============================================================================
--- lucene/hadoop/trunk/src/java/org/apache/hadoop/fs/FsShell.java (original)
+++ lucene/hadoop/trunk/src/java/org/apache/hadoop/fs/FsShell.java Thu Oct 4
17:05:04 2007
@@ -146,7 +146,18 @@
}
cat(srcf);
} else {
- copyToLocal(fs, new Path(srcf), new File(dstf), copyCrc);
+ File dst = new File(dstf);
+ Path src = new Path(srcf);
+ Path [] srcs = fs.globPaths(src);
+ boolean dstIsDir = dst.isDirectory();
+ if (srcs.length > 1 && !dstIsDir) {
+ throw new IOException("When copying multiple files, "
+ + "destination should be a directory.");
+ }
+ for (Path srcPath : srcs) {
+ File dstFile = (dstIsDir ? new File(dst, srcPath.getName()) : dst);
+ copyToLocal(fs, srcPath, dstFile, copyCrc);
+ }
}
}
@@ -168,61 +179,42 @@
private void copyToLocal(final FileSystem srcFS, final Path src,
final File dst, final boolean copyCrc)
throws IOException {
- if (srcFS.isDirectory(src)) { //src is a directory
- dst.mkdir();
- if (!dst.isDirectory()) {
- throw new IOException("cannot create directory for local destination
\""
- + dst + "\".");
+ /* Keep the structure similar to ChecksumFileSystem.copyToLocal().
+ * Ideal these two should just invoke FileUtil.copy() and not repeat
+ * recursion here. Of course, copy() should support two more options :
+ * copyCrc and useTmpFile (may be useTmpFile need not be an option).
+ */
+
+ if (!srcFS.isDirectory(src)) {
+ if (dst.exists()) {
+ // match the error message in FileUtil.checkDest():
+ throw new IOException("Target " + dst + " already exists");
}
- for(Path p : srcFS.listPaths(src)) {
- copyToLocal(srcFS, p,
- srcFS.isDirectory(p)? new File(dst, p.getName()): dst,
copyCrc);
+
+ // use absolute name so that tmp file is always created under dest dir
+ File tmp = FileUtil.createLocalTempFile(dst.getAbsoluteFile(),
+ COPYTOLOCAL_PREFIX, true);
+ if (!FileUtil.copy(srcFS, src, tmp, false, srcFS.getConf())) {
+ throw new IOException("Failed to copy " + src + " to " + dst);
+ }
+
+ if (!tmp.renameTo(dst)) {
+ throw new IOException("Failed to rename tmp file " + tmp +
+ " to local destination \"" + dst + "\".");
}
- }
- else {
- Path [] srcs = srcFS.globPaths(src);
- if (dst.isDirectory()) { //dst is a directory but src is not
- for (Path p : srcs) {
- copyToLocal(srcFS, p, new File(dst, p.getName()), copyCrc);
- }
- } else if (srcs.length == 1)
- {
- if (dst.exists()) {
- throw new IOException("local destination \"" + dst
- + "\" already exists.");
- }
- if (!srcFS.exists(src)) {
- throw new IOException("src \"" + src + "\" does not exist.");
- }
-
- File tmp = FileUtil.createLocalTempFile(dst, COPYTOLOCAL_PREFIX, true);
- if (FileUtil.copy(srcFS, src, tmp, false, srcFS.getConf())) {
- if (!tmp.renameTo(dst)) {
- //try to reanme tmp to another file since tmp will be deleted on exit
- File another = FileUtil.createLocalTempFile(dst,
COPYTOLOCAL_PREFIX,
- false);
- another.delete();
- if (tmp.renameTo(another)) {
- throw new IOException(
- "Failed to rename tmp file to local destination \"" + dst
- + "\". Remote source file \"" + src + "\" is saved to \""
- + another + "\".");
- } else {
- throw new IOException("Failed to rename tmp file.");
- }
- }
- }
- if (copyCrc) {
- ChecksumFileSystem csfs = (ChecksumFileSystem) srcFS;
- File dstcs = FileSystem.getLocal(srcFS.getConf())
- .pathToFile(csfs.getChecksumFile(new
Path(dst.getCanonicalPath())));
- copyToLocal(csfs.getRawFileSystem(), csfs.getChecksumFile(src),
- dstcs, false);
- }
- } else {
- throw new IOException("When copying multiple files, "
- + "destination should be a directory.");
+ if (copyCrc) {
+ ChecksumFileSystem csfs = (ChecksumFileSystem) srcFS;
+ File dstcs = FileSystem.getLocal(srcFS.getConf())
+ .pathToFile(csfs.getChecksumFile(new Path(dst.getCanonicalPath())));
+ copyToLocal(csfs.getRawFileSystem(), csfs.getChecksumFile(src),
+ dstcs, false);
+ }
+ } else {
+ // once FileUtil.copy() supports tmp file, we don't need to mkdirs().
+ dst.mkdirs();
+ for(Path path : srcFS.listPaths(src)) {
+ copyToLocal(srcFS, path, new File(dst, path.getName()), copyCrc);
}
}
}
Modified: lucene/hadoop/trunk/src/test/org/apache/hadoop/dfs/TestDFSShell.java
URL:
http://svn.apache.org/viewvc/lucene/hadoop/trunk/src/test/org/apache/hadoop/dfs/TestDFSShell.java?rev=582033&r1=582032&r2=582033&view=diff
==============================================================================
--- lucene/hadoop/trunk/src/test/org/apache/hadoop/dfs/TestDFSShell.java
(original)
+++ lucene/hadoop/trunk/src/test/org/apache/hadoop/dfs/TestDFSShell.java Thu
Oct 4 17:05:04 2007
@@ -195,18 +195,22 @@
// + sub
// |- f3
// |- f4
+ // ROOT2
+ // |- f1
Path root = mkdir(dfs, new Path("/test/copyToLocal"));
Path sub = mkdir(dfs, new Path(root, "sub"));
+ Path root2 = mkdir(dfs, new Path("/test/copyToLocal2"));
writeFile(fs, new Path(root, "f1"));
writeFile(fs, new Path(root, "f2"));
writeFile(fs, new Path(sub, "f3"));
writeFile(fs, new Path(sub, "f4"));
+ writeFile(fs, new Path(root2, "f1"));
}
// Verify copying the tree
{
- String[] args = {"-copyToLocal", "/test/copyToLocal", TEST_ROOT_DIR};
+ String[] args = {"-copyToLocal", "/test/copyToLocal*", TEST_ROOT_DIR};
try {
assertEquals(0, shell.run(args));
} catch (Exception e) {
@@ -214,13 +218,16 @@
e.getLocalizedMessage());
}
- File f1 = new File(TEST_ROOT_DIR, "f1");
+ File localroot = new File(TEST_ROOT_DIR, "copyToLocal");
+ File localroot2 = new File(TEST_ROOT_DIR, "copyToLocal2");
+
+ File f1 = new File(localroot, "f1");
assertTrue("Copying failed.", f1.isFile());
- File f2 = new File(TEST_ROOT_DIR, "f2");
+ File f2 = new File(localroot, "f2");
assertTrue("Copying failed.", f2.isFile());
- File sub = new File(TEST_ROOT_DIR, "sub");
+ File sub = new File(localroot, "sub");
assertTrue("Copying failed.", sub.isDirectory());
File f3 = new File(sub, "f3");
@@ -228,11 +235,15 @@
File f4 = new File(sub, "f4");
assertTrue("Copying failed.", f4.isFile());
+
+ File f5 = new File(localroot2, "f1");
+ assertTrue("Copying failed.", f5.isFile());
f1.delete();
f2.delete();
f3.delete();
f4.delete();
+ f5.delete();
sub.delete();
}
} finally {
@@ -290,7 +301,7 @@
// Verify that we can get with and without crc
{
- File testFile = new File(TEST_ROOT_DIR, "myFile");
+ File testFile = new File(TEST_ROOT_DIR, "mkdirs/myFile");
File checksumFile = new File(fileSys.getChecksumFile(
new
Path(testFile.getAbsolutePath())).toString());
testFile.delete();
@@ -313,7 +324,7 @@
testFile.delete();
}
{
- File testFile = new File(TEST_ROOT_DIR, "myFile");
+ File testFile = new File(TEST_ROOT_DIR, "mkdirs/myFile");
File checksumFile = new File(fileSys.getChecksumFile(
new
Path(testFile.getAbsolutePath())).toString());
testFile.delete();
