Bug 1317392

Summary: Review Request: perl-Git-Version-Compare - Functions to compare Git versions
Product: [Fedora] Fedora Reporter: Jan Pazdziora <jpazdziora>
Component: Package ReviewAssignee: Petr Pisar <ppisar>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: jpazdziora, package-review, ppisar
Target Milestone: ---Flags: ppisar: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: perl-Git-Version-Compare-1.001-1.fc24 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2016-03-26 18:05:38 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Jan Pazdziora 2016-03-14 08:19:45 UTC
Spec URL: https://adelton.fedorapeople.org/perl-Git-Version-Compare.spec
SRPM URL: https://adelton.fedorapeople.org/perl-Git-Version-Compare-1.001-1.fc25.src.rpm
Description: Git::Version::Compare contains a selection of subroutines that make
dealing with Git-related things (like versions) a little bit easier.
Fedora Account System Username: adelton

Comment 1 Petr Pisar 2016-03-14 12:33:20 UTC
URL and Source0 addresses are usable. Ok.
Source archive is original (SHA-256: 1ffb4450b6dc445a705bfb174a29827169bf1f7094ae2cb88e94e54bdf95c7d1). Ok.
Summary verified from lib/Git/Version/Compare.pm. Ok.
Description verified from lib/Git/Version/Compare.pm. Ok.
License verified from lib/Git/Version/Compare.pm, README, LICENSE, Makefile.PL. Ok.
No XS code, noarch BuildArch is Ok.

TODO: Remove BuildRoot defintion and cleaning. This is done automatically in Fedora.
TODO: You can replace %__perl macros with plain perl command.
TODO: You can replace PERL_INSTALL_ROOT argument with DESTDIR argument to make it more similar to autootols.
TODO: You can remove command for deleting empty directories in the %install section. Current ExtUtils::MakeMaker does not create empty directories.
TODO: Remove %deffattr macro, this is implicit in Fedora.

FIX: Package LICENSE file using %license macro. Not using %doc macro.

TODO: Do not package dist.ini and META.json files. These are instructions for installation, not for using.

FIX: Build-require `make' (perl-Git-Version-Compare.spec:33).
FIX: Build-require `findutils (perl-Git-Version-Compare.spec:40).
FIX: Build-require `perl(strict)' (Makefile.PL:2).
FIX: Build-require `perl(warnings)' (Makefile.PL:3).

Test::Pod::Coverage not used. Ok.
Pod::Coverage::TrustPod not used. Ok.
Test::CPAN::Meta not used. Ok.
Test::Pod not used. Ok.

All tests pass. Ok.

$ rpmlint perl-Git-Version-Compare.spec ../SRPMS/perl-Git-Version-Compare-1.001-1.fc25.src.rpm ../RPMS/noarch/perl-Git-Version-Compare-1.001-1.fc25.noarch.rpm 
2 packages and 1 specfiles checked; 0 errors, 0 warnings.
rpmlint is Ok.

$ rpm -q -lv -p ../RPMS/noarch/perl-Git-Version-Compare-1.001-1.fc25.noarch.rpm 
drwxr-xr-x    2 root    root                        0 Mar 14 13:22 /usr/share/doc/perl-Git-Version-Compare
-rw-r--r--    1 root    root                      571 Feb 20 12:02 /usr/share/doc/perl-Git-Version-Compare/Changes
-rw-r--r--    1 root    root                    18379 Feb 20 12:02 /usr/share/doc/perl-Git-Version-Compare/LICENSE
-rw-r--r--    1 root    root                     1874 Feb 20 12:02 /usr/share/doc/perl-Git-Version-Compare/META.json
-rw-r--r--    1 root    root                      960 Feb 20 12:02 /usr/share/doc/perl-Git-Version-Compare/README
-rw-r--r--    1 root    root                     1065 Feb 20 12:02 /usr/share/doc/perl-Git-Version-Compare/dist.ini
-rw-r--r--    1 root    root                     3311 Mar 14 13:22 /usr/share/man/man3/Git::Version::Compare.3pm.gz
drwxr-xr-x    2 root    root                        0 Mar 14 13:22 /usr/share/perl5/vendor_perl/Git
drwxr-xr-x    2 root    root                        0 Mar 14 13:22 /usr/share/perl5/vendor_perl/Git/Version
-rw-r--r--    1 root    root                     9153 Feb 20 12:02 /usr/share/perl5/vendor_perl/Git/Version/Compare.pm
FIX: Package LICENSE file using %license macro. Not using %doc macro.

