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 br...@apache.org on 2007/05/24 11:33:02 UTC

svn commit: r541236 - /maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java

Author: brett
Date: Thu May 24 02:33:01 2007
New Revision: 541236

URL: http://svn.apache.org/viewvc?view=rev&rev=541236
Log:
[SUREFIRE-326] make it the default to use the system classloader, for consistency with surefire 2.2

Modified:
    maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java

Modified: maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java
URL: http://svn.apache.org/viewvc/maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java?view=diff&rev=541236&r1=541235&r2=541236
==============================================================================
--- maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java (original)
+++ maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java Thu May 24 02:33:01 2007
@@ -379,7 +379,7 @@
      * forking. Prevents problems with JDKs which implement the service provider lookup mechanism by using
      * the system's classloader.
      *
-     * @parameter expression="${surefire.useSystemClassLoader}" default-value="false"
+     * @parameter expression="${surefire.useSystemClassLoader}" default-value="true"
      */
     private boolean useSystemClassLoader;
 



Re: useSystemClassLoader problems

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

>
>
> Brett Porter wrote:
>> Thanks Kenney!
>> Please make sure to merge that up to the 2.3.x branch - this will  
>> close out SUREFIRE-335.
>
> Merged trunk to branch. I merged the last 3 revisions from trunk;  
> the one before
> that was a merge from you from branch to trunk. It cleanly installs  
> so I hope
> I merged the correct things.

Perfect, thanks. Sorry, I'd been doing it back to front for a bit :)

>
> Haven't deployed it yet; I'd rather let you do it after you check  
> SUREFIRe-335.

Seems to be exactly what you've fixed. I'll give it a spin.

Cheers,
Brett


Re: useSystemClassLoader problems

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

Brett Porter wrote:
> Thanks Kenney!
> 
> Please make sure to merge that up to the 2.3.x branch - this will close 
> out SUREFIRE-335.

Merged trunk to branch. I merged the last 3 revisions from trunk; the one before
that was a merge from you from branch to trunk. It cleanly installs so I hope
I merged the correct things.

Haven't deployed it yet; I'd rather let you do it after you check SUREFIRe-335.

-- Kenney

> 
> Cheers,
> Brett
> 
> On 04/06/2007, at 9:43 PM, Kenney Westerhof wrote:
> 
>>
>>
>> Kenney Westerhof wrote:
>>
>>> Brett Porter wrote:
>>
>> [snip]
>>
>>>> However, I'm rethinking setting it to default (which I did) - seems 
>>>> best to retain it as false. I did it because of another regression 
>>>> where clases in the system classloader weren't found in tests by 
>>>> default any more. I think that's just an omission in creating the 
>>>> isolated classloader that can be fixed differently.
>>
>> I'm setting it back to false now.
>>
>> I found the cause of the regression - due to the fact that the default 
>> for useSystemClassloader was true,
>> this bug was made evident.
>>
>> The combination of forkmode=never and useSystemClassLoader produced 
>> illegal classloaders.
>> Surefirebooter itself didn't check wheter it was forking, and just 
>> used the system classloader,
>> even if it wasn't forking. Ofcourse, when it uses the system 
>> classloader, it can't add the testcases.
>> The usesystemclassloader basically means 'don't construct a 
>> classloader for the tests, they
>> are already present in the system classloader'.
>>
>> I refactored surefirebooter to include the usesystemclassloader in the 
>> forkConfiguration.
>> the 'isUseSystemClassLoader' method checks wheter forking is enabled 
>> in addittion to the useSCL flag.
>>
>> I think this fixes a lot of regressions.
>>
>> Note: by moving the flag to the fork configuration, i have to 
>> construct a new forkconfig in main()
>> otherwise it NPE's. I set the forkmode to never and the useSystemCL 
>> flag accordingly. In the runsuitesinprocess
>> only the forkconfig.useSystemCL is used.
>> Design-wise, the configuration used in the runSuitesInProcess methods 
>> should be different from the
>> global config and ideally the useSystemCL should be present globally, 
>> or wrapped in another class.
>> However, having 2 places to look for a flag is error prone. I know 
>> this design may not be the best
>> avail, but at least now there's no difference anymore between calling 
>> surefirebooter from main() or via run().
>>
>> I deployed new snapshots of surefire for people to test. I hope this 
>> fixes your regressions.
>>
>> -- Kenney

