Bug 980960

Aha...

1. Where is the URL tag, that means, where is the program's homepage?

2. Source0 should be a link which can directly be used to download the tarball or other sources. Only this style is for softwares which cannot provide a tarball.

3. "Requires:       perl" is incorrect, please see:

https://fedoraproject.org/wiki/Packaging:Perl#Versioned_MODULE_COMPAT_Requires

4. "Requires: /usr/bin/desktop-file-validate" is right but I think you can change it to desktop-file-utils

5. No need for "%clean section"/"%defattr(-,root,root,-)"/"rm -rf $RPM_BUILD_ROOT"(old habits should be trashed)

6. Do not mix variable and macros, you can change $RPM_BUILD_ROOT to %{buildroot}

7. If there are no docs, you don't need to write %doc with an empty line.

8. /usr/share in %files section can be replaced by %{_datadir}

I'm not a sponsor, but please consider a fix for above issues.

An additional note, I made the same mistake when I built my first RPM, that is you MUST NOT write the perl BuildRequires/Requires like "perl-Module-Build", don't use dash-style, but like this:

perl(Module::Build)

So perl-IO-Socket-SSL should be something like this:

perl(IO::Socket::SSL)

BTW, if RPM can automatically find these Requires(NOT BuildRequires, you still have to define which to be used for building), just use "Requires:  perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))" is enough.

Many, many thanks. I believe I've addressed each issue you brought up.

Diffs: https://git.fedorahosted.org/cgit/rpmgrill.git/commit/?id=0d77b8b89517aa150ec8b995389661a711107d48

New spec: http://www.edsantiago.com/f/rpmgrill.spec
New srpm: http://www.edsantiago.com/f/rpmgrill-0.22-1.fc17.src.rpm

New koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=5570468

Clarifying note: The requirement on perl(IO::Socket::SSL) was hand-specified because it's dynamically tested for by LWP::UserAgent inside an eval, so RPM couldn't autodetect it. I have removed the requirement from the specfile because that code isn't actually used in this version of rpmgrill.

Again, thanks for your thorough and helpful review. You clarified some packaging points I hadn't understood, and (blush) pointed out some that I missed in my haste.

You didn't mention your FAS username.

And BuildRoot is not really needed on Fedora, afaik, nor on EPEL 6

see comment:4 and comment:5

Fedora username: santiago

Buildroot: removed.

New spec: http://www.edsantiago.com/f/rpmgrill.spec
New srpm: http://www.edsantiago.com/f/rpmgrill-0.22-2.fc17.src.rpm

I'll take this.

