Bug 1230874 - Review Request: felix-scr-maven-plugin - Maven plugin for generating OSGi Declarative Services annotations
Summary: Review Request: felix-scr-maven-plugin - Maven plugin for generating OSGi Dec...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Mikolaj Izdebski
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 1193207 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-06-11 16:36 UTC by Jie Kang
Modified: 2016-05-06 01:55 UTC (History)
4 users (show)

Fixed In Version: felix-scr-maven-plugin-1.21.0-2.fc22
Clone Of:
Environment:
Last Closed: 2015-07-29 01:50:45 UTC
Type: Bug
Embargoed:
mizdebsk: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Jie Kang 2015-06-11 16:36:13 UTC
Fedora Account System Username: jkang

Spec URL: https://jkang.fedorapeople.org/maven-scr-plugin.spec
SRPM URL: https://jkang.fedorapeople.org/maven-scr-plugin-1.21.0-1.fc21.src.rpm
Description: Maven plugin for generating OSGi Declarative Services annotations

Comment 1 gil cattaneo 2015-06-11 16:48:42 UTC
see https://fedoraproject.org/w/index.php?title=Licensing:Main&rd=Licensing#Good_Licenses
you should change License field in ASL 2.0

Comment 2 gil cattaneo 2015-06-11 16:53:48 UTC
*** Bug 1193207 has been marked as a duplicate of this bug. ***

Comment 3 Severin Gehwolf 2015-06-11 17:10:12 UTC
I'll do this review.

Comment 4 Severin Gehwolf 2015-06-11 17:12:48 UTC
Not all issues apply any longer. Specifically the comments related to Source0 and License.

Here is what I found. Please let me know if you have questions.

Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated



===== MUST items =====

Generic:
[!]: URL should be fixed to:
     http://felix.apache.org/documentation/subprojects/apache-felix-maven-scr-plugin.html
[!]: Tarball in SRPM suggests gzipped, but is actually xz compressed. Please
     fix the file extension.
[!]: Description in SRPM says "Apache Felix' Service Component Runtime" which
     differs from spec.
[!]: BuildRequires:  mvn(org.apache.felix:org.apache.felix.scr.generator) = 1.12.0
     Is this exact version requirement needed? It whould be better to drop the
     version requirement or use >= if a minimal version is required.
[!]: Why is this %mvn_file there? Is it needed?
[x]: Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[-]: 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 %license.
[!]: License field in the package spec file matches the actual license.
     Use correct license line. Pick "ASL 2.0". See
     https://fedoraproject.org/wiki/Licensing:Main?rd=Licensing#Software_License_List
[-]: License file installed when any subpackage combination is installed.
[x]: Package contains no bundled libraries without FPC exception.
[!]: Changelog in prescribed format.
     Fix changelog so that it includes V-R (Version-Release). Example:
     %changelog
     * Mon May 25 2015 Jie Kang <jkang> - 1.21.0-1
     - Initial package
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
[x]: Package installs properly.
[!]: Rpmlint is run on all rpms the build produces.
[x]: Package requires other packages for directories it uses.
[x]: Package must own all directories that it creates.
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[-]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[!]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
     The comment for producing the sources should use a specific
     revision so as to be able to reproduce creating the source tarball
     with a matching md5sum. Please use svn export over checkout. Tarball
     contains .svn directories.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 0 bytes in 0 files.
[x]: Packages must not store files under /srv, /opt or /usr/local

Java:
[x]: Bundled jar/class files should be removed before build
[!]: Packages have proper BuildRequires/Requires on javapackages-tools
     Note: Maven packages do not need to (Build)Require javapackages-tools. It
     is pulled in by maven-local. Please only keep the maven-local BR and
     drop the javapackages-tools BR.
[x]: Javadoc documentation files are generated and included in -javadoc
     subpackage
[x]: Javadoc subpackages should not have Requires: jpackage-utils
     It does have it, but is auto-generated. OK.
[x]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink)

Maven:
[-]: If package contains pom.xml files install it (including metadata) even
     when building with ant
[x]: POM files have correct Maven mapping
[x]: Maven packages should use new style packaging
[x]: Old add_to_maven_depmap macro is not being used
[x]: Packages DO NOT have Requires(post) and Requires(postun) on jpackage-
     utils for %update_maven_depmap macro
