Spec URL: https://github.com/riboseinc/netpgp/raw/master/packaging/redhat/m4/rpm.spec SRPM URL: https://github.com/riboseinc/yum/raw/master/SRPMS/netpgp-3.99.17-1.src.rpm Description: Freely licensed PGP implementation Fedora Account System Username: jeffreylauribose This is my first package review request, so I would greatly appreciate any sponsorship. If there's anything needing correction, I'm all ears. Github link: [1] https://github.com/riboseinc/netpgp Follow the README on the Github link for build instructions. One thing of note is that the rpm.spec needs preprocessing with m4, which is all automated in the build script detailed in [1]. Cheers!
Tons of strange things in the spec file. This will need _a lot_ of work to bring the package into shape. I wonder whether and when you've had a look at other Fedora [or Red Hat] packages for the last time? As a quickstart, point the fedora-review tool at this ticket: fedora-review -b 1428693 It will evaluate the "Spec URL:" and "SRPM URL:" lines, download the latest files, do local builds in Mock and then perform many checks on the packages. That tool should be of interest to you, since it is not specific to reviewers, and all packagers should use it.
Thanks Michael for pointing me to the fedora-review tool. Over the last few days I've just made a few changes --- specifically, the Spec URL and SRPM URL are now different. Should I be making a new review request with the updated details and then close this bug, or post the details on this same thread?
Just post a comment with a pair of "Spec URL:" and "SRPM URL:" pointing at your update package as per the process description: https://fedoraproject.org/wiki/Package_Review_Process#Contributor
https://raw.githubusercontent.com/riboseinc/netpgp/master/packaging/redhat/extra/netpgp.spec
Thanks Michael, I'll remove the NotReady from the Whiteboard once it's ready for further review.
Very Very Preliminary review: 1) Package failed to build in mock. It failed as spec is using autoreconf but that binary is not available in build environment. Fix this as BuildRequires: autoconf 2) SPEC file must not contain version number. Change netpgp-3.99.17.spec to netpgp.spec 3) Packager and Vendor tags are not used in Fedora packaging. 4) Source0: should provide direct downloadable URL I suggest check with upstream if they can release any tarball otherwise we need to document the steps on how you created source archive as comments just before Source0: line. 5) Any BuildRequires or Requires for any package should not restrict versions. In this package I see Requires: netpgpverify = %{version} which should be Requires: netpgpverify 6) We don't need these %defines. Remove them. %define _unpackaged_files_terminate_build 0 %define _prefix /usr We already define _prefix to /usr. See /usr/lib/rpm/macros file. 7) For %prep section, Fedora packaging guidelines now recommends use of %autosetup 8) for %build we have those macros already defined in /usr/lib/rpm/macros no need to specify them. I think its better if upstream will provide some release where configure will already be available. Then you will not need to use autoreconf. Any C library providing package generally have these lines in spec %configure --disable-static make %{?_smp_mflags} 9) for %install any general library providing package should have lines make install DESTDIR=$RPM_BUILD_ROOT find $RPM_BUILD_ROOT -name '*.la' -exec rm -f {} ';' 9) We no longer need every %files section to write %defattr(-,root,root) and then for every listed files to start with %attr(0755,root,root) 10) for library package we only need %post -p /sbin/ldconfig %postun -p /sbin/ldconfig and not other %pre, %preun. See more on scriptlet here https://fedoraproject.org/wiki/Packaging:Scriptlets#Shared_libraries 11) We no loner need to write %clean Hope this will at least help you to start moving this package updates. Please remember whenever someone suggests any changes you need to increase release tag by 1 and add appropriate changelog entry e.g. "Review suggestions fixes". This is the way package submitter need to provide updates. Also every time you need to specify both the links that is Spec URL and SRPM URL. If not provided reviewer cannot use fedora-review tool to review your package and thus you will not get any inputs here.
This is very helpful for a newbie like me, thanks Parag! Will be working through this list.
(In reply to Parag AN(पराग) from comment #6) > Very Very Preliminary review: > 5) Any BuildRequires or Requires for any package should not restrict > versions. In this package I see > Requires: netpgpverify = %{version} > which should be > Requires: netpgpverify > ah please forget this point. Its for specific version requirements not for the same version. I was going in SPEC one by one line and later at the end found this package also provides netpgpverify. Hence, you can keep it as "Requires: netpgpverify = %{version}-%{release}" Also, To get sponsor for your package in packager group, please follow these things. Make sure you have followed steps given on https://fedoraproject.org/wiki/Join_the_package_collection_maintainers till "Get Sponsored" section. We have this process, http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group to get sponsored into the packager group. Can you either submit few more packages and/or some full detailed package reviews? This is needed to make sure package submitter understands the rpm packaging well and follows the fedora packaging guidelines. Please go through the following links 1) http://fedoraproject.org/wiki/Package_Review_Process 2) https://fedoraproject.org/wiki/PackagingGuidelines 3) To find the packages already submitted for review, check http://fedoraproject.org/PackageReviewStatus/ 4) http://fedoraproject.org/wiki/Packaging:ReviewGuidelines and http://fedoraproject.org/wiki/Package_Review_Process#Reviewer is useful while doing package reviews. 5) https://fedorahosted.org/FedoraReview/ this is fedora-review tool to help review packages in fedora. You need to use this and do un-official package reviews of packages submitted by other contributors. While doing so mention "This is un-official review of the package." at top of your review comment. Good to review packages listed in http://fedoraproject.org/PackageReviewStatus/NEW.html When you do full package review of some packages, provide that review comment link here so that I can look how you have reviewed those packages. An example command to run fedora-review on any package review bugzilla is fedora-review -b <bugid> -m fedora-rawhide-x86_64 If you got any questions please ask here or on Freenode IRC join #fedora-devel :)
Any updates here?
Thanks Parag for the thorough guidance! :) We're actually going to rename this package (+ a whole host of other changes including new dependencies introduced). But the dependencies haven't been resolved yet, so we don't have any new Spec / SRPM URLs to post. Regarding the rename of the package, I've read the Rename Process [1], but I'm still not sure if it's sufficient just to edit this request's Summary, or if a new review request is required, as this package doesn't replace any old packages. [1] https://fedoraproject.org/wiki/Package_Renaming_Process#Re-review_required
I think better close this as NOTABUG and open a new review request bug. If you can't close this just ask here and I will close this bug.
OK. Closing as NOTABUG as a package rename is expected.