You are viewing a plain text version of this content. The canonical link for it is here.
Posted to surefire-commits@maven.apache.org by ke...@apache.org on 2007/05/31 18:24:36 UTC

svn commit: r543178 - /maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java

Author: kenney
Date: Thu May 31 09:24:35 2007
New Revision: 543178

URL: http://svn.apache.org/viewvc?view=rev&rev=543178
Log:
Fix 'isDirectory' check; classpath elements that are directories MUST end in a '/', and 'file://localhost//foo' is NOT a directory according to java.io.File - it's an url!

Modified:
    maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java

Modified: maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java
URL: http://svn.apache.org/viewvc/maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java?view=diff&rev=543178&r1=543177&r2=543178
==============================================================================
--- maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java (original)
+++ maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java Thu May 31 09:24:35 2007
@@ -229,8 +229,7 @@
         for ( Iterator it = classPath.iterator(); it.hasNext(); )
         {
             String el = (String) it.next();
-            el = "file://localhost/" + el;
-            cp += " " + el + ( new File( el ).isDirectory() ? "/" : "" );
+            cp += " file://localhost/" + el + ( new File( el ).isDirectory() ? "/" : "" );
         }
 
         Manifest.Attribute attr = new Manifest.Attribute( "Class-Path", cp.trim() );



Re: svn commit: r543178 - /maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java

Posted by Brett Porter <br...@apache.org>.
On 04/06/2007, at 8:59 PM, Kenney Westerhof wrote:

>> JDK compliance.
>
> ah, toURI isn't in 1.4? pff..
>

I think it is - not sure. However we claim 1.3 compatibility for  
Surefire (since it can fork the booter), so we have even stricter  
requirements there.

- Brett

Re: svn commit: r543178 - /maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java

Posted by Kenney Westerhof <ke...@apache.org>.

Brett Porter wrote:
> 
> On 03/06/2007, at 11:04 PM, Kenney Westerhof wrote:
> 
>> Yup, just tested it on linux - works fine, though the url is different 
>> (missing '//localhost/').
>> I don't know wheter this will work on windows.
> 
> Yep, had already tested it there. There are several combinations that 
> work on Windows, but most of them aren't actually legal according to the 
> RFC. I use the localhost version when constructing by hand because it 
> tends to be consistently functional across platforms and matches the RFC.
> 

Yeah, I remember that. Still it's a strange format ;) maybe it uses samba
underwater ;)

>> btw, the jar manifest specification states:
>>
>> Class-Path:
>> The value of this attribute specifies the relative URLs of the 
>> extensions or libraries that this application or extension needs. URLs 
>> are separated by one or more spaces. The application or extension 
>> class loader uses the value of this attribute to construct its 
>> internal search path.
>> so they must be relative.. but still this works. I don't know if this 
>> is really portable, but we'll see.
> 
> Yep, noticed that. I guess we'll see how it goes...
> 
> If this isn't the default mode of operation and we find some JVMs only 
> respect relative paths then I guess we could copy the libs to target and 
> path from there. But only after we encounter a counter-example :)

Exactly ;)

> 
> Anyway, is there really such a thing as a "relative" URL? Doesn't seem 
> very universal :)

:) no it doesn't hehe.. So far it seems urls are accepted. Haven't tried
any urls with other protocols than file though. Could be interesting to
have classpath elements pointing to the central repo..

>> The main issue is that directories MUST end in a /; I just committed a 
>> comment stating that, since it's
>> no longer explicit from the call to UrlUtils(file).toExternalForm(). 
>> (btw, why isn't File.toURI().toURL() called directly?).
> 
> JDK compliance.

ah, toURI isn't in 1.4? pff..

cheers,

  Kenney

> 
> Thanks,
> Brett

Re: svn commit: r543178 - /maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java

Posted by Brett Porter <br...@apache.org>.
On 03/06/2007, at 11:04 PM, Kenney Westerhof wrote:

> Yup, just tested it on linux - works fine, though the url is  
> different (missing '//localhost/').
> I don't know wheter this will work on windows.

Yep, had already tested it there. There are several combinations that  
work on Windows, but most of them aren't actually legal according to  
the RFC. I use the localhost version when constructing by hand  
because it tends to be consistently functional across platforms and  
matches the RFC.

>
> btw, the jar manifest specification states:
>
> Class-Path:
> The value of this attribute specifies the relative URLs of the  
> extensions or libraries that this application or extension needs.  
> URLs are separated by one or more spaces. The application or  
> extension class loader uses the value of this attribute to  
> construct its internal search path.
> so they must be relative.. but still this works. I don't know if  
> this is really portable, but we'll see.

Yep, noticed that. I guess we'll see how it goes...

If this isn't the default mode of operation and we find some JVMs  
only respect relative paths then I guess we could copy the libs to  
target and path from there. But only after we encounter a counter- 
example :)

Anyway, is there really such a thing as a "relative" URL? Doesn't  
seem very universal :)