Re: useSystemClassLoader problems

Posted by Brett Porter <br...@apache.org>.
Thanks Kenney!

Please make sure to merge that up to the 2.3.x branch - this will  
close out SUREFIRE-335.

Cheers,
Brett

On 04/06/2007, at 9:43 PM, Kenney Westerhof wrote:

>
>
> Kenney Westerhof wrote:
>
>> Brett Porter wrote:
>
> [snip]
>
>>> However, I'm rethinking setting it to default (which I did) -  
>>> seems best to retain it as false. I did it because of another  
>>> regression where clases in the system classloader weren't found  
>>> in tests by default any more. I think that's just an omission in  
>>> creating the isolated classloader that can be fixed differently.
>
> I'm setting it back to false now.
>
> I found the cause of the regression - due to the fact that the  
> default for useSystemClassloader was true,
> this bug was made evident.
>
> The combination of forkmode=never and useSystemClassLoader produced  
> illegal classloaders.
> Surefirebooter itself didn't check wheter it was forking, and just  
> used the system classloader,
> even if it wasn't forking. Ofcourse, when it uses the system  
> classloader, it can't add the testcases.
> The usesystemclassloader basically means 'don't construct a  
> classloader for the tests, they
> are already present in the system classloader'.
>
> I refactored surefirebooter to include the usesystemclassloader in  
> the forkConfiguration.
> the 'isUseSystemClassLoader' method checks wheter forking is  
> enabled in addittion to the useSCL flag.
>
> I think this fixes a lot of regressions.
>
> Note: by moving the flag to the fork configuration, i have to  
> construct a new forkconfig in main()
> otherwise it NPE's. I set the forkmode to never and the useSystemCL  
> flag accordingly. In the runsuitesinprocess
> only the forkconfig.useSystemCL is used.
> Design-wise, the configuration used in the runSuitesInProcess  
> methods should be different from the
> global config and ideally the useSystemCL should be present  
> globally, or wrapped in another class.
> However, having 2 places to look for a flag is error prone. I know  
> this design may not be the best
> avail, but at least now there's no difference anymore between  
> calling surefirebooter from main() or via run().
>
> I deployed new snapshots of surefire for people to test. I hope  
> this fixes your regressions.
>
> -- Kenney


Re: useSystemClassLoader problems

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

Kenney Westerhof wrote:

> Brett Porter wrote:

[snip]

>> However, I'm rethinking setting it to default (which I did) - seems 
>> best to retain it as false. I did it because of another regression 
>> where clases in the system classloader weren't found in tests by 
>> default any more. I think that's just an omission in creating the 
>> isolated classloader that can be fixed differently.

I'm setting it back to false now.

I found the cause of the regression - due to the fact that the default for useSystemClassloader was true,
this bug was made evident.

The combination of forkmode=never and useSystemClassLoader produced illegal classloaders.
Surefirebooter itself didn't check wheter it was forking, and just used the system classloader,
even if it wasn't forking. Ofcourse, when it uses the system classloader, it can't add the testcases.
The usesystemclassloader basically means 'don't construct a classloader for the tests, they
are already present in the system classloader'.

I refactored surefirebooter to include the usesystemclassloader in the forkConfiguration.
the 'isUseSystemClassLoader' method checks wheter forking is enabled in addittion to the useSCL flag.

I think this fixes a lot of regressions.

