Return-Path: X-Original-To: apmail-hadoop-common-commits-archive@www.apache.org Delivered-To: apmail-hadoop-common-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 08C63E12F for ; Thu, 6 Dec 2012 15:22:43 +0000 (UTC) Received: (qmail 15828 invoked by uid 500); 6 Dec 2012 15:22:42 -0000 Delivered-To: apmail-hadoop-common-commits-archive@hadoop.apache.org Received: (qmail 15197 invoked by uid 500); 6 Dec 2012 15:22:35 -0000 Mailing-List: contact common-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: common-dev@hadoop.apache.org Delivered-To: mailing list common-commits@hadoop.apache.org Received: (qmail 15138 invoked by uid 99); 6 Dec 2012 15:22:33 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 06 Dec 2012 15:22:33 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 06 Dec 2012 15:22:28 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 6869C2388A4A for ; Thu, 6 Dec 2012 15:22:07 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1417935 - in /hadoop/common/branches/branch-1-win: ./ src/contrib/streaming/ src/core/org/apache/hadoop/fs/ src/core/org/apache/hadoop/io/nativeio/ src/core/org/apache/hadoop/util/ src/native/src/org/apache/hadoop/io/nativeio/ src/test/org... Date: Thu, 06 Dec 2012 15:22:06 -0000 To: common-commits@hadoop.apache.org From: suresh@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20121206152207.6869C2388A4A@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: suresh Date: Thu Dec 6 15:22:04 2012 New Revision: 1417935 URL: http://svn.apache.org/viewvc?rev=1417935&view=rev Log: HADOOP-9061. Java6+Windows does not work well with symlinks. Contributed by Ivan Mitic. Modified: hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt hadoop/common/branches/branch-1-win/src/contrib/streaming/ivy.xml hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/FileUtil.java hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/io/nativeio/NativeIO.java hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/Shell.java hadoop/common/branches/branch-1-win/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/filecache/TestMRWithDistributedCache.java hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFileUtil.java Modified: hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt?rev=1417935&r1=1417934&r2=1417935&view=diff ============================================================================== --- hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt (original) +++ hadoop/common/branches/branch-1-win/CHANGES.branch-1-win.txt Thu Dec 6 15:22:04 2012 @@ -237,3 +237,6 @@ Branch-hadoop-1-win - unreleased HADOOP-9102. winutils task isAlive does not return a non-zero exit code if the requested task is not alive. (Chris Nauroth via suresh) + + HADOOP-9061. Java6+Windows does not work well with symlinks. + (Ivan Mitic via suresh) Modified: hadoop/common/branches/branch-1-win/src/contrib/streaming/ivy.xml URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/contrib/streaming/ivy.xml?rev=1417935&r1=1417934&r2=1417935&view=diff ============================================================================== --- hadoop/common/branches/branch-1-win/src/contrib/streaming/ivy.xml (original) +++ hadoop/common/branches/branch-1-win/src/contrib/streaming/ivy.xml Thu Dec 6 15:22:04 2012 @@ -28,6 +28,10 @@ name="commons-cli" rev="${commons-cli.version}" conf="common->default"/> + " + target); + org.apache.commons.io.FileUtils.copyFile(targetFile, linkFile); + } catch (IOException ex) { + LOG.warn("FileUtil#symlink failed to copy the file with error: " + + ex.getMessage()); + // Exit with non-zero exit code + return 1; + } + return 0; + } + + String[] cmd = Shell.getSymlinkCommand(targetFile.getPath(), + linkFile.getPath()); ShellCommandExecutor shExec = new ShellCommandExecutor(cmd); try { shExec.execute(); @@ -908,48 +931,4 @@ public class FileUtil { jarStream.close(); return jarFile; } - - /** Returns the file length. In case of a symbolic link, follows the link - * and returns the target file length. API is used to provide - * transparent behavior between Unix and Windows since on Windows - * {@link File#length()} does not follow symbolic links. - * @param file file to extract the length for - * - * @throws IOException if an error occurred. - */ - public static long getLengthFollowSymlink(File file) { - return getLengthFollowSymlink(file, false); - } - - /** Package level API used for unit-testing only. */ - static long getLengthFollowSymlink(File file, boolean disableNativeIO) { - if (!Shell.WINDOWS) { - return file.length(); - } else { - try { - // FIXME: Below logic is not needed On Java 7 under Windows since - // File#length returns the correct value. - if (!disableNativeIO && NativeIO.isAvailable()) { - // Use Windows native API for extracting the file length if NativeIO - // is available. - return NativeIO.Windows.getLengthFollowSymlink( - file.getCanonicalPath()); - } else { - // If NativeIO is not available, we have to provide the fallback - // mechanism since downstream Hadoop projects do not want - // to worry about adding hadoop.dll to the java library path. - - // Extract the target link size via winutils - String output = FileUtil.execCommand( - file, new String[] { Shell.WINUTILS, "ls", "-L", "-F" }); - // Output tokens are separated with a pipe character - String[] args = output.split("\\|"); - return Long.parseLong(args[4]); - } - } catch (IOException ex) { - // In case of an error return zero to be consistent with File#length - return 0; - } - } - } } Modified: hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java?rev=1417935&r1=1417934&r2=1417935&view=diff ============================================================================== --- hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java (original) +++ hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/fs/RawLocalFileSystem.java Thu Dec 6 15:22:04 2012 @@ -435,11 +435,9 @@ public class RawLocalFileSystem extends return !super.getOwner().equals(""); } - RawLocalFileStatus(File f, long defaultBlockSize, FileSystem fs) { - // Extract File#length thru FileUtil#getLengthFollowSymlink to provide - // the same behavior for symbolic links on different platforms. - super(FileUtil.getLengthFollowSymlink(f), f.isDirectory(), 1, defaultBlockSize, - f.lastModified(), new Path(f.getPath()).makeQualified(fs)); + RawLocalFileStatus(File f, long defaultBlockSize, FileSystem fs) { + super(f.length(), f.isDirectory(), 1, defaultBlockSize, + f.lastModified(), new Path(f.getPath()).makeQualified(fs)); } @Override Modified: hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/io/nativeio/NativeIO.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/io/nativeio/NativeIO.java?rev=1417935&r1=1417934&r2=1417935&view=diff ============================================================================== --- hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/io/nativeio/NativeIO.java (original) +++ hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/io/nativeio/NativeIO.java Thu Dec 6 15:22:04 2012 @@ -177,10 +177,6 @@ public class NativeIO { /** Windows only methods used for getOwner() implementation */ private static native String getOwner(FileDescriptor fd) throws IOException; - /** Windows only method used for getting the file length */ - public static native long getLengthFollowSymlink( - String path) throws IOException; - static { if (NativeCodeLoader.isNativeCodeLoaded()) { try { Modified: hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/Shell.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/Shell.java?rev=1417935&r1=1417934&r2=1417935&view=diff ============================================================================== --- hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/Shell.java (original) +++ hadoop/common/branches/branch-1-win/src/core/org/apache/hadoop/util/Shell.java Thu Dec 6 15:22:04 2012 @@ -41,6 +41,13 @@ abstract public class Shell { public static final Log LOG = LogFactory.getLog(Shell.class); + private static boolean IS_JAVA7_OR_ABOVE = + System.getProperty("java.version").substring(0, 3).compareTo("1.7") >= 0; + + public static boolean isJava7OrAbove() { + return IS_JAVA7_OR_ABOVE; + } + /** a Windows utility to emulate Unix commands */ public static final String WINUTILS = System.getenv("HADOOP_HOME") + "\\bin\\winutils"; Modified: hadoop/common/branches/branch-1-win/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c?rev=1417935&r1=1417934&r2=1417935&view=diff ============================================================================== --- hadoop/common/branches/branch-1-win/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c (original) +++ hadoop/common/branches/branch-1-win/src/native/src/org/apache/hadoop/io/nativeio/NativeIO.c Thu Dec 6 15:22:04 2012 @@ -604,42 +604,6 @@ cleanup: #endif } -/* - * Class: org_apache_hadoop_io_nativeio_NativeIO_Windows - * Method: getLengthFollowSymlink - * Signature: (Ljava/lang/String;)J - */ -JNIEXPORT jlong JNICALL Java_org_apache_hadoop_io_nativeio_NativeIO_00024Windows_getLengthFollowSymlink - (JNIEnv *env, jclass clazz, jstring j_path) -{ -#ifdef UNIX - THROW(env, "java/io/IOException", - "The function getLengthFollowSymlink(String) is not supported on Unix"); - return 0; -#endif - -#ifdef WINDOWS - DWORD dwRtnCode = ERROR_SUCCESS; - BY_HANDLE_FILE_INFORMATION fileInfo = { 0 }; - LARGE_INTEGER fileSize = { 0 }; - - const wchar_t *path = (const wchar_t*) (*env)->GetStringChars(env, j_path, NULL); - if (path == NULL) return 0; // JVM throws Exception for us - - dwRtnCode = GetFileInformationByName(path, TRUE, &fileInfo); - if (dwRtnCode != ERROR_SUCCESS) { - throw_ioe(env, dwRtnCode); - } - - (*env)->ReleaseStringChars(env, j_path, path); - - fileSize.HighPart = fileInfo.nFileSizeHigh; - fileSize.LowPart = fileInfo.nFileSizeLow; - - return (jlong)(fileSize.QuadPart); -#endif -} - /** * vim: sw=2: ts=2: et: */ Modified: hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/filecache/TestMRWithDistributedCache.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/filecache/TestMRWithDistributedCache.java?rev=1417935&r1=1417934&r2=1417935&view=diff ============================================================================== --- hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/filecache/TestMRWithDistributedCache.java (original) +++ hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/filecache/TestMRWithDistributedCache.java Thu Dec 6 15:22:04 2012 @@ -119,10 +119,7 @@ public class TestMRWithDistributedCache context.getConfiguration().get("mapred.job.tracker"))) { File symlinkFile = new File("distributed.first.symlink"); TestCase.assertTrue(symlinkFile.exists()); - // Java 6 File#length returns zero for symbolic links on Windows - // FIXME: File#length for symbolic links may be due to change in Java 7 - int expectedValue = Shell.WINDOWS ? 0 : 1; - TestCase.assertEquals(expectedValue, symlinkFile.length()); + TestCase.assertEquals(1, symlinkFile.length()); } } } Modified: hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFileUtil.java URL: http://svn.apache.org/viewvc/hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFileUtil.java?rev=1417935&r1=1417934&r2=1417935&view=diff ============================================================================== --- hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFileUtil.java (original) +++ hadoop/common/branches/branch-1-win/src/test/org/apache/hadoop/fs/TestFileUtil.java Thu Dec 6 15:22:04 2012 @@ -351,7 +351,7 @@ public class TestFileUtil { //ensure that symlink length is correctly reported by Java Assert.assertEquals(data.length, file.length()); - Assert.assertEquals(Shell.WINDOWS ? 0 : data.length, link.length()); + Assert.assertEquals(data.length, link.length()); //ensure that we can read from link. FileInputStream in = new FileInputStream(link); @@ -364,10 +364,66 @@ public class TestFileUtil { } /** - * Test the value of File's length extracted using FileUtil. + * Test that rename on a symlink works as expected. */ @Test - public void testGetLengthFollowSymlink() throws Exception { + public void testSymlinkRenameTo() throws Exception { + Assert.assertFalse(del.exists()); + del.mkdirs(); + + File file = new File(del, FILE); + file.createNewFile(); + File link = new File(del, "_link"); + + // create the symlink + FileUtil.symLink(file.getAbsolutePath(), link.getAbsolutePath()); + + Assert.assertTrue(file.exists()); + Assert.assertTrue(link.exists()); + + File link2 = new File(del, "_link2"); + + // Rename the symlink + Assert.assertTrue(link.renameTo(link2)); + + // Make sure the file still exists + // (NOTE: this would fail on Java6 on Windows if we didn't + // copy the file in FileUtil#symlink) + Assert.assertTrue(file.exists()); + + Assert.assertTrue(link2.exists()); + Assert.assertFalse(link.exists()); + } + + /** + * Test that deletion of a symlink works as expected. + */ + @Test + public void testSymlinkDelete() throws Exception { + Assert.assertFalse(del.exists()); + del.mkdirs(); + + File file = new File(del, FILE); + file.createNewFile(); + File link = new File(del, "_link"); + + // create the symlink + FileUtil.symLink(file.getAbsolutePath(), link.getAbsolutePath()); + + Assert.assertTrue(file.exists()); + Assert.assertTrue(link.exists()); + + // make sure that deleting a symlink works properly + Assert.assertTrue(link.delete()); + Assert.assertFalse(link.exists()); + Assert.assertTrue(file.exists()); + } + + /** + * Test that length on a symlink works as expected. + */ + @Test + public void testSymlinkLength() throws Exception { Assert.assertFalse(del.exists()); del.mkdirs(); @@ -381,37 +437,30 @@ public class TestFileUtil { os.write(data); os.close(); - // ensure that getLengthFollowSymlink returns zero if a file - // does not exist - Assert.assertEquals(0, FileUtil.getLengthFollowSymlink(link)); + Assert.assertEquals(0, link.length()); // create the symlink FileUtil.symLink(file.getAbsolutePath(), link.getAbsolutePath()); - // ensure that getLengthFollowSymlink returns the target file and link size - Assert.assertEquals(data.length, FileUtil.getLengthFollowSymlink(file)); - Assert.assertEquals(data.length, FileUtil.getLengthFollowSymlink(link)); - - // ensure that getLengthFollowSymlink returns the target file and link - // size when NativeIO is not used (tests the fallback functionality - // on Windows) - Assert.assertEquals( - data.length, FileUtil.getLengthFollowSymlink(file, true)); - Assert.assertEquals( - data.length, FileUtil.getLengthFollowSymlink(link, true)); + // ensure that File#length returns the target file and link size + Assert.assertEquals(data.length, file.length()); + Assert.assertEquals(data.length, link.length()); - // Make sure that files can be deleted (no remaining open handles) file.delete(); Assert.assertFalse(file.exists()); - // Link size should be zero when it's pointing to nothing - Assert.assertEquals(0, FileUtil.getLengthFollowSymlink(link)); + if (Shell.WINDOWS && !Shell.isJava7OrAbove()) { + // On Java6 on Windows, we copied the file + Assert.assertEquals(data.length, link.length()); + } else { + // Otherwise, the target file size is zero + Assert.assertEquals(0, link.length()); + } link.delete(); Assert.assertFalse(link.exists()); - } - + private void doUntarAndVerify(File tarFile, File untarDir) throws IOException { if (untarDir.exists() && !FileUtil.fullyDelete(untarDir)) {