>
> The main issue is that directories MUST end in a /; I just  
> committed a comment stating that, since it's
> no longer explicit from the call to UrlUtils(file).toExternalForm 
> (). (btw, why isn't File.toURI().toURL() called directly?).

JDK compliance.

Thanks,
Brett

Re: svn commit: r543178 - /maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java

Posted by Kenney Westerhof <ke...@apache.org>.

Brett Porter wrote:
> Hey Kenney,
> 
> On 2.3.1 I'd already replaced this code with UrlUtils which seemed to 
> cover all the cases consistently, so I merged it over to trunk too 
> (sorry, I'd just forgotten to do that previously).
> 
> Can you double check I haven't missed anything by re-running it against 
> where this was causing a problem for you?

Yup, just tested it on linux - works fine, though the url is different (missing '//localhost/').
I don't know wheter this will work on windows.

btw, the jar manifest specification states:

Class-Path:
The value of this attribute specifies the relative URLs of the extensions or libraries that this application or extension needs. URLs are separated by one or more spaces. The application or extension class loader uses the value of this attribute to construct its internal search path. 

so they must be relative.. but still this works. I don't know if this is really portable, but we'll see.

The main issue is that directories MUST end in a /; I just committed a comment stating that, since it's
no longer explicit from the call to UrlUtils(file).toExternalForm(). 
(btw, why isn't File.toURI().toURL() called directly?).

Thanks,

	Kenney

> 
> Thanks,
> Brett
> 
> On 01/06/2007, at 2:24 AM, kenney@apache.org wrote:
> 
>> Author: kenney
>> Date: Thu May 31 09:24:35 2007
>> New Revision: 543178
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=543178
>> Log:
>> Fix 'isDirectory' check; classpath elements that are directories MUST 
>> end in a '/', and 'file://localhost//foo' is NOT a directory according 
>> to java.io.File - it's an url!
>>
>> Modified:
>>     
>> maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java 
>>
>>
>> Modified: 
>> maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java 
>>
>> URL: 
>> http://svn.apache.org/viewvc/maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java?view=diff&rev=543178&r1=543177&r2=543178 
>>
>> ============================================================================== 
>>
>> --- 
>> maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java 
>> (original)
>> +++ 
>> maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java 
>> Thu May 31 09:24:35 2007
>> @@ -229,8 +229,7 @@
>>          for ( Iterator it = classPath.iterator(); it.hasNext(); )
>>          {
>>              String el = (String) it.next();
>> -            el = "file://localhost/" + el;
>> -            cp += " " + el + ( new File( el ).isDirectory() ? "/" : 
>> "" );
>> +            cp += " file://localhost/" + el + ( new File( el 
>> ).isDirectory() ? "/" : "" );
>>          }
>>
>>          Manifest.Attribute attr = new Manifest.Attribute( 
>> "Class-Path", cp.trim() );
>>

Re: svn commit: r543178 - /maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/ForkConfiguration.java

Posted by Brett Porter <br...@apache.org>.
Hey Kenney,

On 2.3.1 I'd already replaced this code with UrlUtils which seemed to  
cover all the cases consistently, so I merged it over to trunk too  
(sorry, I'd just forgotten to do that previously).

Can you double check I haven't missed anything by re-running it  
against where this was causing a problem for you?

Thanks,
Brett

On 01/06/2007, at 2:24 AM, kenney@apache.org wrote:

> Author: kenney
> Date: Thu May 31 09:24:35 2007
> New Revision: 543178
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=543178
> Log:
> Fix 'isDirectory' check; classpath elements that are directories  
> MUST end in a '/', and 'file://localhost//foo' is NOT a directory  
> according to java.io.File - it's an url!
>
> Modified:
>     maven/surefire/trunk/surefire-booter/src/main/java/org/apache/ 
> maven/surefire/booter/ForkConfiguration.java
>
> Modified: maven/surefire/trunk/surefire-booter/src/main/java/org/ 
> apache/maven/surefire/booter/ForkConfiguration.java
> URL: http://svn.apache.org/viewvc/maven/surefire/trunk/surefire- 
> booter/src/main/java/org/apache/maven/surefire/booter/ 
> ForkConfiguration.java?view=diff&rev=543178&r1=543177&r2=543178
> ====================================================================== 
> ========
> --- maven/surefire/trunk/surefire-booter/src/main/java/org/apache/ 
> maven/surefire/booter/ForkConfiguration.java (original)
> +++ maven/surefire/trunk/surefire-booter/src/main/java/org/apache/ 
> maven/surefire/booter/ForkConfiguration.java Thu May 31 09:24:35 2007
> @@ -229,8 +229,7 @@
>          for ( Iterator it = classPath.iterator(); it.hasNext(); )
>          {
>              String el = (String) it.next();
> -            el = "file://localhost/" + el;
> -            cp += " " + el + ( new File( el ).isDirectory() ?  
> "/" : "" );
> +            cp += " file://localhost/" + el + ( new File 
> ( el ).isDirectory() ? "/" : "" );
>          }
>
>          Manifest.Attribute attr = new Manifest.Attribute( "Class- 
> Path", cp.trim() );
>