From 7c666454170599dc3d901c8b15ac95bca3a3a829 Mon Sep 17 00:00:00 2001 From: Bikas Saha Date: Fri, 19 Apr 2013 19:23:35 +0000 Subject: [PATCH] HADOOP-9488. FileUtil#createJarWithClassPath only substitutes environment variables from current process environment/does not support overriding when launching new process (Chris Nauroth via bikas) git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1469996 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 4 +++ .../java/org/apache/hadoop/fs/FileUtil.java | 29 ++++++++++++++----- .../org/apache/hadoop/fs/TestFileUtil.java | 19 ++++++++---- 3 files changed, 39 insertions(+), 13 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index accd2e06e7..27199debd3 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -370,6 +370,10 @@ Trunk (Unreleased) HADOOP-9433 TestLocalFileSystem#testHasFileDescriptor leaks file handle (Chris Nauroth via sanjay) + HADOOP-9488. FileUtil#createJarWithClassPath only substitutes environment + variables from current process environment/does not support overriding + when launching new process (Chris Nauroth via bikas) + OPTIMIZATIONS HADOOP-7761. Improve the performance of raw comparisons. (todd) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java index b61477e9d7..38f61ed08d 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java @@ -1039,15 +1039,17 @@ public class FileUtil { * * @param inputClassPath String input classpath to bundle into the jar manifest * @param pwd Path to working directory to save jar + * @param callerEnv Map caller's environment variables to use + * for expansion * @return String absolute path to new jar * @throws IOException if there is an I/O error while writing the jar file */ - public static String createJarWithClassPath(String inputClassPath, Path pwd) - throws IOException { + public static String createJarWithClassPath(String inputClassPath, Path pwd, + Map callerEnv) throws IOException { // Replace environment variables, case-insensitive on Windows @SuppressWarnings("unchecked") - Map env = Shell.WINDOWS ? - new CaseInsensitiveMap(System.getenv()) : System.getenv(); + Map env = Shell.WINDOWS ? new CaseInsensitiveMap(callerEnv) : + callerEnv; String[] classPathEntries = inputClassPath.split(File.pathSeparator); for (int i = 0; i < classPathEntries.length; ++i) { classPathEntries[i] = StringUtils.replaceTokens(classPathEntries[i], @@ -1078,9 +1080,22 @@ public class FileUtil { } } } else { - // Append just this jar - classPathEntryList.add(new File(classPathEntry).toURI().toURL() - .toExternalForm()); + // Append just this entry + String classPathEntryUrl = new File(classPathEntry).toURI().toURL() + .toExternalForm(); + + // File.toURI only appends trailing '/' if it can determine that it is a + // directory that already exists. (See JavaDocs.) If this entry had a + // trailing '/' specified by the caller, then guarantee that the + // classpath entry in the manifest has a trailing '/', and thus refers to + // a directory instead of a file. This can happen if the caller is + // creating a classpath jar referencing a directory that hasn't been + // created yet, but will definitely be created before running. + if (classPathEntry.endsWith(Path.SEPARATOR) && + !classPathEntryUrl.endsWith(Path.SEPARATOR)) { + classPathEntryUrl = classPathEntryUrl + Path.SEPARATOR; + } + classPathEntryList.add(classPathEntryUrl); } } String jarClassPath = StringUtils.join(" ", classPathEntryList); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java index 720811746d..1bf3d5f490 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java @@ -755,11 +755,13 @@ public class TestFileUtil { // create classpath jar String wildcardPath = tmp.getCanonicalPath() + File.separator + "*"; + String nonExistentSubdir = tmp.getCanonicalPath() + Path.SEPARATOR + "subdir" + + Path.SEPARATOR; List classPaths = Arrays.asList("cp1.jar", "cp2.jar", wildcardPath, - "cp3.jar"); + "cp3.jar", nonExistentSubdir); String inputClassPath = StringUtils.join(File.pathSeparator, classPaths); String classPathJar = FileUtil.createJarWithClassPath(inputClassPath, - new Path(tmp.getCanonicalPath())); + new Path(tmp.getCanonicalPath()), System.getenv()); // verify classpath by reading manifest from jar file JarFile jarFile = null; @@ -774,15 +776,20 @@ public class TestFileUtil { Assert.assertNotNull(classPathAttr); List expectedClassPaths = new ArrayList(); for (String classPath: classPaths) { - if (!wildcardPath.equals(classPath)) { - expectedClassPaths.add(new File(classPath).toURI().toURL() - .toExternalForm()); - } else { + if (wildcardPath.equals(classPath)) { // add wildcard matches for (File wildcardMatch: wildcardMatches) { expectedClassPaths.add(wildcardMatch.toURI().toURL() .toExternalForm()); } + } else if (nonExistentSubdir.equals(classPath)) { + // expect to maintain trailing path separator if present in input, even + // if directory doesn't exist yet + expectedClassPaths.add(new File(classPath).toURI().toURL() + .toExternalForm() + Path.SEPARATOR); + } else { + expectedClassPaths.add(new File(classPath).toURI().toURL() + .toExternalForm()); } } List actualClassPaths = Arrays.asList(classPathAttr.split(" "));