Bug 648945

Summary: Review Request: maven - Java project management and project comprehension tool
Product: [Fedora] Fedora Reporter: Stanislav Ochotnicky <sochotni>
Component: Package ReviewAssignee: Deepak Bhole <dbhole>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: akurtako, dbhole, fedora-package-review, lemenkov, notting, sflaniga
Target Milestone: ---Flags: dbhole: fedora-review+
j: 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: 2010-12-07 08:48:30 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:

Description Stanislav Ochotnicky 2010-11-02 15:15:54 UTC
Spec URL: http://sochotni.fedorapeople.org/packages/maven.spec
SRPM URL: http://sochotni.fedorapeople.org/packages/maven-3.0-1.fc14.src.rpm


Description:
Maven is a software project management and comprehension tool. Based on the
concept of a project object model (POM), Maven can manage a project's build,
reporting and documentation from a central piece of information.

Notes for reviewer:
This package is installable parallel to maven2 and should not cause any problems. Later on this package will obsolete & provide maven2. Custom resolving such as mvn-jpp is being prepared and will be included in next few revisions.

Comment 1 Deepak Bhole 2010-11-09 22:18:07 UTC
It does not build for me with latest rawhide. Error is due to missing commons-parent:

...
...
[INFO] Unable to find resource 'org.apache.commons:commons-parent:pom:12' in repository __jpp_repo__ (file:///usr/share/maven2/repository)
...
...
[INFO] Trace
org.apache.maven.lifecycle.LifecycleExecutionException: Unable to get dependency information: Unable to read the metadata file for artifact 'commons-lang:commons-lang:jar': Cannot find parent: org.apache.commons:commons-parent for project: commons-lang:commons-lang:jar:2.5 for project commons-lang:commons-lang:jar:2.5
...
...

Comment 2 Stanislav Ochotnicky 2010-11-16 14:29:38 UTC
There were changes in rawhide in between my submitting maven for review and your compile attempt.

Updated spec/srpm:
http://sochotni.fedorapeople.org/packages/maven-3.0-2.fc14.src.rpm
http://sochotni.fedorapeople.org/packages/maven.spec

Koji build:
https://koji.fedoraproject.org/koji/taskinfo?taskID=2604185

(btw you should set bug to "assigned" when doing the review)

Comment 3 Deepak Bhole 2010-11-25 01:30:46 UTC
Built successfully locally on rawhide. Rpmlint shows the following issues:

maven.noarch: W: dangling-symlink /usr/share/maven/lib/plexus_plexus-sec-dispatcher.jar /usr/share/java/plexus/plexus-sec-dispatcher.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/aether_spi.jar /usr/share/java/aether/spi.jar
maven.noarch: W: dangling-symlink /usr/share/maven/boot/plexus-classworlds.jar /usr/share/java/plexus/classworlds.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/maven-wagon_file.jar /usr/share/java/maven-wagon/file.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/plexus_interpolation.jar /usr/share/java/plexus/interpolation.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/plexus_containers-component-annotations.jar /usr/share/java/plexus/containers-component-annotations.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/xerces-j2.jar /usr/share/java/xerces-j2.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/guava.jar /usr/share/java/guava.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/maven-wagon_provider-api.jar /usr/share/java/maven-wagon/provider-api.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/aether_connector-wagon.jar /usr/share/java/aether/connector-wagon.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/sisu_sisu-inject-bean.jar /usr/share/java/sisu/sisu-inject-bean.jar
maven.noarch: W: dangling-symlink /usr/share/maven/repository/JPP /usr/share/java
maven.noarch: W: dangling-symlink /usr/share/maven/lib/sisu_sisu-inject-plexus.jar /usr/share/java/sisu/sisu-inject-plexus.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/maven-wagon_http-lightweight.jar /usr/share/java/maven-wagon/http-lightweight.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/hamcrest_core.jar /usr/share/java/hamcrest/core.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/plexus_containers-container-default.jar /usr/share/java/plexus/containers-container-default.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/jdom.jar /usr/share/java/jdom.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/nekohtml.jar /usr/share/java/nekohtml.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/plexus_plexus-cipher.jar /usr/share/java/plexus/plexus-cipher.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/aether_impl.jar /usr/share/java/aether/impl.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/maven-wagon_http-shared.jar /usr/share/java/maven-wagon/http-shared.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/aether_api.jar /usr/share/java/aether/api.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/xbean_xbean-reflect.jar /usr/share/java/xbean/xbean-reflect.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/plexus_utils.jar /usr/share/java/plexus/utils.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/commons-cli.jar /usr/share/java/commons-cli.jar
maven.noarch: W: dangling-symlink /usr/share/maven/lib/aether_util.jar /usr/share/java/aether/util.jar

Above are OK. I confirmed that the links are valid after installation.

----------------------------------------------------------------------

maven.noarch: W: non-conffile-in-etc /etc/maven/fragments/maven

It is safe to mark the above as %config(noreplace). The user should never modify the above file anyway, and even if they do, it should be overridden when the rpm is updated/re-installed.

----------------------------------------------------------------------

maven.noarch: W: no-manual-page-for-binary mvn3

There is no mvn manual page AFAIK so this is ok too.


How does this hook into the new resolver, and where can I find the code for it? Also, what are the plans to make it buildable with mvn3 itself?

Comment 4 Stanislav Ochotnicky 2010-11-26 11:54:33 UTC
(In reply to comment #3)
> ----------------------------------------------------------------------
> 
> maven.noarch: W: non-conffile-in-etc /etc/maven/fragments/maven
> 
> It is safe to mark the above as %config(noreplace). The user should never
> modify the above file anyway, and even if they do, it should be overridden when
> the rpm is updated/re-installed.

%config(noreplace) does exactly the opposite as I understand it. If user modifies the file it won't be replaced on update.

> How does this hook into the new resolver, and where can I find the code for it?

I started working on new resolver in custom repository [1]. It will most probably be rolled into main maven package. Original idea was to have a separate package (something like maven-fedora), but maven package itself would still have to carry patches to actually use it[2]. So that's why it will be shipped similarly to how jpp resolver was packaged for maven2. The code is more-less copied from maven2 resolver, with changes needed because of maven architecture changes.

> Also, what are the plans to make it buildable with mvn3 itself?

mvn3 is actually buildable with mvn2 already. It won't be able to build itself unless all bugs in listed in [3] are closed (incorrect pom file names). mvn2 seems to have used pom file from jar file itself if it was available. mvn3 doesn't so pom files have to have correct filenames. 

Plan is to have maven (maven 3) and maven2 packages installable in parallel until time when we feel m3 is ready to Obsolete m2 (definitely not before F-16).

[1] git://fedorapeople.org/~sochotni/maven-javadir-resolver.git
[2] http://sochotni.fedorapeople.org/0003-Use-custom-resolver.patch
[3] https://bugzilla.redhat.com/buglist.cgi?quicksearch=installs+pom+with+incorrect+filename

Comment 5 Deepak Bhole 2010-11-26 14:23:25 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > ----------------------------------------------------------------------
> > 
> > maven.noarch: W: non-conffile-in-etc /etc/maven/fragments/maven
> > 
> > It is safe to mark the above as %config(noreplace). The user should never
> > modify the above file anyway, and even if they do, it should be overridden when
> > the rpm is updated/re-installed.
> 
> %config(noreplace) does exactly the opposite as I understand it. If user
> modifies the file it won't be replaced on update.
> 

Doh! Sorry :( you are right. I mean it should be just %config

> > How does this hook into the new resolver, and where can I find the code for it?
> 
> I started working on new resolver in custom repository [1]. It will most
> probably be rolled into main maven package. Original idea was to have a
> separate package (something like maven-fedora), but maven package itself would
> still have to carry patches to actually use it[2]. So that's why it will be
> shipped similarly to how jpp resolver was packaged for maven2. The code is
> more-less copied from maven2 resolver, with changes needed because of maven
> architecture changes.
> 
> > Also, what are the plans to make it buildable with mvn3 itself?
> 
> mvn3 is actually buildable with mvn2 already. It won't be able to build itself
> unless all bugs in listed in [3] are closed (incorrect pom file names). mvn2
> seems to have used pom file from jar file itself if it was available. mvn3
> doesn't so pom files have to have correct filenames. 
> 
> Plan is to have maven (maven 3) and maven2 packages installable in parallel
> until time when we feel m3 is ready to Obsolete m2 (definitely not before
> F-16).
> 
> [1] git://fedorapeople.org/~sochotni/maven-javadir-resolver.git
> [2] http://sochotni.fedorapeople.org/0003-Use-custom-resolver.patch
> [3]
> https://bugzilla.redhat.com/buglist.cgi?quicksearch=installs+pom+with+incorrect+filename

Alright so the package is largely fine. Since it cannot be used to build itself, I assume it can't build anything else either? Is there a way to test it in that case, just to make sure all is ok?

Comment 6 Stanislav Ochotnicky 2010-11-26 14:50:10 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > ----------------------------------------------------------------------
> > > 
> > > maven.noarch: W: non-conffile-in-etc /etc/maven/fragments/maven
> > > 
> > > It is safe to mark the above as %config(noreplace). The user should never
> > > modify the above file anyway, and even if they do, it should be overridden when
> > > the rpm is updated/re-installed.
> > 
> > %config(noreplace) does exactly the opposite as I understand it. If user
> > modifies the file it won't be replaced on update.
> > 
> 
> Doh! Sorry :( you are right. I mean it should be just %config

If you don't mind I'll defer the fix until next revision with more changes

> Alright so the package is largely fine. Since it cannot be used to build
> itself, I assume it can't build anything else either? Is there a way to test it
> in that case, just to make sure all is ok?

Oh. I wasn't clear I guess. mvn3 script is actually vanilla maven and that should work on any package. What can (and does) have problems is jpp mode. Mostly caused by two things for now:
 * wrong pom filenames in several dependencies causing maven to bail
 * some as of yet unindentified problem with maven-plugin-tools (affecting build of maven-surefire and animal-sniffer for example)

But I was nonetheless able to build several packages with mvn-jpp:
 * atinject
 * geronimo-jpa
 * several apache-commons-packages
 * probably a few more libs

So if you just want to test "vanilla" maven install you can install it on F-14 in parallel with maven2. I've been testing it like that. The script is called mvn3.

Comment 7 Deepak Bhole 2010-12-01 15:53:55 UTC
Okay, everything seems to be in order. I tried to build atinject and it went smoothly.

So here is the full review for the latest posted files (comment #2):

PASSED:
Package name is OK
License is OK
Release is consistent with guidelines
Source verified
Patches are OK
No typos found in summary/description
BuildRoot is OK
{?dist} used correctly
License is marked %doc
FHS guidelines met
Package/Vendor/Distribution tags not used
No use of 'Copyright'
Summary does not end in period
Post and Postun reqs used correctly
Package compiled on x86_64
Summary and description are < 80 chars
%clean present
cp -a used, which preserves timestamps
Package owns all files and dirs
No duplicates in %files


FAILED:
No prebuilt binaries- FAILED
  - There is a apache-maven-3.0/maven-ant-tasks-2.1.1.jar that is also in the 
    buildroot.

RPMLint on built packages - FAILED
  - As discussed above, /etc/maven/fragments/maven should be %config

Comment 8 Stanislav Ochotnicky 2010-12-01 17:33:04 UTC
Updated spec/srpm:
http://sochotni.fedorapeople.org/packages/maven-3.0-3.fc14.src.rpm
http://sochotni.fedorapeople.org/packages/maven.spec

I didn't go to 3.0.1 because (without cursing too much) upstream added jboss.netty, tomcat6, jetty and 2 packages we don't yet have in fedora as dependencies. Gotta love them for it...

I'll see what can be done about those dependencies later after contacting upstream.

Comment 9 Deepak Bhole 2010-12-01 18:11:27 UTC
New spec file removed the prebuilt binary before build, and marks /etc/maven/fragments/maven as %config.

Everythink is OK now. Granting +

Comment 10 Stanislav Ochotnicky 2010-12-02 09:31:08 UTC
New Package SCM Request
=======================
Package Name: maven
Short Description: Java project management and project comprehension tool
Owners: sochotni akurtakov
Branches: f14
InitialCC: java-sig

F-14 branch just to workaround fedpkg bug.

Comment 11 Jason Tibbitts 2010-12-02 19:36:30 UTC
Git done (by process-git-requests).

Comment 12 Stanislav Ochotnicky 2010-12-07 08:48:12 UTC
Built in koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2648811

Thanks for review, closing.