Bug 812583 - Review Request: perl-FusionInventory-Agent-Task-NetInventory - Remote inventory support for FusionInventory Agent
Review Request: perl-FusionInventory-Agent-Task-NetInventory - Remote invento...
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Petr Pisar
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-15 02:43 EDT by Remi Collet
Modified: 2012-05-27 02:11 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-05-27 02:11:18 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
ppisar: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Remi Collet 2012-04-15 02:43:43 EDT
Spec URL: https://raw.github.com/remicollet/remirepo/master/perl-FusionInventory-Agent-Task-NetInventory/perl-FusionInventory-Agent-Task-NetInventory.spec
SRPM URL: http://rpms.famillecollet.com/SRPMS/perl-FusionInventory-Agent-Task-NetInventory-2.1-1.remi.src.rpm
Description: 
This task allows the FusionInventory agent to extracts various informations
from remote hosts through SNMP protocol:
* printer cartridges and counters status
* router/switch ports status
* relations between devices and router/switch ports


--
The package  perl-FusionInventory-Agent-Task-SNMPQuery, already in the repository, have been renamed by upstream.

The new package/version is needed to update Agent to version 2.2.0

Others SRPMS needed for this review:
http://rpms.famillecollet.com/SRPMS/fusioninventory-agent-2.2.0-1.remi.src.rpm
http://rpms.famillecollet.com/SRPMS/perl-FusionInventory-Agent-Task-NetDiscovery-2.1-1.remi.src.rpm


I plan an update in fedora 17 ASAP, and in other branches and EPEL later.
Comment 1 Petr Pisar 2012-05-07 06:58:04 EDT
Source file is original. Ok.
URL and Source0 are usable. Ok

