Bug 986900 - tycho: Incorrect usage of XMvn
tycho: Incorrect usage of XMvn
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: tycho (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Roland Grunberg
Fedora Extras Quality Assurance
: Performance
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-07-22 07:28 EDT by Mikolaj Izdebski
Modified: 2013-08-16 19:02 EDT (History)
3 users (show)

See Also:
Fixed In Version: tycho-0.18.1-4.fc19
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-08-16 19:02:32 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Mikolaj Izdebski 2013-07-22 07:28:09 EDT
Description of problem:
To resolve artifacts tycho seems to invoke xmvn-resolve in a separate JVM for each artifact.  I imagine this has big negative performance impact for all tycho builds.

xmvn-resolve is intended mostly for interactive uses and for scripting.  Java applications should use XMvn Java API through Plexus container.

XMvn uses Plexus container for internal dependency injection and it's usage outsdes of plexus is simply wrong.  Even if it appears that XMvn works without Plexus it may cause some unexpected behavior, like random NPEs, if not now then in future.

The right usage of XMvn resolver is:
1) obtain reference to Plexus container used by Tycho,
2) lookup XMvn Resolver in the container,
3) use the resolver to resolve all artifacts.

This way XMvn is initialized only once and artifact resolution is almost instant.  No separate JVM is created.
Comment 1 Roland Grunberg 2013-07-22 12:55:34 EDT
I'll make the necessary changes. Initially I wanted to just call the corresponding XMvn class directly, but couldn't build against it since it wasn't an OSGi bundle. Getting the requirement through plexus should be much simpler (since that is provided as an OSGi bundle).

Thanks again.
Comment 2 Mikolaj Izdebski 2013-07-22 13:10:36 EDT
XMvn classes should already be available in Plexus, if not then XMvn is not being ran.  There's some pseudo-code how I would implement this.  I can provide patch If needed.

PlexusContainer pc = <get PlexusContainer reference somehow>
MavenSession ms = <get MavenSession reference somehow>
Resolver xmvnResolver = null;
try { xmvnResolver = pc.lookup(Resolver.class); } catch(...)

if (xmvnResolver != null) {
  if (ms.isOffline()) {
    // Upstream Maven in offline mode (mvn --offline)
  }
  else {
    // Upstream Maven in online mode (mvn)
  }
}
else {
  if (ms.isOffline()) {
    // XMvn in offline mode (mvn-rpmbuild or %mvn_build)
  }
  else {
    // XMvn in online mode (mvn-local)
  }
}

This way is:
 1) fast
 2) clean
 3) doesn't require hacks like XMVN_ environmental variables
Comment 3 Roland Grunberg 2013-07-22 17:24:04 EDT
I'm thinking of doing these checks in the TyhoMavenLifecycleParticipant since that's the first thing getting executed (and has access to MavenSession / PlexusContainer ). From there we can set a system property to be used by other sections of code that need to know this state.

To use org.fedoraproject.maven.resolver.Resolver , it would need to be an OSGi bundle, so I think it would be easier to look up org.eclipse.aether.repository.WorkspaceReader , assuming eclipse-aether will eventually make it into rawhide. Would that still be acceptable rather than getting the Resolver itself ?
Comment 4 Mikolaj Izdebski 2013-07-22 23:20:40 EDT
(In reply to Roland Grunberg from comment #3)
> To use org.fedoraproject.maven.resolver.Resolver , it would need to be an
> OSGi bundle, so I think it would be easier to look up
> org.eclipse.aether.repository.WorkspaceReader , assuming eclipse-aether will
> eventually make it into rawhide. Would that still be acceptable rather than
> getting the Resolver itself ?

First if all, I can convert XMvn to OSGi bundle, that's not big a problem.

Secondly, it doesn't need to be OSGi bundle.  Yoiu don't need it during compile time nor runtime if you just need to check for XMvn presence.  You could do comething like that:

  boolean usingXmvn;
  try {
    plexusContainer.lookup("org.fedoraproject.maven.resolver.Resolver");
    usingXmvn = true;
  }
  catch (PlexusCOntainerException e) {
    usingXmvn = false;
  }

Lastly, WorkspaceReader may be better or worse, depending how you actually implement it.  WorkspaceReader is used not only by XMvn, but also other extensions for Maven, like Eclipse M2E.  Implementing it properly would allow Tycho to be used in M2E and could even be upstreamed.  Long term that's probably better solution than maintaining Fedora-specific patch.  I am generally for this solution.

Accessing WorkspaceReader *through Plexus* or Maven is also a proper way of using XMVn resolver.
Comment 5 Roland Grunberg 2013-07-31 09:22:00 EDT
tycho-0.18.1-4.fc20 (http://koji.fedoraproject.org/koji/buildinfo?buildID=439715) should resolve this issue. Access to the resolver is done through reflection.
Comment 6 Fedora Update System 2013-08-02 15:47:45 EDT
tycho-0.18.1-4.fc19,tycho-extras-0.18.1-1.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/tycho-0.18.1-4.fc19,tycho-extras-0.18.1-1.fc19
Comment 7 Fedora Update System 2013-08-03 19:58:49 EDT
Package tycho-0.18.1-4.fc19, tycho-extras-0.18.1-1.fc19:
* should fix your issue,
* was pushed to the Fedora 19 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing tycho-0.18.1-4.fc19 tycho-extras-0.18.1-1.fc19'
as soon as you are able to, then reboot.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2013-14180/tycho-0.18.1-4.fc19,tycho-extras-0.18.1-1.fc19
then log in and leave karma (feedback).
Comment 8 Fedora Update System 2013-08-16 19:02:32 EDT
tycho-0.18.1-4.fc19, tycho-extras-0.18.1-1.fc19 has been pushed to the Fedora 19 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.