This is an informal review, hope you can find a sponsor(I think it's easy for RH people):

1. SPEC itself:

- You can remove this:

find %{buildroot} -depth -type d -exec rmdir {} 2>/dev/null \;


- Requires: /usr/bin/xsltproc

can be 

  Requires: libxslt

- I cannot access 2 bugs mentioned in the spec, can you tell me something about them?

Package itself:

Unable to install:

 # ['/usr/bin/yum', '--installroot', '/var/lib/mock/fedora-rawhide-i386/root/', 'install', '/home/rpmaker/Desktop/rpmgrill/results/rpmgrill-0.22-2.fc20.noarch.rpm']
Error: Package: rpmgrill-0.22-2.fc20.noarch (/rpmgrill-0.22-2.fc20.noarch)
           Requires: perl(RPM::Grill::Util)

Again, thanks for your feedback. I'm really not sure why I changed the libxslt requirement to a path, but it's back. And the internal BZ references are gone, replaced with what I hope are useful descriptions of why those packages are needed.

The missing RPM::Grill::Util is pretty embarrassing. Sorry about that.

New spec, srpm, noarch rpm :

   http://www.edsantiago.com/f/rpmgrill.spec
   http://www.edsantiago.com/f/rpmgrill-0.23-1.fc19.src.rpm
   http://www.edsantiago.com/f/rpmgrill-0.23-1.fc19.noarch.rpm

(RPM::Grill::Util does not include a perl-findable $VERSION; I've just checked in a fix for that but have not rebuilt the rpm).

You've been very thorough and helpful. Thank you.

Hint:

http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

This time the package is good.

only

rpmgrill.noarch: E: explicit-lib-dependency libxslt

Is it really needed?

> Hint:
> 
> http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group

I'm taking that as "hint: start helping out with reviews" -- I'm still a bit intimidated. But I'm reading. Thank you.

> rpmgrill.noarch: E: explicit-lib-dependency libxslt
> 
> Is it really needed?

The actual requirement is /usr/bin/xsltproc . One of the plugins invokes it via qx{...}. Is there a better way to specify the dependency?

Not sure, leave it to your sponsor ;)

> - Requires: /usr/bin/xsltproc
> 
> can be 
> 
>   Requires: libxslt

Not a big issue, but: There is nothing wrong with requiring /usr/bin/xsltproc. It is more accurate than depending on a package name. The executable could move, for example.

> Not a big issue, but: There is nothing wrong with requiring
> /usr/bin/xsltproc. It is more accurate than depending on a package name. The
> executable could move, for example.

Thank you. That does seem like the right thing to do, since it removes a potentially confusing (and fragile) level of indirection.

$ rpmlint rpmgrill-0.23-1.fc19.src.rpm 
....
rpmgrill.src: E: specfile-error warning: bogus date in %changelog: Mon Jul 10 2013 Ed Santiago <santiago> 0.22-2
1 packages and 0 specfiles checked; 1 errors, 3 warnings.

Did you mean "Wed" instead of "Mon" ?

Updated:

   http://www.edsantiago.com/f/rpmgrill.spec
   http://www.edsantiago.com/f/rpmgrill-0.24-1.fc19.src.rpm
   http://www.edsantiago.com/f/rpmgrill-0.24-1.fc19.noarch.rpm

Changelog:

   * Mon Sep  9 2013 Ed Santiago <santiago> 0.24-1
   - test suite: clamav output differs between f17 & f19
   - specfile: fix bad date in changelog; re-update some Requires

I have sponsored Ed now. Will check this package review.

review:-

+ mock build is successful

+ rpmlint on rpms gave
rpmgrill.noarch: W: spelling-error Summary(en_US) koji -> Kojak
rpmgrill.noarch: W: spelling-error %description -l en_US unapplied -> unappealing
rpmgrill.noarch: W: spelling-error %description -l en_US multilib -> multilingual
rpmgrill.src: W: spelling-error Summary(en_US) koji -> Kojak
rpmgrill.src: W: spelling-error %description -l en_US unapplied -> unappealing
rpmgrill.src: W: spelling-error %description -l en_US multilib -> multilingual
2 packages and 0 specfiles checked; 0 errors, 6 warnings.

+ source verified with upstream as (sha256sum)
srpm tarball : ace439ae0f1fb7bfcca1172cbdded6bd556a91df05f57ca9283b1ddca1e0e3e2
upstream tarball : ace439ae0f1fb7bfcca1172cbdded6bd556a91df05f57ca9283b1ddca1e0e3e2


+ Package rpmgrill-0.24-1.fc19.noarch
Provides: perl(RPM::Grill) = 0.24 perl(RPM::Grill::Plugin::AAATemplate) = 0.24 perl(RPM::Grill::Plugin::BuildLog) = 0.24 perl(RPM::Grill::Plugin::DesktopLint) = 0.24 perl(RPM::Grill::Plugin::ElfChecks) = 0.24 perl(RPM::Grill::Plugin::LibGather) = 0.24 perl(RPM::Grill::Plugin::ManPages) = 0.24 perl(RPM::Grill::Plugin::Manifest) = 0.24 perl(RPM::Grill::Plugin::Multilib) = 0.24 perl(RPM::Grill::Plugin::Patches) = 0.24 perl(RPM::Grill::Plugin::RpmScripts) = 0.24 perl(RPM::Grill::Plugin::SecurityPolicy) = 0.24 perl(RPM::Grill::Plugin::Setxid) = 0.24 perl(RPM::Grill::Plugin::SpecFileEncoding) = 0.24 perl(RPM::Grill::Plugin::SpecFileSanity) = 0.24 perl(RPM::Grill::Plugin::VirusCheck) = 0.24 perl(RPM::Grill::RPM) = 0.24 perl(RPM::Grill::RPM::Files) = 0.24 perl(RPM::Grill::RPM::Metadata) = 0.24 perl(RPM::Grill::RPM::SpecFile) = 0.24 perl(RPM::Grill::RPM::SpecFile::Line) perl(RPM::Grill::Util) = 0.24 perl(RPM::Grill::dprintf) = 0.24 rpmgrill = 0.24-1.fc19
 
Requires: /usr/bin/perl perl(Algorithm::Diff) perl(CGI) perl(Carp) perl(Encode) perl(Errno) perl(Fcntl) perl(File::Basename) perl(File::Copy) perl(File::LibMagic) perl(File::Path) perl(File::Temp) perl(File::Which) perl(Getopt::Long) perl(HTML::Entities) perl(IPC::Run) perl(JSON::XS) perl(LWP::Simple) perl(List::Util) perl(Pod::POM) perl(Pod::POM::View::HTML) perl(RPM::Grill) perl(RPM::Grill::Plugin::SecurityPolicy) perl(RPM::Grill::RPM) perl(RPM::Grill::RPM::Files) perl(RPM::Grill::RPM::Metadata) perl(RPM::Grill::Util) perl(RPM::Grill::dprintf) perl(Sort::Versions) perl(Text::ParseWords) perl(Tie::File) perl(Time::ParseDate) perl(Time::Piece) perl(XML::Simple) perl(YAML) perl(YAML::Syck) perl(base) perl(constant) perl(open) perl(overload) perl(parent) perl(strict) perl(utf8) perl(warnings)


Looks good but I can't find license file or license text in any perl module file. If you are the upstream then try to include it and release new tarball and package it here.

Oops. Updated:

   http://www.edsantiago.com/f/rpmgrill.spec
   http://www.edsantiago.com/f/rpmgrill-0.25-1.fc19.src.rpm
   http://www.edsantiago.com/f/rpmgrill-0.25-1.fc19.noarch.rpm

Thanks for your sponsorship and review.

You should now package LICENSE file also.
add %doc LICENSE in spec file.


APPROVED.

LICENSE (and README.AAAA_FIRST) now packaged:

   http://www.edsantiago.com/f/rpmgrill.spec
   http://www.edsantiago.com/f/rpmgrill-0.25-2.fc19.src.rpm
   http://www.edsantiago.com/f/rpmgrill-0.25-2.fc19.noarch.rpm

It looks like I should package a ChangeLog as well; now on my todo list.

New Package SCM Request
=======================
Package Name: rpmgrill
Short Description: 
Owners: santiago halfie
Branches: f19 f20
InitialCC:

Hi Ed,
your request should include this
Short Description: A utility for catching problems in koji builds

you can also read how to request SCM for your package at http://fedoraproject.org/wiki/Package_SCM_admin_requests

you should re-submit above request with correct input again.

New Package SCM Request
=======================
Package Name: rpmgrill
Short Description: A utility for catching problems in koji builds
Owners: santiago halfie
Branches: f19 f20
InitialCC:

Git done (by process-git-requests).

Built on rawhide, however no builds on f20/f19.

Closed for Rawhide.

You are not authorized to access bug #876702.

> You are not authorized to access bug #876702.

Sorry! It's an internal RFE for opening rpmgrill. As part of closing it, I linked to this bz -- forgetting the symmetrical nature of the link.