Use FileUtils.getRelativePath which knows how to deal with drive letters. PR 44499

git-svn-id: https://svn.apache.org/repos/asf/ant/core/trunk@812573 13f79535-47bb-0310-9956-ffa450edef68

src/etc/testcases/taskdefs/manifestclasspath.xml

@@ -216,4 +216,22 @@
       </jar>
       <java fork="true" jar="${tmp}/beta.jar"/>
     </target>
+
+  <target name="testSameDrive">
+    <manifestclasspath jarfile="C:/Temp/e.jar"
+                       maxParentLevels="99" property="cp">
+      <classpath>
+        <pathelement location="C:/a/b/x.jar"/>
+      </classpath>
+    </manifestclasspath>
+  </target>
+
+  <target name="testDifferentDrive">
+    <manifestclasspath jarfile="C:/Temp/e.jar"
+                       maxParentLevels="99" property="cp">
+      <classpath>
+        <pathelement location="D:/a/b/x.jar"/>
+      </classpath>
+    </manifestclasspath>
+  </target>
 </project>

src/main/org/apache/tools/ant/taskdefs/ManifestClassPath.java

@@ -66,70 +66,51 @@ public class ManifestClassPath extends Task {
             throw new BuildException("Missing nested <classpath>!");
         }
 
+        StringBuffer tooLongSb = new StringBuffer();
+        for (int i = 0; i < maxParentLevels + 1; i++) {
+            tooLongSb.append("../");
+        }
+        final String tooLongPrefix = tooLongSb.toString();
+
         // Normalize the reference directory (containing the jar)
         final FileUtils fileUtils = FileUtils.getFileUtils();
         dir = fileUtils.normalize(dir.getAbsolutePath());
 
-        // Create as many directory prefixes as parent levels to traverse,
-        // in addition to the reference directory itself
-        File currDir = dir;
-        String[] dirs = new String[maxParentLevels + 1];
-        for (int i = 0; i < maxParentLevels + 1; ++i) {
-            dirs[i] = currDir.getAbsolutePath();
-            if (!dirs[i].equals("" + File.separatorChar)) {
-                dirs[i] = dirs[i] + File.separatorChar;
-            }
-            currDir = currDir.getParentFile();
-            if (currDir == null) {
-                maxParentLevels = i + 1;
-                break;
-            }
-        }
-
         String[] elements = path.list();
         StringBuffer buffer = new StringBuffer();
         StringBuffer element = new StringBuffer();
         for (int i = 0; i < elements.length; ++i) {
             // Normalize the current file
             File pathEntry = new File(elements[i]);
-            pathEntry = fileUtils.normalize(pathEntry.getAbsolutePath());
             String fullPath = pathEntry.getAbsolutePath();
+            pathEntry = fileUtils.normalize(fullPath);
 
-            // Find the longest prefix shared by the current file
-            // and the reference directory.
             String relPath = null;
-            for (int j = 0; j <= maxParentLevels && j < dirs.length; ++j) {
-                String dir = dirs[j];
-                if (!fullPath.startsWith(dir)) {
-                    continue;
-                }
+            String canonicalPath = null;
+            try {
+                relPath = FileUtils.getRelativePath(dir, pathEntry);
 
-                // We have a match! Add as many ../ as parent
-                // directory traversed to get the relative path
-                element.setLength(0);
-                for (int k = 0; k < j; ++k) {
-                    element.append("..");
-                    element.append(File.separatorChar);
+                canonicalPath = pathEntry.getCanonicalPath();
+                // getRelativePath always uses '/' as separator, adapt
+                if (File.separatorChar != '/') {
+                    canonicalPath =
+                        canonicalPath.replace(File.separatorChar, '/');
                 }
-                element.append(fullPath.substring(dir.length()));
-                relPath = element.toString();
-                break;
+            } catch (Exception e) {
+                throw new BuildException("error trying to get the relative path"
+                                         + " from " + dir + " to " + fullPath,
+                                         e);
             }
 
             // No match, so bail out!
-            if (relPath == null) {
+            if (relPath.equals(canonicalPath)
+                || relPath.startsWith(tooLongPrefix)) {
                 throw new BuildException(
                     "No suitable relative path from "
                     + dir + " to " + fullPath);
             }
 
-            // Manifest's ClassPath: attribute always uses forward
-            // slashes '/', and is space-separated. Ant will properly
-            // format it on 72 columns with proper line continuation
-            if (File.separatorChar != '/') {
-                relPath = relPath.replace(File.separatorChar, '/');
-            }
-            if (pathEntry.isDirectory()) {
+            if (pathEntry.isDirectory() && !relPath.endsWith("/")) {
                 relPath = relPath + '/';
             }
             try {
@@ -137,6 +118,9 @@ public class ManifestClassPath extends Task {
             } catch (UnsupportedEncodingException exc) {
                 throw new BuildException(exc);
             }
+            // Manifest's ClassPath: attribute always uses forward
+            // slashes '/', and is space-separated. Ant will properly
+            // format it on 72 columns with proper line continuation
             buffer.append(relPath);
             buffer.append(' ');
         }
@@ -175,6 +159,10 @@ public class ManifestClassPath extends Task {
      * @param  levels the max level. Defaults to 2.
      */
     public void setMaxParentLevels(int levels) {
+        if (levels < 0) {
+            throw new BuildException("maxParentLevels must not be a negative"
+                                     + " number");
+        }
         this.maxParentLevels = levels;
     }
 

src/tests/junit/org/apache/tools/ant/taskdefs/ManifestClassPathTest.java

@@ -142,7 +142,7 @@ public class ManifestClassPathTest
     public void testInternationalGerman() {
         executeTarget("international-german");
         expectLogContaining("run-two-jars", "beta alpha");
-            
+
     }
     public void testInternationalHebrew() {
         if (!Os.isFamily("windows")) {
@@ -154,5 +154,32 @@ public class ManifestClassPathTest
 
     }
 
+    public void testSameWindowsDrive() {
+        if (!Os.isFamily("windows")) {
+            System.out.println("Test with drive letters only run on windows");
+        } else {
+            executeTarget("testSameDrive");
+        }
+        assertPropertyEquals("cp", "../a/b/x.jar");
+    }
+
+    public void testDifferentWindowsDrive() {
+        if (!Os.isFamily("windows")) {
+            System.out.println("Test with drive letters only run on windows");
+        } else {
+            try {
+                new java.io.File("D:/").getCanonicalPath();
+            } catch (java.io.IOException e) {
+                System.out.println("drive d: doesn't exist or is not ready,"
+                                   + " skipping test");
+                return;
+            }
+
+            expectBuildExceptionContaining("testDifferentDrive",
+                                           "different drive",
+                                           "No suitable relative path from ");
+            assertPropertyUnset("cp");
+        }
+    }
 } // END class ManifestClassPathTest