Bug 1242726

I'll take a look and possibly sponsor you.

Before I start -- besides Fedora, do plan to support EPEL as well?  If so, which versions?

If the modules are appropriate for EPEL, I will support them. I am open to discussion about whether they are appropriate :)

Given that most of my work is in perl 5.18, 5.20 and 5.22 I do not want to try to support OSes that are too old. I would plan to support EPEL 7 and 6 only at this time. I do not personally use RHEL or CentOS, but I do not want to exclude that community.

Thank you for your consideration.

EL7 ships with perl5.16, EL6 with perl5.10; hoever, the majority of modules on CPAN maintains compatibility with 5.8, sometimes even 5.6.  This should be fine.

The reason I was asking is that, when packaging for EPEL, you need to use some, from Fedora's point of view, obsolete constructs in your SPEC files.

So, the review...  I'll start from the top :)

* You should always package the latest version available.  In this case, that would be 0.004002, released in April.  The difference[0] is minimal so I'll just continue reviewing the version you've packaged for now.  Updating will be straightforward.

* The package summary could be a little bit more descriptive, "A Moose role to add support for logging via Log::Any", maybe?

* Note: The Group tag is optional nowdays.  Still, since you're packaging for EPEL, you need to keep it.

* The BuildRoot tag is only needed in EPEL5.  You may drop it.

* The perl version constraint isn't really all that useful in our case.

* Build dependencies -- Packaging Guidelines used to list a set of packages that were guaranteed to be present in every buildroot.  That is no longer the case, therefore I suggest explicitly listing everything your package is directly using at build time, including every perl mdoule or pragma, no matter how common they might be.

Your package calls `make' (lines 35, 40 and 48), `find' (lines 42 and 43), and `rm' and `rmdir' (lines 38, 42, 43 and 51).  You should therefore buildrequire `make', `findutils' and `coreutils'.  If you drop some of these (see below), drop the added dependency too, of course.

You also execute Makefile.PL, which requires `strict' and `warnings'.  Add `perl(strict)' and `perl(warnings)' to your list.

Log::Any::Adapter isn't actually used anywhere in the code, it's only mentioned in the metadata and, judging from the changelog, I think upstream meant to remove it.  Drop it from your BuildRequires.

* Runtime dependencies -- rpmbuild tries its best and generates most runtime dependencies automatically by scanning the files present in the resulting RPM package.  There's also some basic support for Perl (via perl-generators) so you don't have to list all the required perl modules explicitly.  In your case, both `Moo::Role' and and `Log::Any' are automatically detected.  Furthermore, as I've already mentioned, Log::Any::Adapter isn't used at all.  You can drop these three `Requires' lines.

To check the resulting dependencies, query the package with the --requires/-R option:
$ rpm -qRp perl-MooX-Log-Any-0.004001-1.fc23.noarch.rpm
perl(:MODULE_COMPAT_5.22.0)
perl(Log::Any)
perl(Log::Any)
perl(Log::Any::Adapter)
perl(Moo::Role)
perl(Moo::Role)
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(PayloadIsXz) <= 5.2-1

