HADOOP-9705. Merging change r1605672 from trunk to branch-2.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1605673 13f79535-47bb-0310-9956-ffa450edef68

hadoop-common-project/hadoop-common/CHANGES.txt

@@ -260,6 +260,9 @@ Release 2.5.0 - UNRELEASED
     HADOOP-10746. TestSocketIOWithTimeout#testSocketIOWithTimeout fails on
     Power PC. (Jinghui Wang via Arpit Agarwal)
 
+    HADOOP-9705. FsShell cp -p does not preserve directory attibutes.
+    (Akira AJISAKA via cnauroth)
+
   BREAKDOWN OF HADOOP-10514 SUBTASKS AND RELATED JIRAS
 
     HADOOP-10520. Extended attributes definition and FileSystem APIs for

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/shell/CommandWithDestination.java

@@ -267,6 +267,9 @@ abstract class CommandWithDestination extends FsCommand {
         dst.refreshStatus(); // need to update stat to know it exists now
       }      
       super.recursePath(src);
+      if (dst.stat.isDirectory()) {
+        preserveAttributes(src, dst);
+      }
     } finally {
       dst = savedDst;
     }
@@ -298,44 +301,7 @@ abstract class CommandWithDestination extends FsCommand {
     try {
       in = src.fs.open(src.path);
       copyStreamToTarget(in, target);
-      if (shouldPreserve(FileAttribute.TIMESTAMPS)) {
+      preserveAttributes(src, target);
-        target.fs.setTimes(
-          target.path,
-          src.stat.getModificationTime(),
-          src.stat.getAccessTime());
-      }
-      if (shouldPreserve(FileAttribute.OWNERSHIP)) {
-        target.fs.setOwner(
-          target.path,
-          src.stat.getOwner(),
-          src.stat.getGroup());
-      }
-      if (shouldPreserve(FileAttribute.PERMISSION) ||
-          shouldPreserve(FileAttribute.ACL)) {
-        target.fs.setPermission(
-          target.path,
-          src.stat.getPermission());
-      }
-      if (shouldPreserve(FileAttribute.ACL)) {
-        FsPermission perm = src.stat.getPermission();
-        if (perm.getAclBit()) {
-          List<AclEntry> srcEntries =
-              src.fs.getAclStatus(src.path).getEntries();
-          List<AclEntry> srcFullEntries =
-              AclUtil.getAclFromPermAndEntries(perm, srcEntries);
-          target.fs.setAcl(target.path, srcFullEntries);
-        }
-      }
-      if (shouldPreserve(FileAttribute.XATTR)) {
-        Map<String, byte[]> srcXAttrs = src.fs.getXAttrs(src.path);
-        if (srcXAttrs != null) {
-          Iterator<Entry<String, byte[]>> iter = srcXAttrs.entrySet().iterator();
-          while (iter.hasNext()) {
-            Entry<String, byte[]> entry = iter.next();
-            target.fs.setXAttr(target.path, entry.getKey(), entry.getValue());
-          }
-        }
-      }
     } finally {
       IOUtils.closeStream(in);
     }
@@ -365,6 +331,56 @@ abstract class CommandWithDestination extends FsCommand {
     }
   }
 
