Surefire 2.3.1, 2.4 and Shade

[dfabulich]

This is a long e-mail, so I divided it into parts.

Summary: I think we're ready to release 2.3.1, because I can't reproduce
"regression" SUREFIRE-347, the only bug targeted for 2.3.1.  I was able to
reproduce a related bug SUREFIRE-334, and was able to fix it using
maven-shade-plugin alpha-14-SNAPSHOT (even though MSHADE-9 isn't fixed
yet).  If we go ahead and release maven-shade-plugin alpha-14-SNAPSHOT, we
can release Surefire 2.4 as well (or instead).

Part 1: The problem with plexus dependency conflicts
Part 2: Testing the problem
Part 3: How I fixed the problem in trunk

1) The problem with plexus dependency conflicts

A couple of weeks ago I did a bunch of work with an eye toward getting
Surefire 2.4 into a state where it could be released.  The main remaining
issue was the problem of plexus dependency conflicts.  In some cases,
Surefire would add its own plexus dependencies into the classpath of the
tests; if the application under test depended on some other version of
those plexus dependencies, you'd get some gnarly errors that were
difficult to diagnose.

Two JIRA issues were associated with this problem, both of which were
originally filed against Surefire 2.3.1: SUREFIRE-347 [regression: plexus
is not properly isolated] and SUREFIRE-334 [useSystemClassLoader
introduces dependencies].  SUREFIRE-332 [remove dependency on
plexus-utils, use a split out cli package] was another related suggestion,
not targeted for 2.3.1.

(There was also SUREFIRE-335 [forkMode = none is incompatible with
useSystemClassLoader = true], but that one didn't make as much sense.
I've marked it "Won't Fix.")

A few days ago, Mauro suggested that we release Surefire 2.3.1; I'd
pointed out that -347 and -334 were still targeted for 2.3.1 and that we'd
probably want to fix them before releasing 2.3.1.  Right now only -347,
the regression, is targeted for 2.3.1.

2) Testing the problem

I made a simple "plexus-conflict" integration test.  (The Surefire
integration tests are all full Maven projects that we fork a separate
process to run, just like the Core tests.)

In the test project, I depended on plexus-utils 1.0.4 and extended the
Commandline object.  In 1.0.4, Commandline had a protected "envVars"
Vector; in 1.4.x, envVars is a Map.  Surefire depends on plexus-utils
1.4.8; if 1.4.8 were inserted on the classpath instead of the expected
1.0.4, you'd get an error.

Interestingly, when I ran the test in the default mode
(useSystemClassLoader=false) I found that the test passed, just fine, in
both Surefire 2.3 and also Surefire 2.4 Trunk.  I think that means that I
couldn't reproduce the regression bug SUREFIRE-347, the only bug targeted
for Surefire 2.3.1!

I was able to reproduce SUREFIRE-334 with useSystemClassLoader=true in
2.4.  (In 2.3 useSystemClassLoader=true utterly failed, even on the
simplest integration test project; I don't think it works at all in 2.3.)

3) How I fixed the problem in trunk

The way I had imagined we'd fix these issues was to use maven-shade-plugin
to relocate the packages for all Surefire dependencies.  (It only had
three dependencies: plexus-archiver, plexus-utils, and commons-lang.)

Unfortunately, when I tried shade a couple of weeks ago, I bumped straight
into MSHADE-9 [failure to shade/relocate plexus-archiver (interfaces not
properly relocated)] which blocked my use of shade.

However, our dependency on plexus-archiver was very minimal: all we were
doing was creating an empty jar with a MANIFEST.MF file with a
"Class-Path" attribute.  So, with an eye toward simplifying things by
reducing dependencies, and also trying to work around MSHADE-9, I checked
in revision 602583, which ripped out our dependency on plexus-archiver,
replacing it with a single class that just creates a simple jar with a
manifest.

Luckily, with plexus-archiver ripped out, I was able to use
maven-shade-plugin alpha-14-SNAPSHOT to shade both of the remaining
dependencies (plexus-utils and commons-lang).

I think that means that if we release maven-shade-plugin
alpha-14, even without fixing MSHADE-9, we can release Surefire 2.4, which
would make me very happy.  :-)

Thanks for reading!

-Dan

[Mauro Talevi]

Dan Fabulich wrote:

>
> This is a long e-mail, so I divided it into parts.
>
> Summary: I think we're ready to release 2.3.1, because I can't
> reproduce "regression" SUREFIRE-347, the only bug targeted for 2.3.1.  
> I was able to reproduce a related bug SUREFIRE-334, and was able to
> fix it using maven-shade-plugin alpha-14-SNAPSHOT (even though
> MSHADE-9 isn't fixed yet).  If we go ahead and release
> maven-shade-plugin alpha-14-SNAPSHOT, we can release Surefire 2.4 as
> well (or instead).
>
> Part 1: The problem with plexus dependency conflicts
> Part 2: Testing the problem
> Part 3: How I fixed the problem in trunk
>
> 1) The problem with plexus dependency conflicts
>
> A couple of weeks ago I did a bunch of work with an eye toward getting
> Surefire 2.4 into a state where it could be released.  The main
> remaining issue was the problem of plexus dependency conflicts.  In
> some cases, Surefire would add its own plexus dependencies into the
> classpath of the tests; if the application under test depended on some
> other version of those plexus dependencies, you'd get some gnarly
> errors that were difficult to diagnose.
>
> Two JIRA issues were associated with this problem, both of which were
> originally filed against Surefire 2.3.1: SUREFIRE-347 [regression:
> plexus is not properly isolated] and SUREFIRE-334
> [useSystemClassLoader introduces dependencies].  SUREFIRE-332 [remove
> dependency on plexus-utils, use a split out cli package] was another
> related suggestion, not targeted for 2.3.1.
>
> (There was also SUREFIRE-335 [forkMode = none is incompatible with
> useSystemClassLoader = true], but that one didn't make as much sense.
> I've marked it "Won't Fix.")
>
> A few days ago, Mauro suggested that we release Surefire 2.3.1; I'd
> pointed out that -347 and -334 were still targeted for 2.3.1 and that
> we'd probably want to fix them before releasing 2.3.1.  Right now only
> -347, the regression, is targeted for 2.3.1.
>
> 2) Testing the problem
>
> I made a simple "plexus-conflict" integration test.  (The Surefire
> integration tests are all full Maven projects that we fork a separate
> process to run, just like the Core tests.)
>
> In the test project, I depended on plexus-utils 1.0.4 and extended the
> Commandline object.  In 1.0.4, Commandline had a protected "envVars"
> Vector; in 1.4.x, envVars is a Map.  Surefire depends on plexus-utils
> 1.4.8; if 1.4.8 were inserted on the classpath instead of the expected
> 1.0.4, you'd get an error.
>
> Interestingly, when I ran the test in the default mode
> (useSystemClassLoader=false) I found that the test passed, just fine,
> in both Surefire 2.3 and also Surefire 2.4 Trunk.  I think that means
> that I couldn't reproduce the regression bug SUREFIRE-347, the only
> bug targeted for Surefire 2.3.1!
>
> I was able to reproduce SUREFIRE-334 with useSystemClassLoader=true in
> 2.4.  (In 2.3 useSystemClassLoader=true utterly failed, even on the
> simplest integration test project; I don't think it works at all in 2.3.)
>
> 3) How I fixed the problem in trunk
>
> The way I had imagined we'd fix these issues was to use
> maven-shade-plugin to relocate the packages for all Surefire
> dependencies.  (It only had three dependencies: plexus-archiver,
> plexus-utils, and commons-lang.)
>
> Unfortunately, when I tried shade a couple of weeks ago, I bumped
> straight into MSHADE-9 [failure to shade/relocate plexus-archiver
> (interfaces not properly relocated)] which blocked my use of shade.

Could it be due to http://jira.codehaus.org/browse/MSHADE-2?

> However, our dependency on plexus-archiver was very minimal: all we
> were doing was creating an empty jar with a MANIFEST.MF file with a
> "Class-Path" attribute.  So, with an eye toward simplifying things by
> reducing dependencies, and also trying to work around MSHADE-9, I
> checked in revision 602583, which ripped out our dependency on
> plexus-archiver, replacing it with a single class that just creates a
> simple jar with a manifest.
>
> Luckily, with plexus-archiver ripped out, I was able to use
> maven-shade-plugin alpha-14-SNAPSHOT to shade both of the remaining
> dependencies (plexus-utils and commons-lang).
>
> I think that means that if we release maven-shade-plugin alpha-14,
> even without fixing MSHADE-9, we can release Surefire 2.4, which would
> make me very happy.  :-)

+1

[dfabulich]

Mauro Talevi wrote:

> Dan Fabulich wrote:
>> Unfortunately, when I tried shade a couple of weeks ago, I bumped straight
>> into MSHADE-9 [failure to shade/relocate plexus-archiver (interfaces not
>> properly relocated)] which blocked my use of shade.
> Could it be due to http://jira.codehaus.org/browse/MSHADE-2?