(you can see the duplicates here -- one comes from perl-generators, one from your explicit `Requires')

* Note: Line 34, `%{__perl} Makefile.PL INSTALLDIRS=vendor' -- this is fine.  I just want to mention the possibility to add `NO_PACKLIST=1' with EU::MM 6.76+ to supress packlist generation.  This is only supported in Fedora.  EPEL's EE::MM is too old.

* Line 38, `rm -rf $RPM_BUILD_ROOT' -- this is no longer needed unless you're packaging for EPEL5.

* Note: Line 40, `make pure_install PERL_INSTALL_ROOT=$RPM_BUILD_ROOT' -- DESTDIR is supported in both Fedora and EPEL.  You can use it instead of PERL_INSTALL_ROOT here.

* Note: Line 42, `find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} \;' -- you wouldn't need this with NO_PACKLIST.  Just so you know.

* Line 43 (`find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null \;') isn't necessary.

* The whole %clean section is only needed in EPEL5.  Drop it.

* The standard %defattr definition is no longer needed.  Not even in EPEL.  Drop it.

* Documentation.  Don't package `dist.ini', `weaver.ini' or `META.json'.  These have no value for the end user.  Also, `README.md' doesn't provide anything the module perldoc doesn't.  In you case, I would keep just the simple `README" file.

The license text requires special handling.  Fedora mandates[1] that license texts are installed with a special macro, %license, which ensures they get installed even if documentation is disabled.  However, EPEL doesn't support this macro (yet?).  There are many ways to work around this.  You can, for example, check whether %_licensedir is defined and if it isn't, define %license as %doc:

%{!?_licensedir:%global license %%doc}
%license LICENSE
%doc Changes README
...

* Finally, your changelog format isn't valid[2] (missing e-mail address).


That should be it.  Feel free to ask if anything's unclear.


[0] https://metacpan.org/diff/file?target=CAZADOR/MooX-Log-Any-0.004002/&source=CAZADOR/MooX-Log-Any-0.004001/

[1] https://fedoraproject.org/wiki/Packaging:LicensingGuidelines

[2] https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs

Spec URL: https://ttorling.fedorapeople.org/perl-MooX-Log-Any/perl-MooX-Log-Any.spec

SRPM URL: https://ttorling.fedorapeople.org/perl-MooX-Log-Any/perl-MooX-Log-Any-0.004002-1.fc23.src.rpm

Git URL: https://fedorapeople.org/cgit/ttorling/public_git/perl-MooX-Log-Any.git/

koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=10426934

> * You should always package the latest version available.  In this case,
> that would be 0.004002, released in April.  The difference[0] is minimal so
> I'll just continue reviewing the version you've packaged for now.  Updating
> will be straightforward.

Ack, updated.

> * The package summary could be a little bit more descriptive, "A Moose role
> to add support for logging via Log::Any", maybe?

Ack, summary changed.

> * The BuildRoot tag is only needed in EPEL5.  You may drop it.

Ack, dropped.

> * The perl version constraint isn't really all that useful in our case.

Ack, version contraint dropped.

> Your package calls `make' (lines 35, 40 and 48), `find' (lines 42 and 43),
> and `rm' and `rmdir' (lines 38, 42, 43 and 51).  You should therefore
> buildrequire `make', `findutils' and `coreutils'.  If you drop some of these
> (see below), drop the added dependency too, of course.

Ack, all three added.

> You also execute Makefile.PL, which requires `strict' and `warnings'.  Add
> `perl(strict)' and `perl(warnings)' to your list.

Ack, both added.

> Log::Any::Adapter isn't actually used anywhere in the code, it's only
> mentioned in the metadata and, judging from the changelog, I think upstream
> meant to remove it.  Drop it from your BuildRequires.

Ack, dropped.

> * Runtime dependencies -- (...) In your
> case, both `Moo::Role' and and `Log::Any' are automatically detected. 
> Furthermore, as I've already mentioned, Log::Any::Adapter isn't used at all.
> You can drop these three `Requires' lines.

Ack, all three dropped.

> * Line 38, `rm -rf $RPM_BUILD_ROOT' -- this is no longer needed unless
> you're packaging for EPEL5.

Ack, dropped.

> * Note: Line 40, `make pure_install PERL_INSTALL_ROOT=$RPM_BUILD_ROOT' --
> DESTDIR is supported in both Fedora and EPEL.  You can use it instead of
> PERL_INSTALL_ROOT here.

Ack, changed to DESTDIR. 

> * Line 43 (`find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null
> \;') isn't necessary.
>
> * The whole %clean section is only needed in EPEL5.  Drop it.
> 
> * The standard %defattr definition is no longer needed.  Not even in EPEL. 
> Drop it.

Ack, all dropped.

> * Documentation.  Don't package `dist.ini', `weaver.ini' or `META.json'. 
> These have no value for the end user.  Also, `README.md' doesn't provide
> anything the module perldoc doesn't.  In you case, I would keep just the
> simple `README" file.
> 
> The license text requires special handling.  Fedora mandates[1] that license
> texts are installed with a special macro, %license, which ensures they get
> installed even if documentation is disabled.  However, EPEL doesn't support
> this macro (yet?).  There are many ways to work around this.  You can, for
> example, check whether %_licensedir is defined and if it isn't, define
> %license as %doc:
> 
> %{!?_licensedir:%global license %%doc}
> %license LICENSE
> %doc Changes README
> ...

Ack, looks good.

> * Finally, your changelog format isn't valid[2] (missing e-mail address).

Ack, also fixed.

--

That went well!  I'm going to approve this package and sponsor you into the Packager group.  You may proceed with an SCM request[0].

Since this is a perl package, please, add `perl-sig' to InitialCC.

Once the package is created in the PkgDB, you may also want to register it in Anitya[1], our upstream release monitoring service.  It automatically checks for new versions, submits FMN[2] notifications and, optionally, files `please update' bugs for you.

[0] https://fedoraproject.org/wiki/Package_SCM_admin_requests
[1] https://release-monitoring.org/
[2] https://apps.fedoraproject.org/notifications/

New Package SCM Request
=======================
Package Name: perl-MooX-Log-Any
Short Description: A Moose role to add support for logging via Log::Any
Upstream URL: http://search.cpan.org/dist/MooX-Log-Any/
Owners: ttorling psabata group::perl-sig
Branches: f21 f22 f23 el6 epel7
InitialCC: perl-sig

perl-MooX-Log-Any-0.004002-1.fc21 has been submitted as an update for Fedora 21.
https://admin.fedoraproject.org/updates/perl-MooX-Log-Any-0.004002-1.fc21

perl-MooX-Log-Any-0.004002-1.fc22 has been submitted as an update for Fedora 22.
https://admin.fedoraproject.org/updates/perl-MooX-Log-Any-0.004002-1.fc22

Exists.

This package is still missing perl-sig in watchbugzilla and watchcommits roles.

(In reply to Jon Ciesla from comment #10)
> Exists.

You actually created it on the 23rd, just didn't switch to cvs flag to + for some reason...  Also, as Petr says, the perl-sig watcher is missing.

perl-MooX-Log-Any-0.004002-1.fc22 has been pushed to the Fedora 22 stable repository.

perl-MooX-Log-Any-0.004002-1.fc21 has been pushed to the Fedora 21 stable repository.

Did you submit a new-package update for Fedora 23, too?

I thought I had, but I'm not seeing it in bodhi, either.
In trying to create a new update:

Creating a new update for  perl-MooX-Log-Any-0.004002-1.fc23 
Traceback (most recent call last):
  File "/usr/bin/bodhi", line 532, in <module>
    main()
  File "/usr/bin/bodhi", line 225, in main
    data = bodhi.save(**update_args)
  File "/usr/lib/python2.7/site-packages/fedora/client/bodhi.py", line 107, in wrapper
    raise BodhiClientException(problems)
fedora.client.bodhi.BodhiClientException: Invalid tag: perl-MooX-Log-Any-0.004002-1.fc23 tagged with [u'f23-updates-candidate', u'epel7-testing-candidate', u'dist-6E-epel-testing-candidate', u'dist-5E-epel-testing-candidate', u'f22-updates-candidate', u'f21-updates-candidate']
Unable to determine release from build: perl-MooX-Log-Any-0.004002-1.fc23
Could not generate update request: Command 'bodhi --new --release f23 --file bodhi.template perl-MooX-Log-Any-0.004002-1.fc23 --username ttorling' returned non-zero exit status 1

Could it be you're using an old version (pre 0.5.5) of fedora-python?  I haven't tried the CLI tools since Bodhi2 was deployed last week.  It all feels rather broken.

Try the web interface; it might work for you.

No, I have updated to 0.5.5-2, unless bodhi-client and fedpkg also need to be updated?
Web interface gave the same exact failure.
I thought perhaps it needed a fresh koji build, so I invoked "fedpkg build --skip-nvr-check" and the build failed?

http://koji.fedoraproject.org/koji/taskinfo?taskID=10856325

It seems the package is already in stable, as I remembered:
http://mirror.utexas.edu/fedora/linux/development/23/x86_64/os/Packages/p/perl-MooX-Log-Any-0.004002-1.fc23.noarch.rpm

Not sure what is going on with bodhi?

Waiting to hear some input on my issue:
https://github.com/fedora-infra/bodhi/issues/658

perl-MooX-Log-Any-0.004003-1.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-338741eda1

This issue will be fixed/closed by #1276810

perl-MooX-Log-Any-0.004003-1.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
If you want to test the update, you can install it with
$ su -c 'dnf --enablerepo=updates-testing update perl-MooX-Log-Any'
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-338741eda1

perl-MooX-Log-Any-0.004003-1.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.