[x]: Package DOES NOT use %update_maven_depmap in %post/%postun
[x]: Packages use .mfiles file list instead of %{_datadir}/maven2/poms

===== SHOULD items =====

Generic:
[?]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane.
[-]: Fully versioned dependency in subpackages if applicable.
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: Reviewer should test that the package builds in mock.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

Java:
[x]: Package uses upstream build method (maven)
[x]: Packages are noarch unless they use JNI

===== EXTRA items =====

[!]: Rpmlint is run on all installed packages.
     Please fix rpmlint warnings. Source0 should be OK.


Rpmlint
-------
Checking: maven-scr-plugin-1.21.0-1.fc23.noarch.rpm
          maven-scr-plugin-javadoc-1.21.0-1.fc23.noarch.rpm
          maven-scr-plugin-1.21.0-1.fc23.src.rpm
maven-scr-plugin.noarch: W: no-version-in-last-changelog
maven-scr-plugin.noarch: W: invalid-license Apache License V2.0
maven-scr-plugin.noarch: W: no-documentation
maven-scr-plugin-javadoc.noarch: W: no-version-in-last-changelog
maven-scr-plugin-javadoc.noarch: W: invalid-license Apache License V2.0
maven-scr-plugin.src: W: no-version-in-last-changelog
maven-scr-plugin.src: W: invalid-license Apache License V2.0
maven-scr-plugin.src:4: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 4)
maven-scr-plugin.src: W: invalid-url Source0: maven-scr-plugin-1.21.0.tar.gz
3 packages and 0 specfiles checked; 0 errors, 9 warnings.




Rpmlint (installed packages)
----------------------------
maven-scr-plugin.noarch: W: no-version-in-last-changelog
maven-scr-plugin.noarch: W: invalid-license Apache License V2.0
maven-scr-plugin.noarch: W: no-documentation
maven-scr-plugin-javadoc.noarch: W: no-version-in-last-changelog
maven-scr-plugin-javadoc.noarch: W: invalid-license Apache License V2.0
2 packages and 0 specfiles checked; 0 errors, 5 warnings.


Requires
--------
maven-scr-plugin (rpmlib, GLIBC filtered):
    felix-osgi-compendium
    felix-osgi-core
    java-headless
    jpackage-utils
    mvn(org.apache.felix:org.apache.felix.scr.generator)
    mvn(org.apache.maven:maven-archiver)
    mvn(org.apache.maven:maven-plugin-api)
    mvn(org.sonatype.plexus:plexus-build-api)
    objectweb-asm

maven-scr-plugin-javadoc (rpmlib, GLIBC filtered):
    jpackage-utils


Provides
--------
maven-scr-plugin:
    maven-scr-plugin
    mvn(org.apache.felix:maven-scr-plugin)
    mvn(org.apache.felix:maven-scr-plugin:pom:)

maven-scr-plugin-javadoc:
    maven-scr-plugin-javadoc