TODO: Upstream links source tarballs to different place (http://forge.fusioninventory.org/projects/fusioninventory-agent-task-snmpquery/files). Please align Source0.

This package is a rename and it obsoletes-provides old package properly. Ok.

Summary verified from lib/FusionInventory/Agent/Task/NetInventory.pm. Ok.
License verified from README and LICENSE. Ok.
Description verified from README. Ok.
This package does not use any XS code. noarch BuildArch is Ok.
BuildRoot defined and cleaned because of EPEL-5 compatibility. Ok.

??? Missing run-time requires for tests

FIX: Build-require `perl(inc::Module::Install)' instead of `perl(ExtUtils::MakeMaker) (Makefile.PL:1).
FIX: Build-require `perl(File::Which)' (Makefile.PL:13).

TODO: Consider removing bundled inc/* files to use the ones provided by distribution. Otherwise you will need to build-require all modules used by by the inc/* code (like YAML::Tiny, File::Temp, etc.)

FIX: Build-require all inc/* code dependencies.

FIX: Build-require `perl(threads)' for running tests (lib/FusionInventory/Agent/Task/NetInventory.pm:5).
FIX: Build-require `perl(threads::shared)' for running tests (lib/FusionInventory/Agent/Task/NetInventory.pm:6).

TODO: Build-require `perl(base)' for running tests (lib/FusionInventory/Agent/Task/NetInventory.pm:7).

FIX: Build-require `perl(FusionInventory::Agent::Task)' for runnig tests (lib/FusionInventory/Agent/Task/NetInventory.pm:7).

TODO: Build-require `perl(constant)' for running tests (lib/FusionInventory/Agent/Task/NetInventory.pm:9).
TODO: Build-require `perl(Encode)' for running tests (lib/FusionInventory/Agent/Task/NetInventory.pm:14).

FIX: Build-require `perl(FusionInventory::Agent::SNMP)' for running tests (lib/FusionInventory/Agent/Task/NetInventory.pm:17).
FIX: Build-require `perl(FusionInventory::Agent::XML::Query)' for running tests (lib/FusionInventory/Agent/Task/NetInventory.pm:18).
FIX: Build-require `perl(FusionInventory::Agent::Tools)' for running tests (lib/FusionInventory/Agent/Task/NetInventory.pm:19).
FIX: Build-require `perl(FusionInventory::Agent::Tools::Network)' for running tests (lib/FusionInventory/Agent/Task/NetInventory.pm:20).
FIX: Build-require `perl(FusionInventory::Agent::XML::Response)' for running tests (t/message.t:9).

FIX: Package does not build because of missing `perl(FusionInventory::Agent::Task::NetDiscovery) >= 2.1' build-requirement.

This is fatal.

Please correct the `FIX' issues and provide new spec file.

Resolution: Package NOT approved.
Comment 2 Remi Collet 2012-05-07 10:07:35 EDT
(In reply to comment #1)
> FIX: Package does not build because of missing
> `perl(FusionInventory::Agent::Task::NetDiscovery) >= 2.1' build-requirement.
> 
> This is fatal.

See link to the .src.rpm in the above description.

I couldn't push this versions (agent 2.2, NetDiscovery 2.1) in the repository before all the stuff is approved.
Comment 3 Remi Collet 2012-05-07 10:42:51 EDT
I prefer to use the CPAN URL which is "stable" between version (upstream redmine URL is really awful, need to be changes on each release)

Changes:
https://github.com/remicollet/remirepo/commit/4e59195140a9262bf8a9c8407c9e2fc7d7e1e301


Do we really need all this BR perl(Fusion...),  as they are implicit by requiring "perl(FusionInventory::Agent::Task::NetDiscovery) >= 2.1" ?
Comment 4 Petr Pisar 2012-05-09 09:45:20 EDT
Spec file changes:

--- perl-FusionInventory-Agent-Task-NetInventory.spec.old       2012-05-07 11:07:33.509000209 +0200
+++ perl-FusionInventory-Agent-Task-NetInventory.spec   2012-05-09 15:02:32.519999974 +0200
@@ -1,6 +1,6 @@
 Name:           perl-FusionInventory-Agent-Task-NetInventory
 Version:        2.1
-Release:        1%{?dist}
+Release:        2%{?dist}
 Summary:        Remote inventory support for FusionInventory Agent
 License:        GPLv2+
 Group:          Development/Libraries
@@ -11,11 +11,22 @@
 BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildArch:      noarch
 BuildRequires:  perl >= 1:5.8.0
-BuildRequires:  perl(ExtUtils::MakeMaker)
+BuildRequires:  perl(inc::Module::Install)
 # For tests
-BuildRequires:  perl(FusionInventory::Agent::Task::NetDiscovery) >= 2.1
 BuildRequires:  perl(Test::More)
 BuildRequires:  perl(Test::Compile)
+BuildRequires:  perl(base)
+BuildRequires:  perl(constant)
+BuildRequires:  perl(threads)
+BuildRequires:  perl(threads::shared)
+BuildRequires:  perl(Encode)
+BuildRequires:  perl(FusionInventory::Agent::Task::NetDiscovery) >= 2.1
+BuildRequires:  perl(FusionInventory::Agent::Task)
+BuildRequires:  perl(FusionInventory::Agent::SNMP)
+BuildRequires:  perl(FusionInventory::Agent::XML::Query)
+BuildRequires:  perl(FusionInventory::Agent::Tools)
+BuildRequires:  perl(FusionInventory::Agent::Tools::Network)
+BuildRequires:  perl(FusionInventory::Agent::XML::Response)
 
 Requires:       perl(FusionInventory::Agent::Task::NetDiscovery) >= 2.1
 Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))
@@ -38,6 +49,10 @@
 %prep
 %setup -q -n FusionInventory-Agent-Task-NetInventory-%{version}
 
+# Use system ones
+rm -rf inc/*
+
+
 %build
 perl Makefile.PL \
      PREFIX=%{_prefix} \
@@ -75,6 +90,9 @@
 
 
 %changelog
+* Mon May 07 2012 Remi Collet <remi@fedoraproject.org> - 2.1-2
+- changes from review (#812583)
+
 * Sun Apr 15 2012 Remi Collet <remi@fedoraproject.org> - 2.1-1
 - rename to perl-FusionInventory-Agent-Task-NetInventory
 - update to 2.1 for agent 2.2.0


> > TODO: Upstream links source tarballs to different place
> > (http://forge.fusioninventory.org/projects/fusioninventory-agent-task-snmpquery/files).
> > Please align Source0.
> I prefer to use the CPAN URL which is "stable" between version (upstream
> redmine URL is really awful, need to be changes on each release)
That makes sense. Ok.


> FIX: Build-require `perl(inc::Module::Install)' instead of
> `perl(ExtUtils::MakeMaker) (Makefile.PL:1).
-BuildRequires:  perl(ExtUtils::MakeMaker)
+BuildRequires:  perl(inc::Module::Install)
Ok.

> FIX: Build-require `perl(File::Which)' (Makefile.PL:13).
TODO: It's in eval, let's make it TODO. I think checking fusioninventory-agent version is good idea.

> TODO: Consider removing bundled inc/* files to use the ones provided by
> distribution. Otherwise you will need to build-require all modules used by by
> the inc/* code (like YAML::Tiny, File::Temp, etc.)
> FIX: Build-require all inc/* code dependencies.
+# Use system ones
+rm -rf inc/*
Ok.

> FIX: Build-require `perl(threads)' for running tests
> (lib/FusionInventory/Agent/Task/NetInventory.pm:5).
+BuildRequires:  perl(threads)
Ok.

> FIX: Build-require `perl(threads::shared)' for running tests
> (lib/FusionInventory/Agent/Task/NetInventory.pm:6).
+BuildRequires:  perl(threads::shared)
Ok.

> TODO: Build-require `perl(base)' for running tests
> (lib/FusionInventory/Agent/Task/NetInventory.pm:7).
+BuildRequires:  perl(base)
Ok.

> FIX: Build-require `perl(FusionInventory::Agent::Task)' for runnig tests
> (lib/FusionInventory/Agent/Task/NetInventory.pm:7).
+BuildRequires:  perl(FusionInventory::Agent::Task)
Ok.

> TODO: Build-require `perl(constant)' for running tests
> (lib/FusionInventory/Agent/Task/NetInventory.pm:9).
+BuildRequires:  perl(constant)
Ok.

> TODO: Build-require `perl(Encode)' for running tests
> (lib/FusionInventory/Agent/Task/NetInventory.pm:14).
+BuildRequires:  perl(Encode)
Ok.

> FIX: Build-require `perl(FusionInventory::Agent::SNMP)' for running tests
> (lib/FusionInventory/Agent/Task/NetInventory.pm:17).
> FIX: Build-require `perl(FusionInventory::Agent::XML::Query)' for running tests
> (lib/FusionInventory/Agent/Task/NetInventory.pm:18).
> FIX: Build-require `perl(FusionInventory::Agent::Tools)' for running tests
> (lib/FusionInventory/Agent/Task/NetInventory.pm:19).
> FIX: Build-require `perl(FusionInventory::Agent::Tools::Network)' for running
> tests (lib/FusionInventory/Agent/Task/NetInventory.pm:20).
> FIX: Build-require `perl(FusionInventory::Agent::XML::Response)' for running
> tests (t/message.t:9).
+BuildRequires:  perl(FusionInventory::Agent::SNMP)
+BuildRequires:  perl(FusionInventory::Agent::XML::Query)
+BuildRequires:  perl(FusionInventory::Agent::Tools)
+BuildRequires:  perl(FusionInventory::Agent::Tools::Network)
+BuildRequires:  perl(FusionInventory::Agent::XML::Response)
Ok.

> > FIX: Package does not build because of missing
> > `perl(FusionInventory::Agent::Task::NetDiscovery) >= 2.1' build-requirement.
>
> I couldn't push this versions (agent 2.2, NetDiscovery 2.1) in the repository
> before all the stuff is approved.

So you have problem with bootstrapping these three packages because of cyclic dependencies. Then you need put those dependencies under a condition and make the condition false until the dependencies get into Fedora.

You need to fill separate review requests for each package. Maybe you will find starting with another package of these three is easier.

Because perl maintainers rebuild all packages on each minor perl release (once a year usually), please keep the condition there and use `perl_bootstrap' for the condition macro value.

See <http://pkgs.fedoraproject.org/gitweb/?p=perl-HTTP-Message.git;a=blob_plain;f=perl-HTTP-Message.spec;hb=HEAD> for an example.

Define the perl_bootstrap macro to 1 at the beginning of your spec file to bootstrap your three modules, and remove the definition only (not the conditions) after all of them appear in Fedora build root. See 
<http://ppisar.fedorapeople.org/perl-Test-Pod-No404s/perl-Test-Pod-No404s.spec> for an example.

Please provide adjusted spec file.
Comment 5 Remi Collet 2012-05-10 01:43:25 EDT
Sorry, I think I wasn't clear enough in my explanation.

There is no cyclic dependencies.

All perl-Fusion* requires fusioninventory-agent (they are optional plugins)

Already in the repository (since f14):
* fusioninventory-agent 2.1.14
* perl-FusionInventory-Agent-Task-ESX 1.1.3
* perl-FusionInventory-Agent-Task-NetDiscovery 1.5
* perl-FusionInventory-Agent-Task-SNMPQuery 1.3

So only perl-FusionInventory-Agent-Task-NetInventory (rename of SNMPQuery) and perl-FusionInventory-Agent-Task-Deploy (new plugin) are submitted for review.

As I cannot update SNMPQuery (renamed), I haven't update the others to avoid broken dep (because of layout change between 2.1 and 2.2)

As this package is now under review, I think I can do the update (and tag SNMPQuery as EOL) in rawhide to make this review possible.
Comment 6 Petr Pisar 2012-05-10 03:29:40 EDT
I see. But I still think there is something wrong with dependencies.

This perl-FusionInventory-Agent-Task-NetInventory.spec says:

BuildRequires:  perl(FusionInventory::Agent::Task::NetDiscovery) >= 2.1
Requires:       perl(FusionInventory::Agent::Task::NetDiscovery) >= 2.1

But latest available perl(FusionInventory::Agent::Task::NetDiscovery) in F18 is perl(FusionInventory::Agent::Task::NetDiscovery) = 1.5.

I don't know why this package requires FusionInventory::Agent::Task::NetInventory and why in this version, but if it requires, you need to upgrade perl-FusionInventory-Agent-Task-NetDiscovery to 2.1 first. If it does not require, then it should not be written in this spec file.
Comment 7 Remi Collet 2012-05-10 14:49:46 EDT
I have update, in rawhide, all the packages already in the repository:
* fusioninventory-agent-2.2.0-1.fc18
* perl-FusionInventory-Agent-Task-NetDiscovery-2.1-1.fc18
* perl-FusionInventory-Agent-Task-ESX-2.1.0-1.fc18

So, I think, all the dependencies are now available.
Comment 8 Petr Pisar 2012-05-11 05:23:19 EDT
Great. Now I can build the package.

All tests pass. Ok.

$ rpmlint perl-FusionInventory-Agent-Task-NetInventory.spec ../SRPMS/perl-FusionInventory-Agent-Task-NetInventory-2.1-2.fc18.src.rpm ../RPMS/noarch/perl-FusionInventory-Agent-Task-NetInventory-2.1-2.fc18.noarch.rpm 
perl-FusionInventory-Agent-Task-NetInventory.src: W: spelling-error %description -l en_US informations -> information, information's, in formations
perl-FusionInventory-Agent-Task-NetInventory.noarch: W: spelling-error %description -l en_US informations -> information, information's, in formations
2 packages and 1 specfiles checked; 0 errors, 2 warnings.

TODO: Correct invalid word `informations' in the description. English has `information' always in singular. Plural is `data'. So I think grammatically correct sentence is `... to extracts various data...'.

$ rpm -q -lv -p ../RPMS/noarch/perl-FusionInventory-Agent-Task-NetInventory-2.1-2.fc18.noarch.rpm                                         drwxr-xr-x    2 root    root                        0 May 11 10:35 /usr/share/doc/perl-FusionInventory-Agent-Task-NetInventory-2.1
-rw-r--r--    1 root    root                     1268 Apr 12 21:02 /usr/share/doc/perl-FusionInventory-Agent-Task-NetInventory-2.1/Changes
-rw-r--r--    1 root    root                    17987 Apr 13  2010 /usr/share/doc/perl-FusionInventory-Agent-Task-NetInventory-2.1/LICENSE
-rw-r--r--    1 root    root                     1418 Apr 10 17:38 /usr/share/doc/perl-FusionInventory-Agent-Task-NetInventory-2.1/README
-rw-r--r--    1 root    root                       99 Apr 10 17:38 /usr/share/doc/perl-FusionInventory-Agent-Task-NetInventory-2.1/THANKS
drwxr-xr-x    2 root    root                        0 May 11 10:35 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/NetInventory
-rw-r--r--    1 root    root                    24516 Apr 12 21:04 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/NetInventory.pm
drwxr-xr-x    2 root    root                        0 May 11 10:35 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/NetInventory/Manufacturer
-rw-r--r--    1 root    root                      614 Apr 10 17:38 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/NetInventory/Manufacturer.pm
-rw-r--r--    1 root    root                     1756 Apr 10 17:38 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/NetInventory/Manufacturer/3Com.pm
-rw-r--r--    1 root    root                     1753 Apr 10 17:38 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/NetInventory/Manufacturer/AlliedTelesis.pm
-rw-r--r--    1 root    root                     3926 Apr 10 17:38 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/NetInventory/Manufacturer/Cisco.pm
-rw-r--r--    1 root    root                     3600 Apr 10 17:38 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/NetInventory/Manufacturer/Nortel.pm
-rw-r--r--    1 root    root                     3578 Apr 10 17:38 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/NetInventory/Manufacturer/Procurve.pm
-rw-r--r--    1 root    root                     1969 May 11 10:35 /usr/share/man/man3/FusionInventory::Agent::Task::NetInventory.3pm.gz
-rw-r--r--    1 root    root                     2004 May 11 10:35 /usr/share/man/man3/FusionInventory::Agent::Task::NetInventory::Manufacturer.3pm.gz
-rw-r--r--    1 root    root                     1992 May 11 10:35 /usr/share/man/man3/FusionInventory::Agent::Task::NetInventory::Manufacturer::3Com.3pm.gz
-rw-r--r--    1 root    root                     1999 May 11 10:35 /usr/share/man/man3/FusionInventory::Agent::Task::NetInventory::Manufacturer::AlliedTelesis.3pm.gz
-rw-r--r--    1 root    root                     2065 May 11 10:35 /usr/share/man/man3/FusionInventory::Agent::Task::NetInventory::Manufacturer::Cisco.3pm.gz
-rw-r--r--    1 root    root                     2036 May 11 10:35 /usr/share/man/man3/FusionInventory::Agent::Task::NetInventory::Manufacturer::Nortel.3pm.gz
-rw-r--r--    1 root    root                     2000 May 11 10:35 /usr/share/man/man3/FusionInventory::Agent::Task::NetInventory::Manufacturer::Procurve.3pm.gz

FIX: Why the Perl code is not installed into standard paths? I see it's because Makefile.PL adjusts it to setting of fusioninventory-agent. Modules installed into non-standard paths are not available for other Perl code and thus must not be exported as `perl()' RPM symbols. You should fix all fusioninventory-agent RPM packages. So either you need to move the modules into standard path and keep the RPM Provides, or you can keep the modules in private location but you need to remove the RPM provides (there are __provides_exclude_from and similar macros for that <http://fedoraproject.org/wiki/User:Tibbs/AutoProvidesAndRequiresFiltering>).

$ rpm -q --requires -p ../RPMS/noarch/perl-FusionInventory-Agent-Task-NetInventory-2.1-2.fc18.noarch.rpm |sort |uniq -c
      1 perl(base)
      1 perl(constant)
      1 perl(Encode)
      1 perl(English)
      1 perl(FusionInventory::Agent::SNMP)
      1 perl(FusionInventory::Agent::Task)
      1 perl(FusionInventory::Agent::Task::NetDiscovery) >= 2.1
      1 perl(FusionInventory::Agent::Tools)
      1 perl(FusionInventory::Agent::Tools::Network)
      1 perl(FusionInventory::Agent::XML::Query)
      1 perl(:MODULE_COMPAT_5.14.2)
      1 perl(strict)
      1 perl(threads)
      1 perl(threads::shared)
      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

FIX: If you do not relocate the fusioninventory-agent Perl modules to standard path, remove the `perl(FusionInventory::Agent::*' dependencies because they are private. Then you will need to depend on fusioninventory-agent symbol.
FIX: The same applies to BuildRequires.

$ rpm -q --provides -p ../RPMS/noarch/perl-FusionInventory-Agent-Task-NetInventory-2.1-2.fc18.noarch.rpm |sort |uniq -c
      1 perl(FusionInventory::Agent::Task::NetInventory) = 2.1
      1 perl-FusionInventory-Agent-Task-NetInventory = 2.1-2.fc18
      1 perl(FusionInventory::Agent::Task::NetInventory::Manufacturer)
      1 perl(FusionInventory::Agent::Task::NetInventory::Manufacturer::3Com)
      1 perl(FusionInventory::Agent::Task::NetInventory::Manufacturer::AlliedTelesis)
      1 perl(FusionInventory::Agent::Task::NetInventory::Manufacturer::Cisco)
      1 perl(FusionInventory::Agent::Task::NetInventory::Manufacturer::Nortel)
      1 perl(FusionInventory::Agent::Task::NetInventory::Manufacturer::Procurve)
      1 perl-FusionInventory-Agent-Task-SNMPQuery = 2.1-2.fc18
FIX: Remove these private `perl(FusionInventory::Agent::Task::NetInventory' Provides, if you do not relocate the files into standard path.

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


Please provide updated spec file.

Resolution: Package NOT approved.
Comment 9 Remi Collet 2012-05-11 10:53:49 EDT
Relocation outside standard path is an upstream choice, so I have add filter for private provides/requires.

https://github.com/remicollet/remirepo/commit/296d60eea235b33b185dd4dd6161410bc0fa3c1d
Comment 10 Petr Pisar 2012-05-14 03:52:42 EDT
Spec file changes:

--- perl-FusionInventory-Agent-Task-NetInventory.spec.old       2012-05-09 15:02:32.519999974 +0200
+++ perl-FusionInventory-Agent-Task-NetInventory.spec   2012-05-14 09:24:17.161998991 +0200
@@ -1,6 +1,6 @@
 Name:           perl-FusionInventory-Agent-Task-NetInventory
 Version:        2.1
-Release:        2%{?dist}
+Release:        3%{?dist}
 Summary:        Remote inventory support for FusionInventory Agent
 License:        GPLv2+
 Group:          Development/Libraries
@@ -20,26 +20,26 @@
 BuildRequires:  perl(threads)
 BuildRequires:  perl(threads::shared)
 BuildRequires:  perl(Encode)
-BuildRequires:  perl(FusionInventory::Agent::Task::NetDiscovery) >= 2.1
-BuildRequires:  perl(FusionInventory::Agent::Task)
-BuildRequires:  perl(FusionInventory::Agent::SNMP)
-BuildRequires:  perl(FusionInventory::Agent::XML::Query)
-BuildRequires:  perl(FusionInventory::Agent::Tools)
-BuildRequires:  perl(FusionInventory::Agent::Tools::Network)
-BuildRequires:  perl(FusionInventory::Agent::XML::Response)
+BuildRequires:  perl-FusionInventory-Agent-Task-NetDiscovery >= 2.1

-Requires:       perl(FusionInventory::Agent::Task::NetDiscovery) >= 2.1
+Requires:       perl-FusionInventory-Agent-Task-NetDiscovery >= 2.1
 Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))

 # upstream have rename from SNMPQuery 1.3 to NetInventory 2.0
 Obsoletes:      perl-FusionInventory-Agent-Task-SNMPQuery < 2.0
 Provides:       perl-FusionInventory-Agent-Task-SNMPQuery = %{version}-%{release}

+# RPM 4.8
+%{?filter_from_provides: %filter_from_provides /perl(FusionInventory::/d}
+%{?filter_from_requires: %filter_from_requires /perl(FusionInventory::/d}
 %{?perl_default_filter}
+# RPM 4.9
+%global __provides_exclude %{?__provides_exclude:__provides_exclude|}^perl\\(FusionInventory::
+%global __requires_exclude %{?__requires_exclude:__requires_exclude|}^perl\\(FusionInventory::


 %description
-This task allows the FusionInventory agent to extracts various informations
+This task allows the FusionInventory agent to collect information
 from remote hosts through SNMP protocol:
 * printer cartridges and counters status
 * router/switch ports status
@@ -90,6 +90,10 @@


 %changelog
+* Fri May 11 2012 Remi Collet <remi@fedoraproject.org> - 2.1-3
+- filter private provides/requires
+- fix description (in sync with upstream)
+
 * Mon May 07 2012 Remi Collet <remi@fedoraproject.org> - 2.1-2
 - changes from review (#812583)


> TODO: Correct invalid word `informations' in the description. English has
> `information' always in singular. Plural is `data'. So I think grammatically
> correct sentence is `... to extracts various data...'.
 %description
-This task allows the FusionInventory agent to extracts various informations
+This task allows the FusionInventory agent to collect information
Ok.

-BuildRequires:  perl(FusionInventory::Agent::Task)
-BuildRequires:  perl(FusionInventory::Agent::XML::Query)
-BuildRequires:  perl(FusionInventory::Agent::Tools)
-BuildRequires:  perl(FusionInventory::Agent::Tools::Network)
-BuildRequires:  perl(FusionInventory::Agent::XML::Response)
FIX: Now, you should depend on `FusionInventory-Agent'.

-BuildRequires:  perl(FusionInventory::Agent::Task::NetDiscovery) >= 2.1
-BuildRequires:  perl(FusionInventory::Agent::SNMP)
+BuildRequires:  perl-FusionInventory-Agent-Task-NetDiscovery >= 2.1
Ok.

$ rpmlint perl-FusionInventory-Agent-Task-NetInventory.spec ../SRPMS/perl-FusionInventory-Agent-Task-NetInventory-2.1-3.fc18.src.rpm ../RPMS/noarch/perl-FusionInventory-Agent-Task-NetInventory-2.1-3.fc18.noarch.rpm 
2 packages and 1 specfiles checked; 0 errors, 0 warnings.
Ok.

> FIX: If you do not relocate the fusioninventory-agent Perl modules to standard
> path, remove the `perl(FusionInventory::Agent::*' dependencies because they
> are private. Then you will need to depend on fusioninventory-agent symbol.
$ rpm -q --requires -p  ../RPMS/noarch/perl-FusionInventory-Agent-Task-NetInventory-2.1-3.fc18.noarch.rpm | sort | uniq -c
      1 perl(base)
      1 perl(constant)
      1 perl(Encode)
      1 perl(English)
      1 perl-FusionInventory-Agent-Task-NetDiscovery >= 2.1
      1 perl(:MODULE_COMPAT_5.14.2)
      1 perl(strict)
      1 perl(threads)
      1 perl(threads::shared)
      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
FIX: Require `FusionInventory-Agent' because it uses modules from that package.

> FIX: Remove these private `perl(FusionInventory::Agent::Task::NetInventory'
> Provides, if you do not relocate the files into standard path.
$ rpm -q --provides -p  ../RPMS/noarch/perl-FusionInventory-Agent-Task-NetInventory-2.1-3.fc18.noarch.rpm | sort | uniq -c
      1 perl-FusionInventory-Agent-Task-NetInventory = 2.1-3.fc18
      1 perl-FusionInventory-Agent-Task-SNMPQuery = 2.1-3.fc18
Ok.

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

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


Please correct all `FIX' prefixed item and provide new spec file.

Resolution: Package NOT approved.
Comment 11 Remi Collet 2012-05-14 04:44:31 EDT
add explicit BR/R on fusioninventory-agent:

https://github.com/remicollet/remirepo/commit/baa3ffd35b23f9669ce34a8e05f76a023fa00521
Comment 12 Petr Pisar 2012-05-14 05:31:40 EDT
Spec file changes:

--- perl-FusionInventory-Agent-Task-NetInventory.spec.old       2012-05-14 09:24:17.161998991 +0200
+++ perl-FusionInventory-Agent-Task-NetInventory.spec   2012-05-14 11:20:58.076994420 +0200
@@ -1,6 +1,6 @@
 Name:           perl-FusionInventory-Agent-Task-NetInventory
 Version:        2.1
-Release:        3%{?dist}
+Release:        4%{?dist}
 Summary:        Remote inventory support for FusionInventory Agent
 License:        GPLv2+
 Group:          Development/Libraries
@@ -20,10 +20,12 @@
 BuildRequires:  perl(threads)
 BuildRequires:  perl(threads::shared)
 BuildRequires:  perl(Encode)
+BuildRequires:  fusioninventory-agent >= 2.2.0
 BuildRequires:  perl-FusionInventory-Agent-Task-NetDiscovery >= 2.1

-Requires:       perl-FusionInventory-Agent-Task-NetDiscovery >= 2.1
 Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))
+Requires:       fusioninventory-agent >= 2.2.0
+Requires:       perl-FusionInventory-Agent-Task-NetDiscovery >= 2.1

 # upstream have rename from SNMPQuery 1.3 to NetInventory 2.0
 Obsoletes:      perl-FusionInventory-Agent-Task-SNMPQuery < 2.0
@@ -90,6 +92,9 @@


 %changelog
+* Mon May 14 2012 Remi Collet <remi@fedoraproject.org> - 2.1-4
+- add explicit BR/R on fusioninventory-agent
+
 * Fri May 11 2012 Remi Collet <remi@fedoraproject.org> - 2.1-3
 - filter private provides/requires
 - fix description (in sync with upstream)


> FIX: Now, you should depend on `FusionInventory-Agent'.
+BuildRequires:  fusioninventory-agent >= 2.2.0
Ok.

$ rpmlint perl-FusionInventory-Agent-Task-NetInventory.spec ../SRPMS/perl-FusionInventory-Agent-Task-NetInventory-2.1-4.fc18.src.rpm ../RPMS/noarch/perl-FusionInventory-Agent-Task-NetInventory-2.1-4.fc18.noarch.rpm 
2 packages and 1 specfiles checked; 0 errors, 0 warnings.
rpmlint is Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-FusionInventory-Agent-Task-NetInventory-2.1-4.fc18.noarch.rpm |sort |uniq -c
      1 fusioninventory-agent >= 2.2.0
      1 perl(base)
      1 perl(constant)
      1 perl(Encode)
      1 perl(English)
      1 perl-FusionInventory-Agent-Task-NetDiscovery >= 2.1
      1 perl(:MODULE_COMPAT_5.14.2)
      1 perl(strict)
      1 perl(threads)
      1 perl(threads::shared)
      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
> FIX: Require `FusionInventory-Agent' because it uses modules from that
> package.
Ok.

$ resolvedeps rawhide ../RPMS/noarch/perl-FusionInventory-Agent-Task-NetInventory-2.1-4.fc18.noarch.rpm
Binary dependencies resolvable. Ok.

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

Package is in line with Fedora and Perl packaging guidelines.

Thank your for big patience. The package is much better now.

Resolution: Package APPROVED.
Comment 13 Remi Collet 2012-05-14 05:37:13 EDT
Thanks for the review and for your patience too

New Package SCM Request
=======================
Package Name: perl-FusionInventory-Agent-Task-NetInventory
Short Description: Remote inventory support for FusionInventory Agent
Owners: remi
Branches: 
InitialCC: perl-sig
Comment 14 Gwyn Ciesla 2012-05-14 08:09:42 EDT
Git done (by process-git-requests).

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