You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ant.apache.org by bo...@apache.org on 2009/09/08 18:16:55 UTC
svn commit: r812573 - in /ant/core/trunk/src:
etc/testcases/taskdefs/manifestclasspath.xml
main/org/apache/tools/ant/taskdefs/ManifestClassPath.java
tests/junit/org/apache/tools/ant/taskdefs/ManifestClassPathTest.java
Author: bodewig
Date: Tue Sep 8 16:16:54 2009
New Revision: 812573
URL: http://svn.apache.org/viewvc?rev=812573&view=rev
Log:
Use FileUtils.getRelativePath which knows how to deal with drive letters. PR 44499
Modified:
ant/core/trunk/src/etc/testcases/taskdefs/manifestclasspath.xml
ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/ManifestClassPath.java
ant/core/trunk/src/tests/junit/org/apache/tools/ant/taskdefs/ManifestClassPathTest.java
Modified: ant/core/trunk/src/etc/testcases/taskdefs/manifestclasspath.xml
URL: http://svn.apache.org/viewvc/ant/core/trunk/src/etc/testcases/taskdefs/manifestclasspath.xml?rev=812573&r1=812572&r2=812573&view=diff
==============================================================================
--- ant/core/trunk/src/etc/testcases/taskdefs/manifestclasspath.xml (original)
+++ ant/core/trunk/src/etc/testcases/taskdefs/manifestclasspath.xml Tue Sep 8 16:16:54 2009
@@ -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>
Modified: ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/ManifestClassPath.java
URL: http://svn.apache.org/viewvc/ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/ManifestClassPath.java?rev=812573&r1=812572&r2=812573&view=diff
==============================================================================
--- ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/ManifestClassPath.java (original)
+++ ant/core/trunk/src/main/org/apache/tools/ant/taskdefs/ManifestClassPath.java Tue Sep 8 16:16:54 2009
@@ -66,70 +66,51 @@
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 @@
} 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 @@
* @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;
}
Modified: ant/core/trunk/src/tests/junit/org/apache/tools/ant/taskdefs/ManifestClassPathTest.java
URL: http://svn.apache.org/viewvc/ant/core/trunk/src/tests/junit/org/apache/tools/ant/taskdefs/ManifestClassPathTest.java?rev=812573&r1=812572&r2=812573&view=diff
==============================================================================
--- ant/core/trunk/src/tests/junit/org/apache/tools/ant/taskdefs/ManifestClassPathTest.java (original)
+++ ant/core/trunk/src/tests/junit/org/apache/tools/ant/taskdefs/ManifestClassPathTest.java Tue Sep 8 16:16:54 2009
@@ -142,7 +142,7 @@
public void testInternationalGerman() {
executeTarget("international-german");
expectLogContaining("run-two-jars", "beta alpha");
-
+
}
public void testInternationalHebrew() {
if (!Os.isFamily("windows")) {
@@ -154,5 +154,32 @@
}
+ 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
Re: svn commit: r812573 - in /ant/core/trunk/src:
etc/testcases/taskdefs/manifestclasspath.xml main/org/apache/tools/ant/taskdefs/ManifestClassPath.java
tests/junit/org/apache/tools/ant/taskdefs/ManifestClassPathTest.java
Posted by Dominique Devienne <dd...@gmail.com>.
On Tue, Sep 8, 2009 at 10:17 PM, Stefan Bodewig <bo...@apache.org> wrote:
>> So I'm not sure the bug report is valid at all,
>
> The report talked about problems when staying on the same drive, this
> failed before I changed the code.
Thanks for the precisions. I get it now, and sorry for the noise. --DD
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r812573 - in /ant/core/trunk/src: etc/testcases/taskdefs/manifestclasspath.xml main/org/apache/tools/ant/taskdefs/ManifestClassPath.java tests/junit/org/apache/tools/ant/taskdefs/ManifestClassPathTest.java
Posted by Stefan Bodewig <bo...@apache.org>.
On 2009-09-08, Dominique Devienne <dd...@gmail.com> wrote:
> On Tue, Sep 8, 2009 at 11:16 AM, <bo...@apache.org> wrote:
>> Author: bodewig
>> Date: Tue Sep 8 16:16:54 2009
>> New Revision: 812573
>> URL: http://svn.apache.org/viewvc?rev=812573&view=rev
>> Log: Use FileUtils.getRelativePath which knows how to deal with drive letters. PR 44499
>> [...]
>>> <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>
> I don't get it Stefan. What is going to be in the manifest?
The task will fail because it cannot find a relative path (and the test
asserts the error). FileUtils.getRelativePath returns the canonical
path of the target file.
> IMHO, a <manifest> classpath should only ever contain true relative paths,
It does.
> So I'm not sure the bug report is valid at all,
The report talked about problems when staying on the same drive, this
failed before I changed the code.
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org
Re: svn commit: r812573 - in /ant/core/trunk/src:
etc/testcases/taskdefs/manifestclasspath.xml main/org/apache/tools/ant/taskdefs/ManifestClassPath.java
tests/junit/org/apache/tools/ant/taskdefs/ManifestClassPathTest.java
Posted by Dominique Devienne <dd...@gmail.com>.
On Tue, Sep 8, 2009 at 11:16 AM, <bo...@apache.org> wrote:
> Author: bodewig
> Date: Tue Sep 8 16:16:54 2009
> New Revision: 812573
>
> URL: http://svn.apache.org/viewvc?rev=812573&view=rev
> Log: Use FileUtils.getRelativePath which knows how to deal with drive letters. PR 44499
> [...]
> + <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>
I don't get it Stefan. What is going to be in the manifest?
AFAIK, there's no way to change drive (letter) with a relative path/filename.
You can't "cd .." when you're in D:\ or C:\ so how does one go from C: to D:
using a relative path??? cd /d allows changing drive letter, but takes an
absolute path, not a relative one.
IMHO, a <manifest> classpath should only ever contain true relative paths,
or absolute URLs (in which case one would need file:///D/a/b/x.jar and
D:/a/b/x.jar is not a valid entry for Class-Path: IMHO.)
So I'm not sure the bug report is valid at all, and why my original code didn't
allow changing drives, because AFAIK each drive is a "root" directory. --DD
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org