Comment 5 Jie Kang 2015-06-11 18:41:31 UTC
(In reply to Severin Gehwolf from comment #4)
> Not all issues apply any longer. Specifically the comments related to
> Source0 and License.
> 
> Here is what I found. Please let me know if you have questions.
> 
> Package Review
> ==============
> 
> Legend:
> [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
> 
> 
> 
> ===== MUST items =====
> 
> Generic:
> [!]: URL should be fixed to:
>     
> http://felix.apache.org/documentation/subprojects/apache-felix-maven-scr-
> plugin.html

Fixed.

> [!]: Tarball in SRPM suggests gzipped, but is actually xz compressed. Please
>      fix the file extension.

The tarball has been brought in from felix' source-release repo's instead.

> [!]: Description in SRPM says "Apache Felix' Service Component Runtime" which
>      differs from spec.

Fixed.

> [!]: BuildRequires:  mvn(org.apache.felix:org.apache.felix.scr.generator) =
> 1.12.0
>      Is this exact version requirement needed? It whould be better to drop
> the
>      version requirement or use >= if a minimal version is required.

The plugin fails to compile with versions below this. Changed to use >=

> [!]: Why is this %mvn_file there? Is it needed?

Removed

> [x]: Package is licensed with an open-source compatible license and meets
>      other legal requirements as defined in the legal section of Packaging
>      Guidelines.
> [-]: 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 %license.
> [!]: License field in the package spec file matches the actual license.
>      Use correct license line. Pick "ASL 2.0". See

Fixed.

>  
> https://fedoraproject.org/wiki/Licensing:
> Main?rd=Licensing#Software_License_List
> [-]: License file installed when any subpackage combination is installed.
> [x]: Package contains no bundled libraries without FPC exception.
> [!]: Changelog in prescribed format.
>      Fix changelog so that it includes V-R (Version-Release). Example:
>      %changelog
>      * Mon May 25 2015 Jie Kang <jkang> - 1.21.0-1
>      - Initial package

Fixed.

> [x]: Sources contain only permissible code or content.
> [-]: Package contains desktop file if it is a GUI application.
> [-]: Development files must be in a -devel package
> [x]: Package uses nothing in %doc for runtime.
> [x]: Package consistently uses macros (instead of hard-coded directory
>      names).
> [x]: Package is named according to the Package Naming Guidelines.
> [x]: Package does not generate any conflict.
> [x]: Package obeys FHS, except libexecdir and /usr/target.
> [-]: If the package is a rename of another package, proper Obsoletes and
>      Provides are present.
> [x]: Requires correct, justified where necessary.
> [x]: Spec file is legible and written in American English.
> [-]: Package contains systemd file(s) if in need.
> [x]: Package is not known to require an ExcludeArch tag.
> [x]: Package complies to the Packaging Guidelines
> [x]: Package successfully compiles and builds into binary rpms on at least
>      one supported primary architecture.
> [x]: Package installs properly.
> [!]: Rpmlint is run on all rpms the build produces.

I have run rpmlint on the spec, rpm and srpm files. Is there is anything else I need to do here?;

> [x]: Package requires other packages for directories it uses.
> [x]: Package must own all directories that it creates.
> [x]: Package does not own files or directories owned by other packages.
> [x]: All build dependencies are listed in BuildRequires, except for any
>      that are listed in the exceptions section of Packaging Guidelines.
> [x]: Macros in Summary, %description expandable at SRPM build time.
> [x]: Dist tag is present.
> [x]: Package does not contain duplicates in %files.
> [x]: Permissions on files are set properly.
> [-]: Package use %makeinstall only when make install DESTDIR=... doesn't
>      work.
> [x]: Package is named using only allowed ASCII characters.
> [x]: Package does not use a name that already exists.
> [x]: Package is not relocatable.
> [!]: Sources used to build the package match the upstream source, as
>      provided in the spec URL.
>      The comment for producing the sources should use a specific
>      revision so as to be able to reproduce creating the source tarball
>      with a matching md5sum. Please use svn export over checkout. Tarball
>      contains .svn directories.

Fixed to use felix' source-release tarball instead.

> [x]: Spec file name must match the spec package %{name}, in the format
>      %{name}.spec.
> [x]: File names are valid UTF-8.
> [-]: Large documentation must go in a -doc subpackage. Large could be size
>      (~1MB) or number of files.
>      Note: Documentation size is 0 bytes in 0 files.
> [x]: Packages must not store files under /srv, /opt or /usr/local
> 
> Java:
> [x]: Bundled jar/class files should be removed before build
> [!]: Packages have proper BuildRequires/Requires on javapackages-tools
>      Note: Maven packages do not need to (Build)Require javapackages-tools.

Fixed.

> It
>      is pulled in by maven-local. Please only keep the maven-local BR and
>      drop the javapackages-tools BR.
> [x]: Javadoc documentation files are generated and included in -javadoc
>      subpackage
> [x]: Javadoc subpackages should not have Requires: jpackage-utils
>      It does have it, but is auto-generated. OK.
> [x]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink)
> 
> Maven:
> [-]: If package contains pom.xml files install it (including metadata) even
>      when building with ant
> [x]: POM files have correct Maven mapping
> [x]: Maven packages should use new style packaging
> [x]: Old add_to_maven_depmap macro is not being used
> [x]: Packages DO NOT have Requires(post) and Requires(postun) on jpackage-
>      utils for %update_maven_depmap macro
> [x]: Package DOES NOT use %update_maven_depmap in %post/%postun
> [x]: Packages use .mfiles file list instead of %{_datadir}/maven2/poms
> 
> ===== SHOULD items =====
> 
> Generic:
> [?]: If the source package does not include license text(s) as a separate
>      file from upstream, the packager SHOULD query upstream to include it.
> [x]: Final provides and requires are sane.
> [-]: Fully versioned dependency in subpackages if applicable.
> [?]: Package functions as described.
> [x]: Latest version is packaged.
> [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
> [x]: Reviewer should test that the package builds in mock.
> [x]: Buildroot is not present
> [x]: Package has no %clean section with rm -rf %{buildroot} (or
>      $RPM_BUILD_ROOT)
> [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
> [x]: SourceX is a working URL.
> [x]: Spec use %global instead of %define unless justified.
> 
> Java:
> [x]: Package uses upstream build method (maven)
> [x]: Packages are noarch unless they use JNI
> 
> ===== EXTRA items =====
> 
> [!]: Rpmlint is run on all installed packages.
>      Please fix rpmlint warnings. Source0 should be OK.
> 
> 
> Rpmlint
> -------
> Checking: maven-scr-plugin-1.21.0-1.fc23.noarch.rpm
>           maven-scr-plugin-javadoc-1.21.0-1.fc23.noarch.rpm
>           maven-scr-plugin-1.21.0-1.fc23.src.rpm
> maven-scr-plugin.noarch: W: no-version-in-last-changelog
> maven-scr-plugin.noarch: W: invalid-license Apache License V2.0
> maven-scr-plugin.noarch: W: no-documentation
> maven-scr-plugin-javadoc.noarch: W: no-version-in-last-changelog
> maven-scr-plugin-javadoc.noarch: W: invalid-license Apache License V2.0
> maven-scr-plugin.src: W: no-version-in-last-changelog
> maven-scr-plugin.src: W: invalid-license Apache License V2.0
> maven-scr-plugin.src:4: W: mixed-use-of-spaces-and-tabs (spaces: line 1,
> tab: line 4)
> maven-scr-plugin.src: W: invalid-url Source0: maven-scr-plugin-1.21.0.tar.gz
> 3 packages and 0 specfiles checked; 0 errors, 9 warnings.
> 
> 
> 
> 
> Rpmlint (installed packages)
> ----------------------------
> maven-scr-plugin.noarch: W: no-version-in-last-changelog
> maven-scr-plugin.noarch: W: invalid-license Apache License V2.0
> maven-scr-plugin.noarch: W: no-documentation
> maven-scr-plugin-javadoc.noarch: W: no-version-in-last-changelog
> maven-scr-plugin-javadoc.noarch: W: invalid-license Apache License V2.0
> 2 packages and 0 specfiles checked; 0 errors, 5 warnings.
> 
> 
> Requires
> --------
> maven-scr-plugin (rpmlib, GLIBC filtered):
>     felix-osgi-compendium
>     felix-osgi-core
>     java-headless
>     jpackage-utils
>     mvn(org.apache.felix:org.apache.felix.scr.generator)
>     mvn(org.apache.maven:maven-archiver)
>     mvn(org.apache.maven:maven-plugin-api)
>     mvn(org.sonatype.plexus:plexus-build-api)
>     objectweb-asm
> 
> maven-scr-plugin-javadoc (rpmlib, GLIBC filtered):
>     jpackage-utils
> 
> 
> Provides
> --------
> maven-scr-plugin:
>     maven-scr-plugin
>     mvn(org.apache.felix:maven-scr-plugin)
>     mvn(org.apache.felix:maven-scr-plugin:pom:)
> 
> maven-scr-plugin-javadoc:
>     maven-scr-plugin-javadoc

Comment 6 Severin Gehwolf 2015-06-12 08:58:09 UTC
(In reply to Jie Kang from comment #5)
> > [!]: Rpmlint is run on all rpms the build produces.
> 
> I have run rpmlint on the spec, rpm and srpm files. Is there is anything
> else I need to do here?;

Sorry, my bad. I've marked this as fail since there were rpmlint warnings. It's fine now anyway.

Comment 7 Severin Gehwolf 2015-06-12 09:01:47 UTC
This updated version is almost good to go. One significant fix has to be made. Sorry, that I missed this earlier. Please rename the spec/package to felix-src-maven-plugin rather than maven-src-plugin. That's because of this:

[ERROR]
Artifact Ids of the format maven-___-plugin are reserved for
plugins in the Group Id org.apache.maven.plugins
Please change your artifactId to the format ___-maven-plugin
In the future this error will break the build.

Thanks!

Comment 8 Jie Kang 2015-06-12 12:50:38 UTC
(In reply to Severin Gehwolf from comment #7)
> This updated version is almost good to go. One significant fix has to be
> made. Sorry, that I missed this earlier. Please rename the spec/package to
> felix-src-maven-plugin rather than maven-src-plugin. That's because of this:
> 

Spec URL: https://jkang.fedorapeople.org/felix-scr-maven-plugin.spec
SRPM URL: https://jkang.fedorapeople.org/felix-scr-maven-plugin-1.21.0-1.fc21.src.rpm

Thanks~

> [ERROR]
> Artifact Ids of the format maven-___-plugin are reserved for
> plugins in the Group Id org.apache.maven.plugins
> Please change your artifactId to the format ___-maven-plugin
> In the future this error will break the build.
> 
> Thanks!

Comment 9 Severin Gehwolf 2015-06-12 13:38:03 UTC
%global source maven-scr-plugin

Please call this "artifactId" or some such. "source" seems too generic and is very confusing when working with RPMs since a source is generally some tarball or a config file or...


# For building from source tarball
Source0:        http://www.apache.org/dist/felix/%{source}-%{version}-source-release.tar.gz

Please drop this "For building from source tarball" comment. It doesn't really make sense. Everything is built from source.

Looks good otherwise.

Comment 10 Severin Gehwolf 2015-06-12 13:46:57 UTC
One more thing. When you change something in the spec you should bump the release and mention in changelog what has changed. Note that there cannot be two builds in koji with the same N-V-R (Name-Version-Release).

Comment 11 Jie Kang 2015-06-12 14:52:55 UTC
(In reply to Severin Gehwolf from comment #9)
> %global source maven-scr-plugin
> 
> Please call this "artifactId" or some such. "source" seems too generic and
> is very confusing when working with RPMs since a source is generally some
> tarball or a config file or...

Okay, changed to 'artifactId'

> 
> 
> # For building from source tarball
> Source0:       
> http://www.apache.org/dist/felix/%{source}-%{version}-source-release.tar.gz
> 
> Please drop this "For building from source tarball" comment. It doesn't
> really make sense. Everything is built from source.

Done.

> 
> Looks good otherwise.

I have also updated the changelog as per your comment

Comment 12 Severin Gehwolf 2015-06-12 15:01:00 UTC
Lifting the FE-NEEDSPONSOR requirement.

Comment 13 Severin Gehwolf 2015-06-12 15:05:27 UTC
(In reply to Jie Kang from comment #11)
> > 
> > Looks good otherwise.
> 
> I have also updated the changelog as per your comment

You've updated the changelog entry and bumped release there, but not actually in the Release field ;-)

Comment 14 Jie Kang 2015-06-12 15:11:15 UTC
(In reply to Severin Gehwolf from comment #13)
> (In reply to Jie Kang from comment #11)
> > > 
> > > Looks good otherwise.
> > 
> > I have also updated the changelog as per your comment
> 
> You've updated the changelog entry and bumped release there, but not
> actually in the Release field ;-)

Ah my bad. I've updated and reuploaded it now. Thanks!

Comment 15 Severin Gehwolf 2015-06-12 16:20:24 UTC
Adding back the FE-NEEDSPONSOR requirement. Informal review done and I think it's good to go. But since this is Jie's first package a sponsor needs to do the review if I understand this whole process correctly.

Comment 16 gil cattaneo 2015-06-13 11:30:04 UTC
(In reply to Jie Kang from comment #8)
> (In reply to Severin Gehwolf from comment #7)
> 
> > [ERROR]
> > Artifact Ids of the format maven-___-plugin are reserved for
> > plugins in the Group Id org.apache.maven.plugins
> > Please change your artifactId to the format ___-maven-plugin
> > In the future this error will break the build.
> > 
> > Thanks!

Please, not rename aid or gIg of the plugin, if this does not cause the failure of the build. This changes create difficulties in its use only and not meant to exist.

Comment 17 Severin Gehwolf 2015-06-15 08:40:21 UTC
(In reply to gil cattaneo from comment #16)
> (In reply to Jie Kang from comment #8)
> > (In reply to Severin Gehwolf from comment #7)
> > 
> > > [ERROR]
> > > Artifact Ids of the format maven-___-plugin are reserved for
> > > plugins in the Group Id org.apache.maven.plugins
> > > Please change your artifactId to the format ___-maven-plugin
> > > In the future this error will break the build.
> > > 
> > > Thanks!
> 
> Please, not rename aid or gIg of the plugin, if this does not cause the
> failure of the build. This changes create difficulties in its use only and
> not meant to exist.

The aId:gId pairs have not been changed, but the proposed name of the package in fedora (maven-scr-plugin vs. felix-src-maven-plugin). Changing aId:gId would be something to do upstream first.

Comment 18 gil cattaneo 2015-06-15 13:25:22 UTC
(In reply to Severin Gehwolf from comment #17)
> > Please, not rename aid or gIg of the plugin, if this does not cause the
> > failure of the build. This changes create difficulties in its use only and
> > not meant to exist.
> 
> The aId:gId pairs have not been changed, but the proposed name of the
> package in fedora (maven-scr-plugin vs. felix-src-maven-plugin). Changing
> aId:gId would be something to do upstream first.

Then it makes few sense to rename the package.
Needless to wrap your head if it is not broken. :)
https://fedoraproject.org/wiki/Packaging:NamingGuidelines#General_Naming

Comment 19 Severin Gehwolf 2015-06-15 14:45:56 UTC
(In reply to gil cattaneo from comment #18)
> (In reply to Severin Gehwolf from comment #17)
> > > Please, not rename aid or gIg of the plugin, if this does not cause the
> > > failure of the build. This changes create difficulties in its use only and
> > > not meant to exist.
> > 
> > The aId:gId pairs have not been changed, but the proposed name of the
> > package in fedora (maven-scr-plugin vs. felix-src-maven-plugin). Changing
> > aId:gId would be something to do upstream first.
> 
> Then it makes few sense to rename the package.
> Needless to wrap your head if it is not broken. :)
> https://fedoraproject.org/wiki/Packaging:NamingGuidelines#General_Naming

What's your point exactly? It's not in fedora yet, so IMHO it makes more sense to call it <foo>-maven-plugin rather than maven-<foo>-plugin since it's not a plugin with group ID org.apache.maven.plugins. Both would satisfy naming guidelines. There are already example packages in fedora:
jetty-version-maven-plugin
port-allocator-maven-plugin
exec-maven-plugin
cobertura-maven-plugin

felix-scr-maven-plugin makes sense to me since it doesn't use gId org.apache.maven.plugins but has org.apache.felix and it's actually an upstream bug that they don't follow maven plug-in's naming conventions.

Comment 20 Mikolaj Izdebski 2015-07-01 20:37:41 UTC
I agree with either package name - it is up to maintainer to choose.
Everything looks good, except for one problem - release field does not match changelog (release is 1%{?dist}, while changelog says 1.21.0-2)

Comment 21 Jie Kang 2015-07-02 14:50:36 UTC
(In reply to Mikolaj Izdebski from comment #20)
> I agree with either package name - it is up to maintainer to choose.

Okay. I think I will go with "felix-scr-maven-plugin".

> Everything looks good, except for one problem - release field does not match
> changelog (release is 1%{?dist}, while changelog says 1.21.0-2)

I have updated the spec file with Release as 2%{?dist}. The spec [1] and srpm [2] on my fedorapeople space should have the corrections now.

Are there any other concerns to address?


Thanks!

[1] https://jkang.fedorapeople.org/felix-scr-maven-plugin.spec
[2] https://jkang.fedorapeople.org/felix-scr-maven-plugin-1.21.0-2.fc21.src.rpm

Comment 22 Mikolaj Izdebski 2015-07-02 18:53:38 UTC
No more problems, package is approved.


Package Review
==============

Key:
- = N/A
x = Check
! = Problem

[x] rpmlint must be run on the source rpm and all binary rpms the
    build produces.  The output should be posted in the review.

[x] The package must be named according to the Package Naming
    Guidelines.

[x] The spec file name must match the base package %{name}, in the
    format %{name}.spec unless your package has an exemption.

[x] The package must meet the Packaging Guidelines.

[x] The package must be licensed with a Fedora approved license and
    meet the Licensing Guidelines.

[x] The License field in the package spec file must match the actual
    license.

[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 must be included in %doc.

[x] The spec file must be written in American English.

[x] The spec file for the package MUST be legible.

[x] The sources used to build the package must match the upstream
    source, as provided in the spec URL.  Reviewers should use
    sha256sum for this task as it is used by the sources file once
    imported into git.  If no upstream URL can be specified for this
    package, please see the Source URL Guidelines for how to deal with
    this.

[x] The package MUST successfully compile and build into binary rpms
    on at least one primary architecture.

[x] If the package does not successfully compile, build or work on an
    architecture, then those architectures should be listed in the
    spec in ExcludeArch.  Each architecture listed in ExcludeArch MUST
    have a bug filed in bugzilla, describing the reason that the
    package does not compile/build/work on that architecture.  The bug
    number MUST be placed in a comment, next to the corresponding
    ExcludeArch line.

[x] All build dependencies must be listed in BuildRequires, except for
    any that are listed in the exceptions section of the Packaging
    Guidelines; inclusion of those as BuildRequires is optional.
    Apply common sense.

[x] The spec file MUST handle locales properly.  This is done by using
    the %find_lang macro.  Using %{_datadir}/locale/* is strictly
    forbidden.

[x] Every binary RPM package (or subpackage) which stores shared
    library files (not just symlinks) in any of the dynamic linker's
    default paths, must call ldconfig in %post and %postun.

[x] Packages must NOT bundle copies of system libraries.

[x] If the package is designed to be relocatable, the packager must
    state this fact in the request for review, along with the
    rationalization for relocation of that specific package.  Without
    this, use of Prefix: /usr is considered a blocker.

[x] A package must own all directories that it creates.  If it does
    not create a directory that it uses, then it should require a
    package which does create that directory.

[x] A Fedora package must not list a file more than once in the spec
    file's %files listings.  (Notable exception: license texts in
    specific situations.)

[x] Permissions on files must be set properly.  Executables should be
    set with executable permissions, for example.

[x] Each package must consistently use macros.

[x] The package must contain code, or permissible content.

[x] Large documentation files must go in a -doc subpackage.  (The
    definition of large is left up to the packager's best judgement,
    but is not restricted to size.  Large can refer to either size or
    quantity).

[x] If a package includes something as %doc, it must not affect the
    runtime of the application.  To summarize: If it is in %doc, the
    program must run properly if it is not present.

[x] Static libraries must be in a -static package.

[x] Development files must be in a -devel package.

[x] In the vast majority of cases, devel packages must require the
    base package using a fully versioned dependency: Requires:
    %{name}%{?_isa} = %{version}-%{release}

[x] Packages must NOT contain any .la libtool archives, these must be
    removed in the spec if they are built.

[x] Packages containing GUI applications must include a
    %{name}.desktop file, and that file must be properly installed
    with desktop-file-install in the %install section.  If you feel
    that your packaged GUI application does not need a .desktop file,
    you must put a comment in the spec file with your explanation.

[x] Packages must not own files or directories already owned by other
    packages.  The rule of thumb here is that the first package to be
    installed should own the files or directories that other packages
    may rely upon.  This means, for example, that no package in Fedora
    should ever share ownership with any of the files or directories
    owned by the filesystem or man package.  If you feel that you have
    a good reason to own a file or directory that another package
    owns, then please present that at package review time.

[x] All filenames in rpm packages must be valid UTF-8.


rpmlint output
--------------
3 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 23 Jie Kang 2015-07-07 18:01:26 UTC
New Package SCM Request
=======================
Package Name: felix-scr-maven-plugin
Short Description: Maven plugin for generating OSGi Declarative Services annotations
Upstream URL: http://felix.apache.org/documentation/subprojects/apache-felix-maven-scr-plugin.html
Owners: jkang
Branches: f22 f23
InitialCC:

Comment 24 Gwyn Ciesla 2015-07-08 11:58:37 UTC
Git done (by process-git-requests).

Comment 25 Fedora Update System 2015-07-08 15:35:12 UTC
felix-scr-maven-plugin-1.21.0-2.fc22 has been submitted as an update for Fedora 22.
https://admin.fedoraproject.org/updates/felix-scr-maven-plugin-1.21.0-2.fc22

Comment 26 Fedora Update System 2015-07-13 19:07:35 UTC
felix-scr-maven-plugin-1.21.0-2.fc22 has been pushed to the Fedora 22 testing repository.

Comment 27 Fedora Update System 2015-07-29 01:50:45 UTC
felix-scr-maven-plugin-1.21.0-2.fc22 has been pushed to the Fedora 22 stable repository.


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