$ rpm -q --requires -p ../RPMS/noarch/perl-Git-Version-Compare-1.001-1.fc25.noarch.rpm | sort -f | uniq -c
      1 perl(:MODULE_COMPAT_5.22.1)
      2 perl(Carp)
      2 perl(Exporter)
      2 perl(namespace::clean)
      1 perl(strict)
      1 perl(warnings)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
TODO: Do not run-require `perl(Carp)', `perl(Exporter)', and `perl(namespace::clean)' explicitly. The are found automatically.

$ rpm -q --provides -p ../RPMS/noarch/perl-Git-Version-Compare-1.001-1.fc25.noarch.rpm | sort -f | uniq -c
      1 perl(Git::Version::Compare) = 1.001
      1 perl-Git-Version-Compare = 1.001-1.fc25
Binary provides are Ok.

$ resolvedeps rawhide ../RPMS/noarch/perl-Git-Version-Compare-1.001-1.fc25.noarch.rpm 
Binary dependencies resolvable. Ok.

Package build in F25 (http://koji.fedoraproject.org/koji/taskinfo?taskID=13343461). Ok.

Otherwise the package is in line with Fedora and Perl packaging guidelines.

Please correct all `FIX' items, consider fixing `TODO' items, and provide new spec file.
Resolution: Package NOT approved.

Comment 2 Jan Pazdziora 2016-03-14 13:08:03 UTC
Thank you very much.

The .spec at https://adelton.fedorapeople.org/perl-Git-Version-Compare.spec was updated with the patch below. I hope I managed to incorporate all FIXes and TODOs, except the PERL_INSTALL_ROOT one (which seems a bit more expressive than DESTDIR).

By the way, should we track the changes that were needed as requirements for cpanspec?

--- perl-Git-Version-Compare.spec.take1	2016-03-14 09:16:36.810425824 +0100
+++ perl-Git-Version-Compare.spec	2016-03-14 13:58:30.492172823 +0100
@@ -8,6 +8,9 @@
 Source0:        http://www.cpan.org/modules/by-module/Git/Git-Version-Compare-%{version}.tar.gz
 BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildArch:      noarch
+
+BuildRequires:  findutils
+BuildRequires:  make
 BuildRequires:  perl >= 0:5.006
 BuildRequires:  perl(Carp)
 BuildRequires:  perl(Exporter)
@@ -15,11 +18,11 @@
 BuildRequires:  perl(File::Spec)
 BuildRequires:  perl(namespace::clean)
 BuildRequires:  perl(Scalar::Util)
+BuildRequires:  perl(strict)
 BuildRequires:  perl(Test::More)
-Requires:       perl(Carp)
-Requires:       perl(Exporter)
-Requires:       perl(namespace::clean)
-Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))
+BuildRequires:  perl(warnings)
+
+Requires:       perl(:MODULE_COMPAT_%(eval "`perl -V:version`"; echo $version))
 
 %description
 Git::Version::Compare contains a selection of subroutines that make dealing
@@ -29,7 +32,7 @@
 %setup -q -n Git-Version-Compare-%{version}
 
 %build
-%{__perl} Makefile.PL INSTALLDIRS=vendor
+perl Makefile.PL INSTALLDIRS=vendor
 make %{?_smp_mflags}
 
 %install
@@ -38,19 +41,15 @@
 make pure_install PERL_INSTALL_ROOT=$RPM_BUILD_ROOT
 
 find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} \;
