Bug 847160

Summary: Review Request: eclipse-m2e-core - Maven integration for Eclipse
Product: [Fedora] Fedora Reporter: Gerard Ryan <fedora>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: akurtako, jan.public, jon.vanalten, juan.hernandez, mizdebsk, notting, package-review
Target Milestone: ---Flags: juan.hernandez: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-02-13 04:36:58 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 847157    
Bug Blocks:    

Description Gerard Ryan 2012-08-09 22:58:53 UTC
Spec URL: http://galileo.fedorapeople.org/eclipse-m2e-core/1.1.0-1/eclipse-m2e-core.spec
SRPM URL: http://galileo.fedorapeople.org/eclipse-m2e-core/1.1.0-1/eclipse-m2e-core-1.1.0-1.fc17.src.rpm

Description:
Maven integration for Eclipse

There are a couple of things that may need attention:
Patch5: org.codehaus.plexus.ContainerConfiguration is provided by both plexus-containers-container-default and sisu (different versions or implementations, I'm not sure what). This package is looking for the version that is provided by sisu, but the one that comes from plexus-containers-container-default is the one that maven chooses. The patch modifies a constructor to make it fit, and comments out one other line, so that it compiles successfully. I don't know if this is good, it could cause unexpected results I guess.

This package builds, but I can't verify that it works. To get this working on F17, more patches are needed for both this and maven. I got a successful build on F17, but then when I installed, it didn't actually work (Eclipse seemed to ignore most of it). I don't know what was causing that - if it was something in one of the extra wrangling that was needed to get it to work, or if it was caused by something that's in this srpm. I wasn't able to get a working rawhide installation to test it on.

Fedora Account System Username: galileo

Comment 1 Mikolaj Izdebski 2012-08-10 12:55:53 UTC
Still fails to build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=4376275

cp: cannot stat '%{SOURCE1}': No such file or directory

Comment 2 Gerard Ryan 2012-08-10 14:21:09 UTC
Builds now: http://koji.fedoraproject.org/koji/taskinfo?taskID=4376592

Comment 3 Alexander Kurtakov 2012-08-14 07:13:06 UTC
Gerard, please post new links when you fix things. This is needed so tools like fedora-review can auto-fetch.

Spec URL: http://galileo.fedorapeople.org/eclipse-m2e-core/1.1.0-2/eclipse-m2e-core.spec
SRPM URL: http://galileo.fedorapeople.org/eclipse-m2e-core/1.1.0-2/eclipse-m2e-core-1.1.0-2.fc17.src.rpm

Comment 4 Gerard Ryan 2012-08-14 11:17:12 UTC
(In reply to comment #3)
> Gerard, please post new links when you fix things. This is needed so tools
> like fedora-review can auto-fetch.

Sorry about that, I usually do that, I don't know how I missed it on this occasion!

Comment 5 Roland Grunberg 2012-08-20 17:59:23 UTC
I've tried using eclipse-m2e-core on an f18 machine, but faced some issues :

- slf4j.api needs "org.slf4j.impl;version=1.6.0", which wasn't being found, so I symlinked it into the dropins folder.
- m2e needed org.apache.maven.archetype.{catalog,descriptor} so I symlinked these as well.

These fixes would get the plugins to resolve correctly during startup, and I would be able to see the menu from importing maven projects. However when attempting to import a project, I would see this :

Caused by: java.lang.NoSuchMethodError: org.codehaus.plexus.DefaultPlexusContainer.<init>(Lorg/codehaus/plexus/ContainerConfiguration;)V
	at org.eclipse.m2e.core.internal.MavenPluginActivator.start(MavenPluginActivator.java:178)
	at org.eclipse.osgi.framework.internal.core.BundleContextImpl$1.run(BundleContextImpl.java:711)
	at java.security.AccessController.doPrivileged(Native Method)
	at org.eclipse.osgi.framework.internal.core.BundleContextImpl.startActivator(BundleContextImpl.java:702)
	... 109 more
	
It seems there's 2 jars that provide DefaultPlexusContainer (guice-plexus-shim and plexus-container-default). By removing plexus-container-default, from the runtime plugin's jars, I could get father and import the projects into the workspace, but then fail on this :

java.lang.ClassCastException: org.apache.maven.project.artifact.DefaultMavenMetadataCache cannot be cast to org.eclipse.m2e.core.internal.project.IManagedCache
	at org.eclipse.m2e.core.internal.project.registry.ProjectRegistryManager.flushCaches(ProjectRegistryManager.java:253)
	at org.eclipse.m2e.core.internal.project.registry.ProjectRegistryManager.refresh(ProjectRegistryManager.java:359)
	at org.eclipse.m2e.core.internal.project.registry.ProjectRegistryManager.refresh(ProjectRegistryManager.java:327)
	at org.eclipse.m2e.core.internal.project.registry.ProjectRegistryManager.refresh(ProjectRegistryManager.java:278)
	at org.eclipse.m2e.core.internal.project.registry.MavenProjectManager.refresh(MavenProjectManager.java:58)
	at org.eclipse.m2e.core.internal.builder.MavenBuilder.build(MavenBuilder.java:87)
	...

Comment 6 Gerard Ryan 2012-08-22 00:53:39 UTC
Thanks for the feedback Roland, it's much appreciated! I'll look into that tomorrow. I think it might be caused by one or two of the patches that I made to get it to build.

Comment 7 Roland Grunberg 2012-12-04 16:41:17 UTC
*** Bug 814245 has been marked as a duplicate of this bug. ***

Comment 8 Gerard Ryan 2012-12-04 21:42:00 UTC
Oh yeah, this. I'll see if I can make any progress on it now.

Comment 9 Gerard Ryan 2012-12-06 14:32:47 UTC
Spec URL: http://galileo.fedorapeople.org/eclipse-m2e-core/1.1.0-3/eclipse-m2e-core.spec
SRPM URL: http://galileo.fedorapeople.org/eclipse-m2e-core/1.1.0-3/eclipse-m2e-core-1.1.0-3.fc18.src.rpm

This is the latest that I'm working on. It has all of the changes that Roland had to make in comment #5 (thanks Roland!), and some others that were necessary also to get it that far.

Everything appears to work, but when trying to create a new maven project, an error dialog occurs, caused by the ClassCastException mentioned by Roland above. It looks like the project gets created fine even though the dialog appears, but the problem should still not occur. I'll try to solve this today, there's a chance that I introduced it in one of the patches that I made to get it to build.

Comment 10 Gerard Ryan 2012-12-10 01:46:41 UTC
Spec URL: http://galileo.fedorapeople.org/eclipse-m2e-core/1.1.0-4/eclipse-m2e-core.spec
SRPM URL: http://galileo.fedorapeople.org/eclipse-m2e-core/1.1.0-4/eclipse-m2e-core-1.1.0-4.fc18.src.rpm

This version works fine and fixes all the things. The only problem is, it's no longer the latest version, there's a 1.2.0 out since this thread started. I've tried to build the 1.2.0, but I get InternalErrorExceptions from maven. I'll see if I can find someone who knows about that this week.

Comment 11 Mikolaj Izdebski 2012-12-10 06:44:08 UTC
(In reply to comment #10)
> I've tried to build the 1.2.0, but I get InternalErrorExceptions from maven.
> I'll see if I can find someone who knows about that this week.

If you get some exception thrown by Maven itself (i.e. not plugin) then I should be able to help you investigate what is going on. (If it's a plugin then it depends if I know that particular plugin.). Just ping me in IRC.

Comment 12 Gerard Ryan 2012-12-10 20:30:15 UTC
(In reply to comment #11)

Thanks Mikolaj, I'll probably try to contact you tomorrow some time. You might be able to understand it more with the output[0]. The spec and srpm used to generate that are also there in that folder[1]. Any help you can give is much appreciated!

[0] http://galileo.fedorapeople.org/eclipse-m2e-core/1.2.0-0.1/output.log
[1] http://galileo.fedorapeople.org/eclipse-m2e-core/1.2.0-0.1/

Comment 13 Roland Grunberg 2012-12-10 21:41:37 UTC
I can reproduce this, and the NPE corresponds to 'hash = hash * 31 + version.hashCode();' . It's happening for the creation of a cached key for Plugin [org.codehaus.modello:modello-maven-plugin], which is listed as a dependency of MavenProject: org.eclipse.m2e:org.eclipse.m2e.core:1.2.0-SNAPSHOT. Looking at the pom.xml for m2e.core I see the 'modello.version' property is set but the plugin itself is not given a version, and isn't in pluginManagement in the parent.

If you just set a version for that plugin, maybe it'll fix the issue.

Comment 14 Gerard Ryan 2012-12-11 14:28:55 UTC
(In reply to comment #13)
> If you just set a version for that plugin, maybe it'll fix the issue.

That worked, thanks!

This is now ready for review.

SPEC URL: http://galileo.fedorapeople.org/eclipse-m2e-core/1.2.0-1/eclipse-m2e-core.spec
SRPM URL: http://galileo.fedorapeople.org/eclipse-m2e-core/1.2.0-1/eclipse-m2e-core-1.2.0-1.fc18.src.rpm

Comment 15 Mikolaj Izdebski 2012-12-11 14:38:00 UTC
I also did a licensing review and checked for bundling libraries -- everything is OK now. I also briefly it and basic functionality appears to be forking.

Great job!

Comment 16 Juan Hernández 2013-01-22 16:30:01 UTC
I am doing the review.

Comment 17 Juan Hernández 2013-01-22 17:32:18 UTC
Package Review
==============

Key:
- = N/A
x = Check
! = Problem
? = Not evaluated

=== REQUIRED ITEMS ===
[!]  Rpmlint output:

Output of rpmlint of the source package:

eclipse-m2e-core.src: W: spelling-error %description -l en_US xml -> XML, ml, x ml
eclipse-m2e-core.src: W: invalid-url Source0: http://git.eclipse.org/c/m2e/m2e-core.git/snapshot/1.2.0.20120903-1050.tar.bz2 HTTP Error 404: Not found
1 packages and 0 specfiles checked; 0 errors, 2 warnings.

The spelling warning is acceptable.

It looks like the URL has changed and is this one now:

http://git.eclipse.org/c/m2e/m2e-core.git/snapshot/m2e-core-releases/1.2/1.2.0.20120903-1050.tar.bz2

Output of rpmlint of the binary packages:

rpmlint eclipse-m2e-core-1.2.0-1.fc19.noarch.rpm eclipse-m2e-core-javadoc-1.2.0-1.fc19.noarch.rpm
eclipse-m2e-core.noarch: E: explicit-lib-dependency cglib
eclipse-m2e-core.noarch: W: spelling-error %description -l en_US xml -> XML, ml, x ml
eclipse-m2e-core-javadoc.noarch: E: script-without-shebang /usr/share/javadoc/eclipse-m2e-core/javadoc.sh
2 packages and 0 specfiles checked; 2 errors, 73 warnings.

(I ommited many warnings about dangling symlinks caused by missing Java packages in the environment where I ran rpmlint)

The error about the explicit lib dependency on cglib is a false positive, as cglib is not a library but a regular Java package.

The spelling warning is acceptable.

The error related to the javadoc.sh script can be fixed excluding that file from the package.

[x]  Package is named according to the Package Naming Guidelines[1].
[x]  Spec file name must match the base package name, in the format %{name}.spec.
[x]  Package meets the Packaging Guidelines[2].
[x]  Package successfully compiles and builds into binary rpms.

Built in koji:

http://koji.fedoraproject.org/koji/taskinfo?taskID=4894325

[x]  Buildroot definition is not present
[x]  Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines[3,4].
[x]  License field in the package spec file matches the actual license.
License type:
[x]  If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %doc.
[x]  All independent sub-packages have license of their own
[x]  Spec file is legible and written in American English.
[x]  Sources used to build the package matches the upstream source, as provided in the spec URL.

Checked using a recursive diff of the sources in the package and the sources downloaded from the source URL.

[x]  All build dependencies are listed in BuildRequires, except for any that are listed in the exceptions section of Packaging Guidelines[5].
[x]  Package must own all directories that it creates or must require other packages for directories it uses.
[x]  Package does not contain duplicates in %files.
[x]  File sections do not contain %defattr(-,root,root,-) unless changed with good reason
[x]  Permissions on files are set properly.
[x]  Package does NOT have a %clean section which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). (not needed anymore)
[x]  Package consistently uses macros (no %{buildroot} and $RPM_BUILD_ROOT mixing)
[x]  Package contains code, or permissable content.
[-]  Fully versioned dependency in subpackages, if present.
[-]  Package contains a properly installed %{name}.desktop file if it is a GUI application.
[x]  Package does not own files or directories owned by other packages.
[x]  Javadoc documentation files are generated and included in -javadoc subpackage
[x]  Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlinks)
[x]  Packages have proper BuildRequires/Requires on jpackage-utils
[!]  Javadoc subpackages have Require: jpackage-utils
[x]  Package uses %global not %define
[-]  If package uses tarball from VCS include comment how to re-create that tarball (svn export URL, git clone URL, ...)
[!]  If source tarball includes bundled jar/class files these need to be removed prior to building

The following .jar files need to be removed before building:

org.eclipse.m2e.launching/org.eclipse.m2e.cliresolver30.jar
org.eclipse.m2e.launching/org.eclipse.m2e.cliresolver.jar
org.eclipse.m2e.tests.common/jars/jetty-util-6.1.22.jar
org.eclipse.m2e.tests.common/jars/jetty-6.1.22.jar
org.eclipse.m2e.tests.common/jars/servlet-api-2.5-20081211.jar

[x]  All filenames in rpm packages must be valid UTF-8.
[x]  Jar files are installed to %{_javadir}/%{name}.jar (see [6] for details)

Jar files are installed not installed to %{_javadir}, but to the Eclipse plugins directory and this is expected and acceptable.

[!]  If package contains pom.xml files install it (including depmaps) even when building with ant
[!]  pom files has correct add_maven_depmap

Not sure if this is a hard requirement for Eclipse releated packages.

=== Maven ===
[-]  Use %{_mavenpomdir} macro for placing pom files instead of %{_datadir}/maven2/poms
[-]  If package uses "-Dmaven.test.skip=true" explain why it was needed in a comment
[-]  If package uses custom depmap "-Dmaven.local.depmap.file=*" explain why it's needed in a comment
[x]  Package DOES NOT use %update_maven_depmap in %post/%postun
[x]  Packages DOES NOT have Requires(post) and Requires(postun) on jpackage-utils for %update_maven_depmap macro

=== Other suggestions ===
[x]  If possible use upstream build method (maven/ant/javac)
[x]  Avoid having BuildRequires on exact NVR unless necessary
[x]  Package has BuildArch: noarch (if possible)
[x]  Latest version is packaged.
[x]  Reviewer should test that the package builds in mock.

Tested in Koji:

http://koji.fedoraproject.org/koji/taskinfo?taskID=4894325

=== Issues ===
1. Apparently the URL of the source has changed, please review and fix it if needed.
2. Don't include the javadoc.sh script in the package.
3. Add the jpackage-utils requirement to the javadoc package.
4. Remove the .jar files included in the tarball before building.
5. Install the POM files (I am not sure this is mandatory for Eclipse related packages).

=== Final Notes ===
Address the issues above and I will review again. Thanks for your work Gerard!

[1] https://fedoraproject.org/wiki/Packaging:NamingGuidelines
[2] https://fedoraproject.org/wiki/Packaging:Guidelines
[3] https://fedoraproject.org/wiki/Packaging:LicensingGuidelines
[4] https://fedoraproject.org/wiki/Licensing:Main
[5] https://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2 
[6] https://fedoraproject.org/wiki/Packaging:Java#Filenames

Comment 18 Mikolaj Izdebski 2013-01-22 17:44:53 UTC
Just a note from my side.

(In reply to comment #17)
> [!]  Javadoc subpackages have Require: jpackage-utils

In Fedora Rawhide all javadoc packages have auto-generated "Require: jpackage-utils", there is no need to explicitly specify it in the spec file.

Comment 19 Juan Hernández 2013-01-22 17:46:42 UTC
(In reply to comment #18)
> In Fedora Rawhide all javadoc packages have auto-generated "Require:
> jpackage-utils", there is no need to explicitly specify it in the spec file.

Great, one issue less! Mikolaj, what do you think about the POM files?

Comment 20 Gerard Ryan 2013-01-23 00:53:35 UTC
(In reply to comment #17)
> === Issues ===
> 1. Apparently the URL of the source has changed, please review and fix it if
> needed.
> 2. Don't include the javadoc.sh script in the package.
> 3. Add the jpackage-utils requirement to the javadoc package.

1,2,3 are done, I'll post new spec/srpm when the other issues are clarified.

> 4. Remove the .jar files included in the tarball before building.

The jar files are being removed before building, see line 194:
find -name '*.jar' -delete

Will that suffice?

> 5. Install the POM files (I am not sure this is mandatory for Eclipse related > packages).

Hmm, I'm not sure about this one either. I'll enquire of the Eclipse guys tomorrow. I don't know if they would be used in any case. I've used this package as a dependency in a local maven/tycho build for eclipse-jbosstools-openshift, and it didn't complain about not finding it. I'm guessing tycho uses information from OSGi manifests to resolve dependencies. I'll get clarification before going any further.

(In reply to comment #18) 
> In Fedora Rawhide all javadoc packages have auto-generated "Require:
> jpackage-utils", there is no need to explicitly specify it in the spec file.

Is it still necessary for F18? If so, I'll add it back in.

Comment 21 Alexander Kurtakov 2013-01-23 07:09:56 UTC
about 5. Installing pom files for eclipse plugins is usually not needed because the pom files contain only build information and the dependency information is coming from the OSGi MANIFEST.MF. This makes the pom files sufficient only for building the eclipse plugins as they are missing critical information (e.g. dependencies) and as a result one can not use them in Maven resolution.

Comment 22 Juan Hernández 2013-01-23 07:57:41 UTC
Gerard, regarding the bundled .jar files you are right, I missed that, sorry.

Alexander, thanks for your response, that clears issue 5.

So the only remaining issues are 1, 2 and 3. Gerard, please post new spec and srpms and I will review again.

Comment 23 Mikolaj Izdebski 2013-01-23 11:17:56 UTC
(In reply to comment #20)
> > In Fedora Rawhide all javadoc packages have auto-generated "Require:
> > jpackage-utils", there is no need to explicitly specify it in the spec file.
> 
> Is it still necessary for F18? If so, I'll add it back in.

No, in Fedora 18 you don't need to add explicit "Require: jpackage-utils" either. This feature was added in javapackages-tools-0.9.0-1, Fedora 18 currently has version 0.9.1-1.

Comment 24 Gerard Ryan 2013-01-23 18:15:07 UTC
Spec URL: http://galileo.fedorapeople.org/eclipse-m2e-core/1.2.0-2/eclipse-m2e-core.spec
SRPM URL: http://galileo.fedorapeople.org/eclipse-m2e-core/1.2.0-2/eclipse-m2e-core-1.2.0-2.fc19.src.rpm

> (In reply to comment #17)
> === Issues ===
> 1. Apparently the URL of the source has changed, please review and fix it if 
> needed.

Done.

> 2. Don't include the javadoc.sh script in the package.

Done.

> 3. Add the jpackage-utils requirement to the javadoc package.

No longer necessary, (comment #18 and comment #23 above).

> 4. Remove the .jar files included in the tarball before building.

Done.

> 5. Install the POM files (I am not sure this is mandatory for Eclipse related 
> packages).

Not necessary (comment #21 above).

Thanks!

Comment 25 Juan Hernández 2013-01-23 18:24:21 UTC
Looks good to me. Thanks Gerard!

================
*** APPROVED ***
================

Comment 26 Gerard Ryan 2013-01-23 18:34:41 UTC
New Package SCM Request
=======================
Package Name: eclipse-m2e-core
Short Description: Maven integration for Eclipse
Owners: galileo
Branches: f18
InitialCC:

Comment 27 Gwyn Ciesla 2013-01-23 18:41:12 UTC
Git done (by process-git-requests).

Comment 28 Fedora Update System 2013-01-24 23:41:25 UTC
eclipse-m2e-core-1.2.0-3.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/eclipse-m2e-core-1.2.0-3.fc18

Comment 29 Fedora Update System 2013-01-25 21:26:12 UTC
eclipse-m2e-core-1.2.0-3.fc18 has been pushed to the Fedora 18 testing repository.

Comment 30 Fedora Update System 2013-01-30 03:19:40 UTC
eclipse-m2e-core-1.2.0-4.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/eclipse-m2e-core-1.2.0-4.fc18

Comment 31 Jon VanAlten 2013-02-12 19:22:51 UTC
Hi,

Just adding this to help others who may be trying to get m2e from the update site working in Fedora Eclipse and searching for the error.

I was getting the following error:

loader constraint violation: when resolving interface method "org.eclipse.wst.xml.core.internal.provisional.document.IDOMDocument.getDocumentElement()Lorg/w3c/dom/Element;" the class loader (instance of org/eclipse/osgi/internal/baseadaptor/DefaultClassLoader) of the current class, org/eclipse/m2e/editor/xml/internal/MarkerLocationService, and the class loader (instance of org/eclipse/osgi/internal/baseadaptor/DefaultClassLoader) for resolved class, org/eclipse/wst/xml/core/internal/provisional/document/IDOMDocument, have different Class objects for the type l.provisional.document.IDOMDocument.getDocumentElement()Lorg/w3c/dom/Element; used in the signature

Installing the eclipse-m2e-core-1.2.0-4.fc18 package from updates-testing instead of the plugin from upstream software source makes this issue go away,

Thanks a lot for packaging this very useful plugin!  But please, push it to stable :)

Comment 32 Fedora Update System 2013-02-13 04:37:00 UTC
eclipse-m2e-core-1.2.0-4.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.