maven-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stephen Connolly <stephen.alan.conno...@gmail.com>
Subject Re: Dependency on system property user.dir
Date Mon, 08 Feb 2010 09:58:16 GMT
On 8 February 2010 09:18, Kristian Rosenvold
<kristian.rosenvold@gmail.com>wrote:

> On Mon, Feb 8, 2010 at 2:32 AM, Brett Porter <brett@apache.org> wrote:
>
> >
> > On 07/02/2010, at 6:16 AM, Kristian Rosenvold wrote:
> >
> > > I just discovered that the source of the hideous concurrency problem
> > > I've been tracking for some time is the system property "user.dir".
> >
> >
> > > Surefire basically sets the following three system properties:
> > >
> > > "basedir" = basedir.getAbsolutePath()
> > > "user.dir" = workingDirectory.getAbsolutePath();
> > > "localRepository" =  localRepository.getBasedir();
> >
>
> I may be wrong, it may have been any of the three system properties.
> Commenting
> them out in surefire "solved" my problem, and it does not seem to have any
> immediate
> effect on anything (there is a SUREFIRE-419 attached to one of the
> properties so I'll
> see if that's still relevant).
>
>
>
> > >
> > > These properties are also used inside maven core, and creates
> > > some interesting concurrency problems because by the time surefire sets
> > > these values they may have changed already ;)
> >
> > Where are they used in the core? I thought it had been removed and would
> > only be accessed on the initial set up from the cli.
> >
>
> PluginParameterExpressionEvaluator (line 89)  references "user.dir", which
> was
> what had me immediately suspecting that property. But I am unsure if this
> block
> of code is "live"; there are unit tests using it but upon real execution it
> doesn't
> get executed. I just compiled the following code  (skipTests=true on m3
> itself, since
> it's being used by a single unit test)
>
>        if ( basedir == null )
>        {
>            throw new RuntimeException("user.dir not in use");
>            //basedir = System.getProperty( "user.dir" );
>        }
>
> I ran the full set of integration tests and the surefire tests and they all
> passed, so I think this
> code is dead.
>
> It seems like this forks into 2 different problems:
>
> A) Why/what/where is core picking up the system properties
>
> B) Why is surefire setting them.
>
> My immediate interest is really B. I see that the setting of the system
> properties were
> introduced in r382683 and r606809. Grepping for "System.setProperty" within
> surefire
> reveals that surefire is being very eager to manipulate local environment
> to run in-process, there's more cases SureFireBooter.java line
> 330<
> http://svn.apache.org/viewvc/maven/surefire/trunk/surefire-booter/src/main/java/org/apache/maven/surefire/booter/SurefireBooter.java?view=markup&pathrev=HEAD
> >
>
> It feels to me like the "correct" thing to do is not to manipulate the
> runtime-environment
> with forkmode=never, in other words stop supporting /any/ kind of
> System.setProperty
> for non-forked operation.  This would break backwards compatibility, which
> I
> understand is
> not too popular. An alternate solution would be to disallow forkmode=never
> within concurrent
> running (or just upgrade to forkmode=once with a nice message)
>

I don't think it is acceptable to tweak an explicitly set parameter on a
mojo just to "fix" the user's build (also know as doing a "Hudson m2 project
type").  the default forkmode is once, they must have some good reason for
setting it to never (like they want to run out of permgen space, etc) ;-)

I think it would be acceptable to break a build if forkmode=never and
parallel build=true... i.e. have the maven api's expose the fact that we are
running in a parallel build, and then surefire can bomb the build out saying
that forkmode=never is incompatible with parallel builds... then the user
can decide to switch back to forkmode=once, or not use parallel builds...

I think that there are a number of other plugins which might want to be
aware of a parallel build... e.g. maven-invoker-plugin, jetty-maven-plugin,
etc... so I do think that the maven 3 API's need to have some mechanism of
exposing to plugins that they are running in a parallel build.... how we add
this information in such a way that plugins will still work for 2.x as well
as 3.x is an interesting question

-Stephen



>
>
> > I wouldn't be opposed to Surefire not supporting setting system
> properties
> > in non-forking mode in a future release, it seems like a recipe for
> disaster
> > in an embedded / concurrent environment. I think it was done to encourage
> > some consistency between the forked and non-forked versions.
> >
>
> I'm basically feature-complete with the m3 core for weave mode, and now
> it's
> all down to
> bug-hunting in the actual end-user experience; mvn install.
>
> War-plugin and surefire will be needing patches, and I suppose I'll make
> jira issues
> and fix these once I get a stable version running 24/7. I think there's
> little point in
> committing any code "live" before 24/7 stability can be demonstrated for a
> decent-sized
> group of projects.
>
> Kristian
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message