Note: by moving the flag to the fork configuration, i have to construct a new forkconfig in main()
otherwise it NPE's. I set the forkmode to never and the useSystemCL flag accordingly. In the runsuitesinprocess
only the forkconfig.useSystemCL is used.
Design-wise, the configuration used in the runSuitesInProcess methods should be different from the
global config and ideally the useSystemCL should be present globally, or wrapped in another class.
However, having 2 places to look for a flag is error prone. I know this design may not be the best
avail, but at least now there's no difference anymore between calling surefirebooter from main() or via run().

I deployed new snapshots of surefire for people to test. I hope this fixes your regressions.

-- Kenney

Re: useSystemClassLoader problems

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

Brett Porter wrote:
> Sorry, read these out of order with my other response :)
> 
> On 01/06/2007, at 2:29 AM, Kenney Westerhof wrote:
> 
>> I'll have to see what the issues are, but there are some usecases for 
>> flat classloading
>> apps, where surefire needs everything in the system classloader. I'm 
>> sure you understand
>> the need for this feature - i've tried to convince people otherwise 
>> but finally saw that
>> it was a valid usecase.
> 
> Ok, makes sense. It's more consistent with how IDEs run it too.
> 
> However, I'm rethinking setting it to default (which I did) - seems best 
> to retain it as false. I did it because of another regression where 
> clases in the system classloader weren't found in tests by default any 
> more. I think that's just an omission in creating the isolated 
> classloader that can be fixed differently.

Hm, that shouldn't have anything to do with this approach. Surefire and
the test classes are (IIRC) all in the system classloader - at least according
to the manifest. I think there are no extra classloaders created when running
with useSystemClassLoader=true; at least that should be the case.

The regressions may have to do with the issue I described in the other mail, that the
directories need to end in /; test-classes are typically located in target/test-classes
so this may be the issue there.

Anyway, I think useSystemClassLoader should be false by default, as it's not really safe. 
See below.

>>> On 25/05/2007, at 5:01 PM, Brett Porter wrote:
>>>> I'm starting to find a number of bugs related to 
>>>> useSystemClassLoader, by virtue of it being a forked JAR call. I'd 
>>>> be interested to get more input on why this route was taken and how 
>>>> the classloading in general works now (would be a handy document to 
>>>> have).
>>
>> See above for the reason of this solution.
>>
>> Which bugs exactly?
> 
> There were a couple I fixed up already (not working on windows because 
> of the urls in the manifest, seem to be fine now which is the urlutils 
> thing I used - hope it still works for you). There are 334 (easy to fix) 
> and 335 (doesn't look critical).

334: indeed, if you run all in the system classloader, surefire and it's dependencies
are present there too. There's no real easy way around this. I can think of two solutions:

1) reorder the manifest classpath to put surefire last, perhaps the booter first. This may
   introduce conflicts when running this on surefire itself, or when the tests include a dep
   on anything surefire uses. Since 2 classpaths are merged, there are duplicate entries. If you
   filter them out, which one should you take - the one surefire mentions, so surefire works, but the
   tests are invalid, or the one the project declares, where surefire may no longer work.

