Bug 2072968 - Review Request: perl-Alien-libmaxminddb - Find libmaxminddb
Summary: Review Request: perl-Alien-libmaxminddb - Find libmaxminddb
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Michal Josef Spacek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-NEEDSPONSOR 2072972
TreeView+ depends on / blocked
 
Reported: 2022-04-07 11:25 UTC by Andreas Vögele
Modified: 2023-06-16 08:02 UTC (History)
5 users (show)

Fixed In Version: perl-Alien-libmaxminddb-1.012-1.fc39
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2023-06-16 07:44:02 UTC
Type: ---
Embargoed:
mspacek: fedora-review+


Attachments (Terms of Use)

Description Andreas Vögele 2022-04-07 11:25:20 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/voegelas/fedora/fedora-rawhide-x86_64/05942861-perl-Alien-libmaxminddb/perl-Alien-libmaxminddb.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/voegelas/fedora/fedora-rawhide-x86_64/05942861-perl-Alien-libmaxminddb/perl-Alien-libmaxminddb-1.012-1.fc39.src.rpm
Description: An Alien module for Perl that provides the C library libmaxminddb to other modules
Fedora Account System Username: voegelas

This is my first package. There will be another package named perl-IP-Geolocation-MMDB that depends on this package. I am the upstream author of both modules. I am looking for a sponsor.

Comment 1 Michal Josef Spacek 2023-05-18 10:29:20 UTC
Sorry, I forgot about this review.

Comment 2 Michal Josef Spacek 2023-05-18 10:54:54 UTC
@andreas Hi Andreas, could we update the spec file to the last version of the module from CPAN?

Review:
Source file is ok
Summary is ok
License need to update license string to "GPL-1.0-or-later OR Artistic-1.0-Perl". There is new format in SPDX form.
Description is ok
URL and Source0 are ok
All tests passed
BuildRequires are ok

> rpm -qp --requires perl-Alien-libmaxminddb-1.006-1.fc39.x86_64.rpm  | sort | uniq -c | grep -v rpmlib
      1 perl(Alien::Base)
      1 perl(Alien::libmaxminddb)
      1 perl-libs
      1 perl(:MODULE_COMPAT_5.36.1)
      1 perl(parent)
      1 perl(strict)
      1 perl(utf8)
      1 perl(:VERSION) >= 5.16.0
      1 perl(warnings)
      1 pkgconfig(libmaxminddb)

We need to remove "Requires:       perl(:MODULE_COMPAT_%(eval "`perl -V:version`"; echo $version))" line from spec file.

> rpm -qp --provides perl-Alien-libmaxminddb-1.006-1.fc39.x86_64.rpm | sort | uniq -c
      1 perl(Alien::libmaxminddb::Install::Files)
      1 perl-Alien-libmaxminddb(x86-64) = 1.006-1.fc39
      1 perl(Alien::libmaxminddb) = 1.006
      1 perl-Alien-libmaxminddb = 1.006-1.fc39

Rpmlint is ok

btw: If you put spec and srpm files, there is automatic process which helping review.

Comment 3 Andreas Vögele 2023-05-19 11:50:20 UTC
Hello Michal,

thanks a lot for the review. I've removed the MODULE_COMPAT requirement. I had already updated the version and the license in Copr. Today's build with review output in the "fedora-review" subfolder is here:

https://download.copr.fedorainfracloud.org/results/voegelas/fedora/fedora-rawhide-x86_64/05934631-perl-Alien-libmaxminddb/

Kind regards,
Andreas

Spec URL: https://download.copr.fedorainfracloud.org/results/voegelas/fedora/fedora-rawhide-x86_64/05934631-perl-Alien-libmaxminddb/perl-Alien-libmaxminddb.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/voegelas/fedora/fedora-rawhide-x86_64/05934631-perl-Alien-libmaxminddb/perl-Alien-libmaxminddb-1.012-1.fc39.src.rpm
Description: An Alien module for Perl that provides the C library libmaxminddb to other modules
Fedora Account System Username: voegelas

Comment 4 Michal Josef Spacek 2023-05-22 10:58:50 UTC
Hi Andreas,

I think that you can remove BuildRequires for coreutils, gcc, and perl-devel
The library delivers libmaxminddb, detect it and sets files. 
There is no compiling.

Other seems ok.

Comment 5 Fedora Review Service 2023-05-22 10:59:03 UTC
Hello @voegelas,
since this is your first Fedora package, you need to get sponsored by a package
sponsor before it can be accepted.

A sponsor is an experienced package maintainer who will guide you through
the processes that you will follow and the tools that you will use as a future
maintainer. A sponsor will also be there to answer your questions related to
packaging.

