Bug 847160
Summary: | Review Request: eclipse-m2e-core - Maven integration for Eclipse | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Gerard Ryan <fedora> |
Component: | Package Review | Assignee: | 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: | rawhide | CC: | 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
Still fails to build: https://koji.fedoraproject.org/koji/taskinfo?taskID=4376275 cp: cannot stat '%{SOURCE1}': No such file or directory 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 (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! 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) ... 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. *** Bug 814245 has been marked as a duplicate of this bug. *** Oh yeah, this. I'll see if I can make any progress on it now. 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. 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. (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. (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/ 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. (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 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! I am doing the review. 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 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. (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? (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. 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. 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. (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. 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! Looks good to me. Thanks Gerard! ================ *** APPROVED *** ================ New Package SCM Request ======================= Package Name: eclipse-m2e-core Short Description: Maven integration for Eclipse Owners: galileo Branches: f18 InitialCC: Git done (by process-git-requests). 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 eclipse-m2e-core-1.2.0-3.fc18 has been pushed to the Fedora 18 testing repository. 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 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 :) 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. |