+  /**
+   * Preserve the attributes of the source to the target.
+   * The method calls {@link #shouldPreserve(FileAttribute)} to check what
+   * attribute to preserve.
+   * @param src source to preserve
+   * @param target where to preserve attributes
+   * @throws IOException if fails to preserve attributes
+   */
+  protected void preserveAttributes(PathData src, PathData target)
+      throws IOException {
+    if (shouldPreserve(FileAttribute.TIMESTAMPS)) {
+      target.fs.setTimes(
+          target.path,
+          src.stat.getModificationTime(),
+          src.stat.getAccessTime());
+    }
+    if (shouldPreserve(FileAttribute.OWNERSHIP)) {
+      target.fs.setOwner(
+          target.path,
+          src.stat.getOwner(),
+          src.stat.getGroup());
+    }
+    if (shouldPreserve(FileAttribute.PERMISSION) ||
+        shouldPreserve(FileAttribute.ACL)) {
+      target.fs.setPermission(
+          target.path,
+          src.stat.getPermission());
+    }
+    if (shouldPreserve(FileAttribute.ACL)) {
+      FsPermission perm = src.stat.getPermission();
+      if (perm.getAclBit()) {
+        List<AclEntry> srcEntries =
+            src.fs.getAclStatus(src.path).getEntries();
+        List<AclEntry> srcFullEntries =
+            AclUtil.getAclFromPermAndEntries(perm, srcEntries);
+        target.fs.setAcl(target.path, srcFullEntries);
+      }
+    }
+    if (shouldPreserve(FileAttribute.XATTR)) {
+      Map<String, byte[]> srcXAttrs = src.fs.getXAttrs(src.path);
+      if (srcXAttrs != null) {
+        Iterator<Entry<String, byte[]>> iter = srcXAttrs.entrySet().iterator();
+        while (iter.hasNext()) {
+          Entry<String, byte[]> entry = iter.next();
+          target.fs.setXAttr(target.path, entry.getKey(), entry.getValue());
+        }
+      }
+    }
+  }
+
   // Helper filter filesystem that registers created files as temp files to
   // be deleted on exit unless successfully renamed
   private static class TargetFileSystem extends FilterFileSystem {

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/shell/TestCopyPreserveFlag.java

@@ -74,6 +74,8 @@ public class TestCopyPreserveFlag {
     output.close();
     fs.setTimes(FROM, MODIFICATION_TIME, 0);
     fs.setPermission(FROM, PERMISSIONS);
+    fs.setTimes(new Path("d1"), MODIFICATION_TIME, 0);
+    fs.setPermission(new Path("d1"), PERMISSIONS);
   }
 
   @After
@@ -132,4 +134,22 @@ public class TestCopyPreserveFlag {
       run(new Cp(), FROM.toString(), TO.toString());
       assertAttributesChanged();
   }
+
+  @Test(timeout = 10000)
+  public void testDirectoryCpWithP() throws Exception {
+    run(new Cp(), "-p", "d1", "d3");
+    assertEquals(fs.getFileStatus(new Path("d1")).getModificationTime(),
+        fs.getFileStatus(new Path("d3")).getModificationTime());
+    assertEquals(fs.getFileStatus(new Path("d1")).getPermission(),
+        fs.getFileStatus(new Path("d3")).getPermission());
+  }
+
+  @Test(timeout = 10000)
+  public void testDirectoryCpWithoutP() throws Exception {
+    run(new Cp(), "d1", "d4");
+    assertTrue(fs.getFileStatus(new Path("d1")).getModificationTime() !=
+        fs.getFileStatus(new Path("d4")).getModificationTime());
+    assertTrue(!fs.getFileStatus(new Path("d1")).getPermission()
+        .equals(fs.getFileStatus(new Path("d4")).getPermission()));
+  }
 }

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSShell.java

@@ -56,6 +56,7 @@ import org.junit.Test;
 
 import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.FS_TRASH_INTERVAL_KEY;
 import static org.apache.hadoop.fs.permission.AclEntryScope.ACCESS;
+import static org.apache.hadoop.fs.permission.AclEntryScope.DEFAULT;
 import static org.apache.hadoop.fs.permission.AclEntryType.*;
 import static org.apache.hadoop.fs.permission.FsAction.*;
 import static org.apache.hadoop.hdfs.server.namenode.AclTestHelpers.aclEntry;
@@ -1762,6 +1763,166 @@ public class TestDFSShell {
     }
   }
 