Nope, IMO MSHADE-9 is a real bug in Shade's ASM code.  If you look at the
relocated class in a decompiler, you can see that it's malformed, only
partially relocated.

-Dan

[Mauro Talevi]

On Sat, 8 Dec 2007 22:09:45 -0800 (Pacific Standard Time), Dan Fabulich <dan@...> wrote:

> I think that means that if we release maven-shade-plugin
> alpha-14, even without fixing MSHADE-9, we can release Surefire 2.4, which
> would make me very happy.  :-)

Staged alpha-14 at http://people.apache.org/~mauro/staging-repo

When you guys give the thumbs up, I'll copy stage to repo.

Cheers

[dkulp]

On Monday 10 December 2007, Mauro Talevi wrote:
> On Sat, 8 Dec 2007 22:09:45 -0800 (Pacific Standard Time), Dan Fabulich
<dan@...> wrote:
> > I think that means that if we release maven-shade-plugin
> > alpha-14, even without fixing MSHADE-9, we can release Surefire 2.4,
> > which would make me very happy.  :-)
>
> Staged alpha-14 at http://people.apache.org/~mauro/staging-repo
>
> When you guys give the thumbs up, I'll copy stage to repo.
>
> Cheers

You would need to call a vote on dev@maven and have the vote pass.

That said, it looks good to me.

--
J. Daniel Kulp
Principal Engineer
IONA
P: 781-902-8727    C: 508-380-7194
daniel.kulp@...
http://www.dankulp.com/blog

[Mauro Talevi]

On Mon, 10 Dec 2007 13:47:27 -0500, Daniel Kulp <dkulp@...> wrote:

> On Monday 10 December 2007, Mauro Talevi wrote:
>> On Sat, 8 Dec 2007 22:09:45 -0800 (Pacific Standard Time), Dan Fabulich
> <dan@...> wrote:
>> > I think that means that if we release maven-shade-plugin
>> > alpha-14, even without fixing MSHADE-9, we can release Surefire 2.4,
>> > which would make me very happy.  :-)
>>
>> Staged alpha-14 at http://people.apache.org/~mauro/staging-repo
>>
>> When you guys give the thumbs up, I'll copy stage to repo.
>>
>> Cheers
>
> You would need to call a vote on dev@maven and have the vote pass.
>

Well - it was recently agreed that no vote was required for alphas.

> That said, it looks good to me.
>

Cool!

[dfabulich]

Mauro Talevi wrote:

>>> When you guys give the thumbs up, I'll copy stage to repo.

+1 Thumbs up!

I think it's also time to move the code out of sandbox and into trunk, and
call the version beta-1-SNAPSHOT.

>> You would need to call a vote on dev@maven and have the vote pass.
>>
>
> Well - it was recently agreed that no vote was required for alphas.

That's surprising to me...  I'd at least post to dev to make sure you
don't get a -1.

-Dan

[Mauro Talevi]

Dan Fabulich wrote:
>> Well - it was recently agreed that no vote was required for alphas.
>
> That's surprising to me...  I'd at least post to dev to make sure you
> don't get a -1.
I distinctly remember that alphas should be released with more ease and
not the same formality.

But I decided to call for a vote unifying it to the move out of sandbox.

I'll then double check the alpha release process and update the webpage
for reference.

Cheers

[brettporter]

On 11/12/2007, at 9:08 AM, Mauro Talevi wrote:

> Dan Fabulich wrote:
>>> Well - it was recently agreed that no vote was required for alphas.
>>
>> That's surprising to me...  I'd at least post to dev to make sure  
>> you don't get a -1.
> I distinctly remember that alphas should be released with more ease  
> and not the same formality.
>
> But I decided to call for a vote unifying it to the move out of  
> sandbox.
>
> I'll then double check the alpha release process and update the  
> webpage for reference.

Someone suggested that, but I happen to disagree with it (if you need  
less formality, make nightlies accessible) - a releases is a releases  
is a release :) This seems consistent with the Apache foundation  
documentation also.

Cheers,
Brett

[Jason van Zyl-2]

On 10 Dec 07, at 11:10 AM 10 Dec 07, Dan Fabulich wrote:

> Mauro Talevi wrote:
>
>>>> When you guys give the thumbs up, I'll copy stage to repo.
>
> +1 Thumbs up!
>
> I think it's also time to move the code out of sandbox and into  
> trunk, and call the version beta-1-SNAPSHOT.
>
>>> You would need to call a vote on dev@maven and have the vote pass.
>>>
>>
>> Well - it was recently agreed that no vote was required for alphas.
>
> That's surprising to me...  I'd at least post to dev to make sure  
> you don't get a -1.
>