2) Add a single class without dependencies to the test classpath as the Main (so not surefire-booter),
   that reads the surefire*.tmp files and constructs a new classloader (child of system), containing
   surefire and surefirebooter.
   Also put no surefire deps in the test classpath. Either create a new project (with the single class and no deps)
   and add that as a dep to the jar classpath, or copy the class into the test-classes directory (from
   the surefire-booter project, using getResourceAsstream("org/apache/maven/surefire/Main.class") or something.

I think the 2nd option is best - the jar itself should not contain any surefire classes, except the main one.

wdyt? We can also just say 'dont use this except you're dealing with legacy apps that don't understand classloading',
which rules out all maven code. The usesystemclasspath

issue 335: must be the '/' bug i fixed (which was not ported to trunk).
Do you have a testcase; can you reproduce this?

> I'd definitely be interested in getting together a doc on how the 
> classloading works though. I can certainly do it by investigation, but 
> if you have any thoughts you could jot down that'd be great :)

Sure, but it may require some more changes..

The idea is that there are 2 classloaders; the parent containing the tests, the child containing surefire,
as to avoid polution. The child should have 'childFirst' set to true (so the surefire loader is consulted first,
then the test classloader. 
For testing surefire itself, you'll have the same setup and it should work fine, as the tests themselves
are only present in the parent classloader. When those tests load surefire classes, they come from the parent,
so the correct classes are tested. The classes in the surefire child classloader are invisible to the parent
and will not pollute the parent. That's the idea.

There are currently 3 operating modes:
- forkmode none
    parent loader: isolated (no parent, not even system) classloader with tests
    child loader: parent=parent loader, configured by plugin/booter, containing surefire.
- forkmode once/pertest, usesystemclassloader=false
  have to check the code for this one, but i think it's something like this:
    system classloader: contains surefire booter+deps
    parent loader: isolated (no parent) loader with test classes
    child loader: parent=parent loader, contains surefire provider (junit etc)
- forkmode once/pertest, usesystemcl=true
    system classloader: contains tests and surefire booter + deps:
       surefire-booter
       surefire-api
       plexus-utils
       commons-lang
       plexus-archiver
    child loader: parent is system loader, contains surefire provider+deps

That's how it is right now, but the code may be different atm - it's been a while since I worked on this.

If you have any more questions or need some help, don't hesitate to ask.

Cheers,

   Kenney

>>>>> Also, can you check whether 
>>>>> http://jira.codehaus.org/browse/SUREFIRE-117?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel 
>>>>> should be closed as fixed as it was originally, and a new issue 
>>>>> created for their problems, or if it wasn't in fact fixed?
>>
>> It was fixed; i asked for feedback there. No idea why it doesn't work 
>> for them.
>> I do however know there were issues with the surefire plugin, but 
>> haven't seen those problems since.
> 
> Cool, I'll take another look.
> 
> Cheers,
> Brett

Re: useSystemClassLoader problems

Posted by Brett Porter <br...@apache.org>.
Sorry, read these out of order with my other response :)

On 01/06/2007, at 2:29 AM, Kenney Westerhof wrote:

> I'll have to see what the issues are, but there are some usecases  
> for flat classloading
> apps, where surefire needs everything in the system classloader.  
> I'm sure you understand
> the need for this feature - i've tried to convince people otherwise  
> but finally saw that
> it was a valid usecase.

Ok, makes sense. It's more consistent with how IDEs run it too.

However, I'm rethinking setting it to default (which I did) - seems  
best to retain it as false. I did it because of another regression  
where clases in the system classloader weren't found in tests by  
default any more. I think that's just an omission in creating the  
isolated classloader that can be fixed differently.

>> On 25/05/2007, at 5:01 PM, Brett Porter wrote:
>>> I'm starting to find a number of bugs related to  
>>> useSystemClassLoader, by virtue of it being a forked JAR call.  
>>> I'd be interested to get more input on why this route was taken  
>>> and how the classloading in general works now (would be a handy  
>>> document to have).
>
> See above for the reason of this solution.
>
> Which bugs exactly?

There were a couple I fixed up already (not working on windows  
because of the urls in the manifest, seem to be fine now which is the  
urlutils thing I used - hope it still works for you). There are 334  
(easy to fix) and 335 (doesn't look critical).

I'd definitely be interested in getting together a doc on how the  
classloading works though. I can certainly do it by investigation,  
but if you have any thoughts you could jot down that'd be great :)

>
>>>> Also, can you check whether http://jira.codehaus.org/browse/ 
>>>> SUREFIRE-117? 
>>>> page=com.atlassian.jira.plugin.system.issuetabpanels:all- 
>>>> tabpanel should be closed as fixed as it was originally, and a  
>>>> new issue created for their problems, or if it wasn't in fact  
>>>> fixed?
>
> It was fixed; i asked for feedback there. No idea why it doesn't  
> work for them.
> I do however know there were issues with the surefire plugin, but  
> haven't seen those problems since.

Cool, I'll take another look.

Cheers,
Brett

Re: useSystemClassLoader problems

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

Brett Porter wrote:
> Hi Kenney,
> 
> Would be interested to get your thoughts on the issues being seen with 
> useSystemClassLoader and the reasoning for going with a JAR to execute 
> it that way. Before I can continue with the release I need to put them 
> to bed.

I'll have to see what the issues are, but there are some usecases for flat classloading
apps, where surefire needs everything in the system classloader. I'm sure you understand
the need for this feature - i've tried to convince people otherwise but finally saw that
it was a valid usecase.

I'm using the jar solution (pretty nifty, if I say so myself ;) to circumvent the
'commandline too long' problem on most architectures.
The problem is, that in order to have all dependencies in the system classloader, they need to be specified
after the -classpath argument. Having a metadata file and a custom classloader won't work here.
The commandline gets long pretty quick, so I write an empty jar with MANIFEST.MF file containing all
the classpath elements, and then just run java -jar on that jar. 
(You can even distribute that jar as a standalone testcase, as long as the artifacts are present in the local repo
and no workspace locations are used - thought that might be something we may explore in the future).

> - Brett
> 
> On 25/05/2007, at 5:01 PM, Brett Porter wrote:
> 
>> I'm starting to find a number of bugs related to useSystemClassLoader, 
>> by virtue of it being a forked JAR call. I'd be interested to get more 
>> input on why this route was taken and how the classloading in general 
>> works now (would be a handy document to have).

See above for the reason of this solution.

Which bugs exactly?

>> On 24/05/2007, at 7:39 PM, Brett Porter wrote:
>>
>>> Hi Kenney,
>>>
>>> Are you able to comment on whether this is a correct solution? You 
>>> added this feature in 
>>> http://svn.apache.org/viewvc?view=rev&revision=489098

Solution to what? The stuff you merged was thorougly tested so it should be ok.

update, just checked trunk and somebody broke it. It uses urls now, which is wrong. This is not the same
output as is on the jira issue btw.

>>> Also, can you check whether 
>>> http://jira.codehaus.org/browse/SUREFIRE-117?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel 
>>> should be closed as fixed as it was originally, and a new issue 
>>> created for their problems, or if it wasn't in fact fixed?

It was fixed; i asked for feedback there. No idea why it doesn't work for them.
I do however know there were issues with the surefire plugin, but haven't seen those problems since.

I'll fix up trunk to work like it did before again, then deploy and let's see if 2.4 works then.

-- Kenney

>>>
>>> Thanks!
>>>
>>> - Brett
>>>
>>> On 24/05/2007, at 7:33 PM, brett@apache.org wrote:
>>>
>>>> Author: brett
>>>> Date: Thu May 24 02:33:01 2007
>>>> New Revision: 541236
>>>>
>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=541236
>>>> Log:
>>>> [SUREFIRE-326] make it the default to use the system classloader, 
>>>> for consistency with surefire 2.2
>>>>
>>>> Modified:
>>>>     
>>>> maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java 
>>>>
>>>>
>>>> Modified: 
>>>> maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java 
>>>>
>>>> URL: 
>>>> http://svn.apache.org/viewvc/maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java?view=diff&rev=541236&r1=541235&r2=541236 
>>>>
>>>> ============================================================================== 
>>>>
>>>> --- 
>>>> maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java 
>>>> (original)
>>>> +++ 
>>>> maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java 
>>>> Thu May 24 02:33:01 2007
>>>> @@ -379,7 +379,7 @@
>>>>       * forking. Prevents problems with JDKs which implement the 
>>>> service provider lookup mechanism by using
>>>>       * the system's classloader.
>>>>       *
>>>> -     * @parameter expression="${surefire.useSystemClassLoader}" 
>>>> default-value="false"
>>>> +     * @parameter expression="${surefire.useSystemClassLoader}" 
>>>> default-value="true"
>>>>       */
>>>>      private boolean useSystemClassLoader;
>>>>
>>>>


Re: useSystemClassLoader problems

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

Brett Porter wrote:
> Hi Kenney,
> 
> Would be interested to get your thoughts on the issues being seen with 
> useSystemClassLoader and the reasoning for going with a JAR to execute 
> it that way. Before I can continue with the release I need to put them 
> to bed.

I'll have to see what the issues are, but there are some usecases for flat classloading
apps, where surefire needs everything in the system classloader. I'm sure you understand
the need for this feature - i've tried to convince people otherwise but finally saw that
it was a valid usecase.

I'm using the jar solution (pretty nifty, if I say so myself ;) to circumvent the
'commandline too long' problem on most architectures.
The problem is, that in order to have all dependencies in the system classloader, they need to be specified
after the -classpath argument. Having a metadata file and a custom classloader won't work here.
The commandline gets long pretty quick, so I write an empty jar with MANIFEST.MF file containing all
the classpath elements, and then just run java -jar on that jar. 
(You can even distribute that jar as a standalone testcase, as long as the artifacts are present in the local repo
and no workspace locations are used - thought that might be something we may explore in the future).

> - Brett
> 
> On 25/05/2007, at 5:01 PM, Brett Porter wrote:
> 
>> I'm starting to find a number of bugs related to useSystemClassLoader, 
>> by virtue of it being a forked JAR call. I'd be interested to get more 
>> input on why this route was taken and how the classloading in general 
>> works now (would be a handy document to have).

See above for the reason of this solution.

Which bugs exactly?

>> On 24/05/2007, at 7:39 PM, Brett Porter wrote:
>>
>>> Hi Kenney,
>>>
>>> Are you able to comment on whether this is a correct solution? You 
>>> added this feature in 
>>> http://svn.apache.org/viewvc?view=rev&revision=489098

Solution to what? The stuff you merged was thorougly tested so it should be ok.

update, just checked trunk and somebody broke it. It uses urls now, which is wrong.

>>> Also, can you check whether 
>>> http://jira.codehaus.org/browse/SUREFIRE-117?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel 
>>> should be closed as fixed as it was originally, and a new issue 
>>> created for their problems, or if it wasn't in fact fixed?

It was fixed; i asked for feedback there. No idea why it doesn't work for them.
I do however know there were issues with the surefire plugin, but haven't seen those problems since.



>>>
>>> Thanks!
>>>
>>> - Brett
>>>
>>> On 24/05/2007, at 7:33 PM, brett@apache.org wrote:
>>>
>>>> Author: brett
>>>> Date: Thu May 24 02:33:01 2007
>>>> New Revision: 541236
>>>>
>>>> URL: http://svn.apache.org/viewvc?view=rev&rev=541236
>>>> Log:
>>>> [SUREFIRE-326] make it the default to use the system classloader, 
>>>> for consistency with surefire 2.2
>>>>
>>>> Modified:
>>>>     
>>>> maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java 
>>>>
>>>>
>>>> Modified: 
>>>> maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java 
>>>>
>>>> URL: 
>>>> http://svn.apache.org/viewvc/maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java?view=diff&rev=541236&r1=541235&r2=541236 
>>>>
>>>> ============================================================================== 
>>>>
>>>> --- 
>>>> maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java 
>>>> (original)
>>>> +++ 
>>>> maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java 
>>>> Thu May 24 02:33:01 2007
>>>> @@ -379,7 +379,7 @@
>>>>       * forking. Prevents problems with JDKs which implement the 
>>>> service provider lookup mechanism by using
>>>>       * the system's classloader.
>>>>       *
>>>> -     * @parameter expression="${surefire.useSystemClassLoader}" 
>>>> default-value="false"
>>>> +     * @parameter expression="${surefire.useSystemClassLoader}" 
>>>> default-value="true"
>>>>       */
>>>>      private boolean useSystemClassLoader;
>>>>
>>>>


useSystemClassLoader problems

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

Would be interested to get your thoughts on the issues being seen  
with useSystemClassLoader and the reasoning for going with a JAR to  
execute it that way. Before I can continue with the release I need to  
put them to bed.

- Brett

On 25/05/2007, at 5:01 PM, Brett Porter wrote:

> I'm starting to find a number of bugs related to  
> useSystemClassLoader, by virtue of it being a forked JAR call. I'd  
> be interested to get more input on why this route was taken and how  
> the classloading in general works now (would be a handy document to  
> have).
>
> Cheers,
> Brett
>
> On 24/05/2007, at 7:39 PM, Brett Porter wrote:
>
>> Hi Kenney,
>>
>> Are you able to comment on whether this is a correct solution? You  
>> added this feature in http://svn.apache.org/viewvc? 
>> view=rev&revision=489098
>>
>> Also, can you check whether http://jira.codehaus.org/browse/ 
>> SUREFIRE-117? 
>> page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel  
>> should be closed as fixed as it was originally, and a new issue  
>> created for their problems, or if it wasn't in fact fixed?
>>
>> Thanks!
>>
>> - Brett
>>
>> On 24/05/2007, at 7:33 PM, brett@apache.org wrote:
>>
>>> Author: brett
>>> Date: Thu May 24 02:33:01 2007
>>> New Revision: 541236
>>>
>>> URL: http://svn.apache.org/viewvc?view=rev&rev=541236
>>> Log:
>>> [SUREFIRE-326] make it the default to use the system classloader,  
>>> for consistency with surefire 2.2
>>>
>>> Modified:
>>>     maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/ 
>>> src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java
>>>
>>> Modified: maven/surefire/branches/surefire-2.3.x/maven-surefire- 
>>> plugin/src/main/java/org/apache/maven/plugin/surefire/ 
>>> SurefirePlugin.java
>>> URL: http://svn.apache.org/viewvc/maven/surefire/branches/ 
>>> surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/ 
>>> maven/plugin/surefire/SurefirePlugin.java? 
>>> view=diff&rev=541236&r1=541235&r2=541236
>>> ==================================================================== 
>>> ==========
>>> --- maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/ 
>>> src/main/java/org/apache/maven/plugin/surefire/ 
>>> SurefirePlugin.java (original)
>>> +++ maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/ 
>>> src/main/java/org/apache/maven/plugin/surefire/ 
>>> SurefirePlugin.java Thu May 24 02:33:01 2007
>>> @@ -379,7 +379,7 @@
>>>       * forking. Prevents problems with JDKs which implement the  
>>> service provider lookup mechanism by using
>>>       * the system's classloader.
>>>       *
>>> -     * @parameter expression="${surefire.useSystemClassLoader}"  
>>> default-value="false"
>>> +     * @parameter expression="${surefire.useSystemClassLoader}"  
>>> default-value="true"
>>>       */
>>>      private boolean useSystemClassLoader;
>>>
>>>

Re: svn commit: r541236 - /maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java

Posted by Brett Porter <br...@apache.org>.
I'm starting to find a number of bugs related to  
useSystemClassLoader, by virtue of it being a forked JAR call. I'd be  
interested to get more input on why this route was taken and how the  
classloading in general works now (would be a handy document to have).

Cheers,
Brett

On 24/05/2007, at 7:39 PM, Brett Porter wrote:

> Hi Kenney,
>
> Are you able to comment on whether this is a correct solution? You  
> added this feature in http://svn.apache.org/viewvc? 
> view=rev&revision=489098
>
> Also, can you check whether http://jira.codehaus.org/browse/ 
> SUREFIRE-117? 
> page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel  
> should be closed as fixed as it was originally, and a new issue  
> created for their problems, or if it wasn't in fact fixed?
>
> Thanks!
>
> - Brett
>
> On 24/05/2007, at 7:33 PM, brett@apache.org wrote:
>
>> Author: brett
>> Date: Thu May 24 02:33:01 2007
>> New Revision: 541236
>>
>> URL: http://svn.apache.org/viewvc?view=rev&rev=541236
>> Log:
>> [SUREFIRE-326] make it the default to use the system classloader,  
>> for consistency with surefire 2.2
>>
>> Modified:
>>     maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/ 
>> src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java
>>
>> Modified: maven/surefire/branches/surefire-2.3.x/maven-surefire- 
>> plugin/src/main/java/org/apache/maven/plugin/surefire/ 
>> SurefirePlugin.java
>> URL: http://svn.apache.org/viewvc/maven/surefire/branches/ 
>> surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/ 
>> maven/plugin/surefire/SurefirePlugin.java? 
>> view=diff&rev=541236&r1=541235&r2=541236
>> ===================================================================== 
>> =========
>> --- maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/ 
>> src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java  
>> (original)
>> +++ maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/ 
>> src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java  
>> Thu May 24 02:33:01 2007
>> @@ -379,7 +379,7 @@
>>       * forking. Prevents problems with JDKs which implement the  
>> service provider lookup mechanism by using
>>       * the system's classloader.
>>       *
>> -     * @parameter expression="${surefire.useSystemClassLoader}"  
>> default-value="false"
>> +     * @parameter expression="${surefire.useSystemClassLoader}"  
>> default-value="true"
>>       */
>>      private boolean useSystemClassLoader;
>>
>>

Re: svn commit: r541236 - /maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java

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

Are you able to comment on whether this is a correct solution? You  
added this feature in http://svn.apache.org/viewvc? 
view=rev&revision=489098

Also, can you check whether http://jira.codehaus.org/browse/ 
SUREFIRE-117?page=com.atlassian.jira.plugin.system.issuetabpanels:all- 
tabpanel should be closed as fixed as it was originally, and a new  
issue created for their problems, or if it wasn't in fact fixed?

Thanks!

- Brett

On 24/05/2007, at 7:33 PM, brett@apache.org wrote:

> Author: brett
> Date: Thu May 24 02:33:01 2007
> New Revision: 541236
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=541236
> Log:
> [SUREFIRE-326] make it the default to use the system classloader,  
> for consistency with surefire 2.2
>
> Modified:
>     maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/ 
> src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java
>
> Modified: maven/surefire/branches/surefire-2.3.x/maven-surefire- 
> plugin/src/main/java/org/apache/maven/plugin/surefire/ 
> SurefirePlugin.java
> URL: http://svn.apache.org/viewvc/maven/surefire/branches/ 
> surefire-2.3.x/maven-surefire-plugin/src/main/java/org/apache/maven/ 
> plugin/surefire/SurefirePlugin.java? 
> view=diff&rev=541236&r1=541235&r2=541236
> ====================================================================== 
> ========
> --- maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/ 
> src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java  
> (original)
> +++ maven/surefire/branches/surefire-2.3.x/maven-surefire-plugin/ 
> src/main/java/org/apache/maven/plugin/surefire/SurefirePlugin.java  
> Thu May 24 02:33:01 2007
> @@ -379,7 +379,7 @@
>       * forking. Prevents problems with JDKs which implement the  
> service provider lookup mechanism by using
>       * the system's classloader.
>       *
> -     * @parameter expression="${surefire.useSystemClassLoader}"  
> default-value="false"
> +     * @parameter expression="${surefire.useSystemClassLoader}"  
> default-value="true"
>       */
>      private boolean useSystemClassLoader;
>
>