-find $RPM_BUILD_ROOT -depth -type d -exec rmdir {} 2>/dev/null \;
 
 %{_fixperms} $RPM_BUILD_ROOT/*
 
 %check
 make test
 
-%clean
-rm -rf $RPM_BUILD_ROOT
-
 %files
-%defattr(-,root,root,-)
-%doc Changes dist.ini LICENSE META.json README
+%license LICENSE
+%doc Changes README
 %{perl_vendorlib}/*
 %{_mandir}/man3/*

Comment 3 Petr Pisar 2016-03-14 13:50:54 UTC
> By the way, should we track the changes that were needed as requirements
> for cpanspec?

We know that cpanspec generates old spec files. We contacted upstream many times and he always claimed he will fix it soon.

Documenting the required/recommended changes could be helpful. Maybe this page <https://fedoraproject.org/wiki/Perl/Tips> could host it.

> TODO: Remove BuildRoot defintion and cleaning. This is done automatically
> in Fedora.
Partially addressed. You can still remove the first line in the %install section
and the `BuildRoot:' tag on line 9.

> TODO: You can replace %__perl macros with plain perl command.
Ok.

> TODO: You can replace PERL_INSTALL_ROOT argument with DESTDIR argument to make it
> more similar to autootols.
Not addressed. Ok.

> TODO: You can remove command for deleting empty directories in the %install section.
> Current ExtUtils::MakeMaker does not create empty directories.
Ok.

> TODO: Remove %deffattr macro, this is implicit in Fedora.
Ok.

> FIX: Package LICENSE file using %license macro. Not using %doc macro.
Ok.

> TODO: Do not package dist.ini and META.json files. These are instructions for
> installation, not for using.
Ok.

> FIX: Build-require `make' (perl-Git-Version-Compare.spec:33).
Ok.

> FIX: Build-require `findutils (perl-Git-Version-Compare.spec:40).
Ok.

> FIX: Build-require `perl(strict)' (Makefile.PL:2).
Ok.

> FIX: Build-require `perl(warnings)' (Makefile.PL:3).
Ok.

FIX: Build-require `coreutils' (perl-Git-Version-Compare.spec:43) or replace the "-exec rm -f {} \;" with "-delete" argument.

$ rpmlint perl-Git-Version-Compare.spec ../SRPMS/perl-Git-Version-Compare-1.001-1.fc25.src.rpm ../RPMS/noarch/perl-Git-Version-Compare-1.001-1.fc25.noarch.rpm 
2 packages and 1 specfiles checked; 0 errors, 0 warnings.
rpmlint is Ok.

> TODO: Do not run-require `perl(Carp)', `perl(Exporter)', and
> `perl(namespace::clean)' explicitly. The are found automatically.

$ rpm -q --requires -p ../RPMS/noarch/perl-Git-Version-Compare-1.001-1.fc25.noarch.rpm | sort -f | uniq -c
      1 perl(:MODULE_COMPAT_5.22.1)
      1 perl(Carp)
      1 perl(Exporter)
      1 perl(namespace::clean)
      1 perl(strict)
      1 perl(warnings)
      1 rpmlib(CompressedFileNames) <= 3.0.4-1
      1 rpmlib(FileDigests) <= 4.6.0-1
      1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1
      1 rpmlib(PayloadIsXz) <= 5.2-1
Binary requires are Ok.

Package builds in F25 (http://koji.fedoraproject.org/koji/taskinfo?taskID=13344091). Ok.

Please correct the `FIX' item before building this package.
Resolution: Package APPROVED.

Comment 4 Jan Pazdziora 2016-03-14 14:17:33 UTC
Thank you. For the record, the patch for items from comment 3 is:

--- perl-Git-Version-Compare.spec.take2	2016-03-14 13:58:30.492172823 +0100
+++ perl-Git-Version-Compare.spec	2016-03-14 15:07:11.201275239 +0100
@@ -6,7 +6,6 @@
 Group:          Development/Libraries
 URL:            http://search.cpan.org/dist/Git-Version-Compare/
 Source0:        http://www.cpan.org/modules/by-module/Git/Git-Version-Compare-%{version}.tar.gz
-BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildArch:      noarch
 
 BuildRequires:  findutils
@@ -36,11 +35,9 @@
 make %{?_smp_mflags}
 
 %install
-rm -rf $RPM_BUILD_ROOT
-
 make pure_install PERL_INSTALL_ROOT=$RPM_BUILD_ROOT
 
-find $RPM_BUILD_ROOT -type f -name .packlist -exec rm -f {} \;
+find $RPM_BUILD_ROOT -type f -name .packlist -delete
 
 %{_fixperms} $RPM_BUILD_ROOT/*

Comment 5 Gwyn Ciesla 2016-03-14 19:42:47 UTC
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/perl-Git-Version-Compare

Comment 6 Jan Pazdziora 2016-03-15 18:24:01 UTC
perl-Git-Version-Compare-1.001-1.fc24 was built.

Comment 8 Fedora Update System 2016-03-15 21:29:50 UTC
perl-Git-Version-Compare-1.001-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-3122ec6414

Comment 9 Fedora Update System 2016-03-16 09:31:52 UTC
perl-Git-Repository-1.318-1.fc24 perl-Git-Version-Compare-1.001-1.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2016-9af442bcb2

Comment 10 Fedora Update System 2016-03-18 15:00:26 UTC
perl-Git-Repository-1.318-1.fc24, perl-Git-Version-Compare-1.001-1.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-9af442bcb2

Comment 11 Fedora Update System 2016-03-26 18:05:32 UTC
perl-Git-Repository-1.318-1.fc24, perl-Git-Version-Compare-1.001-1.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.