You can find all active sponsors here:
https://docs.pagure.org/fedora-sponsors/

I created a sponsorship request for you:
https://pagure.io/packager-sponsors/issue/570
Please take a look and make sure the information is correct.

Thank you, and best of luck on your packaging journey.

---
This comment was created by the fedora-review-service
https://github.com/FrostyX/fedora-review-service

Comment 6 Michal Josef Spacek 2023-05-22 11:01:25 UTC
Sorry I set review as done, but not done.

Comment 7 Paul Howarth 2023-05-22 15:30:38 UTC
(In reply to Michal Josef Spacek from comment #4)
> I think that you can remove BuildRequires for coreutils ...

The %{_fixperms} macro is implemented using chmod, hence it makes sense to retain coreutils as a build requirement, though it's hard to imagine the build system not having that by default.

Comment 8 Michal Josef Spacek 2023-05-22 16:00:11 UTC
(In reply to Paul Howarth from comment #7)
> (In reply to Michal Josef Spacek from comment #4)
> > I think that you can remove BuildRequires for coreutils ...
> 
> The %{_fixperms} macro is implemented using chmod, hence it makes sense to
> retain coreutils as a build requirement, though it's hard to imagine the
> build system not having that by default.

Heh, thanks. I thought it was a residue of COMPAT.

Comment 9 Andreas Vögele 2023-05-22 18:03:51 UTC
(In reply to Michal Josef Spacek from comment #4)
> Hi Andreas,
> 
> I think that you can remove BuildRequires for coreutils, gcc, and perl-devel
> The library delivers libmaxminddb, detect it and sets files. 
> There is no compiling.

There's a test that compiles C code, which is at the end of t/xs.t. I've moved perl-devel and gcc to the spec file's "Tests" requirements.

Spec URL: https://download.copr.fedorainfracloud.org/results/voegelas/fedora/fedora-rawhide-x86_64/05942861-perl-Alien-libmaxminddb/perl-Alien-libmaxminddb.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/voegelas/fedora/fedora-rawhide-x86_64/05942861-perl-Alien-libmaxminddb/perl-Alien-libmaxminddb-1.012-1.fc39.src.rpm

Comment 10 Michal Josef Spacek 2023-05-22 19:30:41 UTC
Review:
Source file is ok
Summary is ok
License is ok
Description is ok
URL and Source0 are ok
All tests passed
BuildRequires are ok

> rpm -qp --requires perl-Alien-libmaxminddb-1.012-1.fc39.x86_64.rpm | sort | uniq -c | grep -v rpmlib
      1 perl(Alien::Base)
      1 perl(Alien::libmaxminddb)
      1 perl-libs
      1 perl(parent)
      1 perl(strict)
      1 perl(utf8)
      1 perl(:VERSION) >= 5.16.0
      1 perl(warnings)
      1 pkgconfig(libmaxminddb)

> rpm -qp --provides perl-Alien-libmaxminddb-1.012-1.fc39.x86_64.rpm | sort | uniq -c
      1 perl(Alien::libmaxminddb::Install::Files)
      1 perl-Alien-libmaxminddb(x86-64) = 1.012-1.fc39
      1 perl(Alien::libmaxminddb) = 1.012
      1 perl-Alien-libmaxminddb = 1.012-1.fc39

Rpmlint is ok

Package looks good now.

Resolution:
Approved

Comment 11 Michal Josef Spacek 2023-05-22 19:33:13 UTC
(In reply to Andreas Vögele from comment #9)
> (In reply to Michal Josef Spacek from comment #4)
> > I think that you can remove BuildRequires for coreutils, gcc, and perl-devel
> > The library delivers libmaxminddb, detect it and sets files. 
> > There is no compiling.
> 
> There's a test that compiles C code, which is at the end of t/xs.t. I've
> moved perl-devel and gcc to the spec file's "Tests" requirements.

You are right, thank you.

Comment 12 Michal Josef Spacek 2023-06-13 11:31:45 UTC
@andreas Hi Andreas, you were sponsored. You could request branch and build package.

Comment 13 Fedora Admin user for bugzilla script actions 2023-06-13 16:03:50 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/perl-Alien-libmaxminddb

Comment 14 Petr Pisar 2023-06-16 08:02:38 UTC
Andreas, I recommend you to register Alien-libmaxminddb upstream project at <https://release-monitoring.org/projects/search/?pattern=Alien-libmaxminddb> release monitoring service and set there a mapping to Fedora's perl-Alien-libmaxminddb package. This enables you to automatically receive a new report in Bugzilla whenever upstream releases a new version on CPAN. See e.g. Alien-Build <https://release-monitoring.org/project/15795/> for a comparison.


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