No, I asked that it required 3 +1s and no 72 hours vote. That alphas  
time to community be reduced, and official releases time to community  
be elongated. Not that we not vote.

> -Dan

Thanks,

Jason

----------------------------------------------------------
Jason van Zyl
Founder,  Apache Maven
jason at sonatype dot com
----------------------------------------------------------

People develop abstractions by generalizing from concrete examples.
Every attempt to determine the correct abstraction on paper without
actually developing a running system is doomed to failure. No one
is that smart. A framework is a resuable design, so you develop it by
looking at the things it is supposed to be a design of. The more  
examples
you look at, the more general your framework will be.

-- Ralph Johnson & Don Roberts, Patterns for Evolving Frameworks

[Jason van Zyl-2]

On 10 Dec 07, at 4:04 PM 10 Dec 07, Brett Porter wrote:

>
> On 11/12/2007, at 9:08 AM, Mauro Talevi wrote:
>
>> Dan Fabulich wrote:
>>>> Well - it was recently agreed that no vote was required for alphas.
>>>
>>> That's surprising to me...  I'd at least post to dev to make sure  
>>> you don't get a -1.
>> I distinctly remember that alphas should be released with more ease  
>> and not the same formality.
>>
>> But I decided to call for a vote unifying it to the move out of  
>> sandbox.
>>
>> I'll then double check the alpha release process and update the  
>> webpage for reference.
>
> Someone suggested that, but I happen to disagree with it (if you  
> need less formality, make nightlies accessible) - a releases is a  
> releases is a release :) This seems consistent with the Apache  
> foundation documentation also.
>

A release in terms of legal requirements and packaging sure. Official  
releases should get more attention, while alphas should be expelled as  
fast as humanly possible for feedback. How can you argue against that?  
We probably make 10x releases then anyone else at the ASF. Getting  
something working out to users to try in the least cumbersome way  
should be the goal. Because we don't have a standard default snapshot  
repository it's generally a pain for users to try, so they don't, or  
adding a repository to their POM is enough to stop them.

> Cheers,
> Brett
>

Thanks,

Jason

----------------------------------------------------------
Jason van Zyl
Founder,  Apache Maven
jason at sonatype dot com
----------------------------------------------------------

Simplex sigillum veri. (Simplicity is the seal of truth.)

[brettporter]

On 11/12/2007, at 3:17 PM, Jason van Zyl wrote:

>
> On 10 Dec 07, at 4:04 PM 10 Dec 07, Brett Porter wrote:
>
>>
>> On 11/12/2007, at 9:08 AM, Mauro Talevi wrote:
>>
>>> Dan Fabulich wrote:
>>>>> Well - it was recently agreed that no vote was required for  
>>>>> alphas.
>>>>
>>>> That's surprising to me...  I'd at least post to dev to make sure  
>>>> you don't get a -1.
>>> I distinctly remember that alphas should be released with more  
>>> ease and not the same formality.
>>>
>>> But I decided to call for a vote unifying it to the move out of  
>>> sandbox.
>>>
>>> I'll then double check the alpha release process and update the  
>>> webpage for reference.
>>
>> Someone suggested that, but I happen to disagree with it (if you  
>> need less formality, make nightlies accessible) - a releases is a  
>> releases is a release :) This seems consistent with the Apache  
>> foundation documentation also.
>>
>
> A release in terms of legal requirements and packaging sure.  
> Official releases should get more attention, while alphas should be  
> expelled as fast as humanly possible for feedback. How can you argue  
> against that? We probably make 10x releases then anyone else at the  
> ASF. Getting something working out to users to try in the least  
> cumbersome way should be the goal.

3 +1's is a requirement, 72 hours is a common courtesy to people who  
don't have all day to watch the maven lists. I have no objections on a  
previously-unreleased, still-in-the-sandbox plugin being released  
faster if it makes some sense. I was just clarifying that it isn't any  
less formal in terms of voting, though I guess I did a poor job on the  
clarity aspect.

FWIW, my mindset comes from wanting to eradicate the endless alpha  
mentality from this project. I'd rather see us never do one again and  
make snapshots easier to consume, and final releases happen more  
regularly.

But I really don't need to go into it here because it's irrelevant to  
the release-quality work that Dan, Mauro and others have been doing on  
surefire.

- Brett