+  // verify cp -ptopxa option will preserve directory attributes.
+  @Test (timeout = 120000)
+  public void testCopyCommandsToDirectoryWithPreserveOption()
+      throws Exception {
+    Configuration conf = new Configuration();
+    conf.setBoolean(DFSConfigKeys.DFS_NAMENODE_XATTRS_ENABLED_KEY, true);
+    conf.setBoolean(DFSConfigKeys.DFS_NAMENODE_ACLS_ENABLED_KEY, true);
+    MiniDFSCluster cluster = new MiniDFSCluster.Builder(conf).numDataNodes(1)
+        .format(true).build();
+    FsShell shell = null;
+    FileSystem fs = null;
+    final String testdir =
+        "/tmp/TestDFSShell-testCopyCommandsToDirectoryWithPreserveOption-"
+        + counter.getAndIncrement();
+    final Path hdfsTestDir = new Path(testdir);
+    try {
+      fs = cluster.getFileSystem();
+      fs.mkdirs(hdfsTestDir);
+      Path srcDir = new Path(hdfsTestDir, "srcDir");
+      fs.mkdirs(srcDir);
+
+      fs.setAcl(srcDir, Lists.newArrayList(
+          aclEntry(ACCESS, USER, ALL),
+          aclEntry(ACCESS, USER, "foo", ALL),
+          aclEntry(ACCESS, GROUP, READ_EXECUTE),
+          aclEntry(DEFAULT, GROUP, "bar", READ_EXECUTE),
+          aclEntry(ACCESS, OTHER, EXECUTE)));
+      // set sticky bit
+      fs.setPermission(srcDir,
+          new FsPermission(ALL, READ_EXECUTE, EXECUTE, true));
+
+      // Create a file in srcDir to check if modification time of
+      // srcDir to be preserved after copying the file.
+      // If cp -p command is to preserve modification time and then copy child
+      // (srcFile), modification time will not be preserved.
+      Path srcFile = new Path(srcDir, "srcFile");
+      fs.create(srcFile).close();
+
+      FileStatus status = fs.getFileStatus(srcDir);
+      final long mtime = status.getModificationTime();
+      final long atime = status.getAccessTime();
+      final String owner = status.getOwner();
+      final String group = status.getGroup();
+      final FsPermission perm = status.getPermission();
+
+      fs.setXAttr(srcDir, "user.a1", new byte[]{0x31, 0x32, 0x33});
+      fs.setXAttr(srcDir, "trusted.a1", new byte[]{0x31, 0x31, 0x31});
+
+      shell = new FsShell(conf);
+
+      // -p
+      Path targetDir1 = new Path(hdfsTestDir, "targetDir1");
+      String[] argv = new String[] { "-cp", "-p", srcDir.toUri().toString(),
+          targetDir1.toUri().toString() };
+      int ret = ToolRunner.run(shell, argv);
+      assertEquals("cp -p is not working", SUCCESS, ret);
+      FileStatus targetStatus = fs.getFileStatus(targetDir1);
+      assertEquals(mtime, targetStatus.getModificationTime());
+      assertEquals(atime, targetStatus.getAccessTime());
+      assertEquals(owner, targetStatus.getOwner());
+      assertEquals(group, targetStatus.getGroup());
+      FsPermission targetPerm = targetStatus.getPermission();
+      assertTrue(perm.equals(targetPerm));
+      Map<String, byte[]> xattrs = fs.getXAttrs(targetDir1);
+      assertTrue(xattrs.isEmpty());
+      List<AclEntry> acls = fs.getAclStatus(targetDir1).getEntries();
+      assertTrue(acls.isEmpty());
+      assertFalse(targetPerm.getAclBit());
+
+      // -ptop
+      Path targetDir2 = new Path(hdfsTestDir, "targetDir2");
+      argv = new String[] { "-cp", "-ptop", srcDir.toUri().toString(),
+          targetDir2.toUri().toString() };
+      ret = ToolRunner.run(shell, argv);
+      assertEquals("cp -ptop is not working", SUCCESS, ret);
+      targetStatus = fs.getFileStatus(targetDir2);
+      assertEquals(mtime, targetStatus.getModificationTime());
+      assertEquals(atime, targetStatus.getAccessTime());
+      assertEquals(owner, targetStatus.getOwner());
+      assertEquals(group, targetStatus.getGroup());
+      targetPerm = targetStatus.getPermission();
+      assertTrue(perm.equals(targetPerm));
+      xattrs = fs.getXAttrs(targetDir2);
+      assertTrue(xattrs.isEmpty());
+      acls = fs.getAclStatus(targetDir2).getEntries();
+      assertTrue(acls.isEmpty());
+      assertFalse(targetPerm.getAclBit());
+
+      // -ptopx
+      Path targetDir3 = new Path(hdfsTestDir, "targetDir3");
+      argv = new String[] { "-cp", "-ptopx", srcDir.toUri().toString(),
+          targetDir3.toUri().toString() };
+      ret = ToolRunner.run(shell, argv);
+      assertEquals("cp -ptopx is not working", SUCCESS, ret);
+      targetStatus = fs.getFileStatus(targetDir3);
+      assertEquals(mtime, targetStatus.getModificationTime());
+      assertEquals(atime, targetStatus.getAccessTime());
+      assertEquals(owner, targetStatus.getOwner());
+      assertEquals(group, targetStatus.getGroup());
+      targetPerm = targetStatus.getPermission();
+      assertTrue(perm.equals(targetPerm));
+      xattrs = fs.getXAttrs(targetDir3);
+      assertEquals(xattrs.size(), 2);
+      assertArrayEquals(new byte[]{0x31, 0x32, 0x33}, xattrs.get("user.a1"));
+      assertArrayEquals(new byte[]{0x31, 0x31, 0x31}, xattrs.get("trusted.a1"));
+      acls = fs.getAclStatus(targetDir3).getEntries();
+      assertTrue(acls.isEmpty());
+      assertFalse(targetPerm.getAclBit());
+
+      // -ptopa
+      Path targetDir4 = new Path(hdfsTestDir, "targetDir4");
+      argv = new String[] { "-cp", "-ptopa", srcDir.toUri().toString(),
+          targetDir4.toUri().toString() };
+      ret = ToolRunner.run(shell, argv);
+      assertEquals("cp -ptopa is not working", SUCCESS, ret);
+      targetStatus = fs.getFileStatus(targetDir4);
+      assertEquals(mtime, targetStatus.getModificationTime());
+      assertEquals(atime, targetStatus.getAccessTime());
+      assertEquals(owner, targetStatus.getOwner());
+      assertEquals(group, targetStatus.getGroup());
+      targetPerm = targetStatus.getPermission();
+      assertTrue(perm.equals(targetPerm));
+      xattrs = fs.getXAttrs(targetDir4);
+      assertTrue(xattrs.isEmpty());
+      acls = fs.getAclStatus(targetDir4).getEntries();
+      assertFalse(acls.isEmpty());
+      assertTrue(targetPerm.getAclBit());
+      assertEquals(fs.getAclStatus(srcDir), fs.getAclStatus(targetDir4));
+
+      // -ptoa (verify -pa option will preserve permissions also)
+      Path targetDir5 = new Path(hdfsTestDir, "targetDir5");
+      argv = new String[] { "-cp", "-ptoa", srcDir.toUri().toString(),
+          targetDir5.toUri().toString() };
+      ret = ToolRunner.run(shell, argv);
+      assertEquals("cp -ptoa is not working", SUCCESS, ret);
+      targetStatus = fs.getFileStatus(targetDir5);
+      assertEquals(mtime, targetStatus.getModificationTime());
+      assertEquals(atime, targetStatus.getAccessTime());
+      assertEquals(owner, targetStatus.getOwner());
+      assertEquals(group, targetStatus.getGroup());
+      targetPerm = targetStatus.getPermission();
+      assertTrue(perm.equals(targetPerm));
+      xattrs = fs.getXAttrs(targetDir5);
+      assertTrue(xattrs.isEmpty());
+      acls = fs.getAclStatus(targetDir5).getEntries();
+      assertFalse(acls.isEmpty());
+      assertTrue(targetPerm.getAclBit());
+      assertEquals(fs.getAclStatus(srcDir), fs.getAclStatus(targetDir5));
+    } finally {
+      if (shell != null) {
+        shell.close();
+      }
+      if (fs != null) {
+        fs.delete(hdfsTestDir, true);
+        fs.close();
+      }
+      cluster.shutdown();
+    }
+  }
+
   // Verify cp -pa option will preserve both ACL and sticky bit.
   @Test (timeout = 120000)
   public void testCopyCommandsPreserveAclAndStickyBit() throws Exception {