Bug 812587

Summary:	Review Request: perl-FusionInventory-Agent-Task-Deploy - Software deployment support for FusionInventory Agent
Product:	[Fedora] Fedora	Reporter:	Remi Collet <fedora>
Component:	Package Review	Assignee:	Petr Pisar <ppisar>
Status:	CLOSED RAWHIDE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	notting, package-review, ppisar
Target Milestone:	---	Flags:	ppisar: fedora-review+ gwync: fedora-cvs+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2012-09-01 07:02:35 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:
Bug Depends On:	812586
Bug Blocks:

Description Remi Collet 2012-04-15 07:39:59 UTC

Spec URL: https://raw.github.com/remicollet/remirepo/master/perl-FusionInventory-Agent-Task-Deploy/perl-FusionInventory-Agent-Task-Deploy.spec
SRPM URL: http://rpms.famillecollet.com/SRPMS/perl-FusionInventory-Agent-Task-Deploy-2.0.0-1.remi.src.rpm
Description: 
With this module, FusionInventory can accept software deployment request
from an GLPI server with the FusionInventory plugin.

--
Need for the review:
http://rpms.famillecollet.com/SRPMS/fusioninventory-agent-2.2.0-1.remi.src.rpm

Comment 1 Remi Collet 2012-05-11 15:02:46 UTC

Cleanups + filter :

https://github.com/remicollet/remirepo/commit/a558fcd820da11efddbb6873ca9c09281a50948c

Comment 2 Petr Pisar 2012-05-29 12:00:52 UTC

Source tar ball is original. Ok.
Summary verified from lib/FusionInventory/Agent/Task/Deploy.pm. Ok.
License verified from README and LICENSE. Ok.
URL and Source0 are usable. Ok.
No XS code. noarch BuildArch is Ok.

FIX: Build- and run-require `perl(POE::Component::Client::TCP)' (lib/FusionInventory/Agent/Task/Deploy/P2P.pm:9).
FIX: Build-require `perl(File::Which)' (Makefile.PL:13).

TODO: Build-require `perl(base)' (lib/FusionInventory/Agent/Task/Deploy.pm:8).

FIX: Build-require `perl(LWP)' (lib/FusionInventory/Agent/Task/Deploy.pm:11).
FIX: Build-require `perl(Compress::Zlib)' (lib/FusionInventory/Agent/Task/Deploy/Datastore/WorkDir.pm:7).

TODO: Build-require `perl(File::Path)' (lib/FusionInventory/Agent/Task/Deploy/Datastore/WorkDir.pm:9).
TODO: Build-require `perl(Exporter)' (lib/FusionInventory/Agent/Task/Deploy/DiskFree.pm:5).

FIX: Build-require `perl(UNIVERSAL::require)' (lib/FusionInventory/Agent/Task/Deploy/DiskFree.pm:10).
FIX: Build-require `perl(File::Spec)' (lib/FusionInventory/Agent/Task/Deploy/Datastore.pm:8).

FIX: Build-require `perl(HTTP::Request::Common)' (lib/FusionInventory/Agent/Task/Deploy/P2P.pm:7).
FIX: Build-require `perl(Net::IP)' (lib/FusionInventory/Agent/Task/Deploy/P2P.pm:8).
FIX: Build-require `perl(POE)' (lib/FusionInventory/Agent/Task/Deploy/P2P.pm:9).
FIX: Build-require `perl(Cwd)' (lib/FusionInventory/Agent/Task/Deploy/ActionProcessor.pm:6).
FIX: Build-require `perl(HTTP::Request)' (lib/FusionInventory/Agent/Task/Deploy/File.pm:10).

FIX: Build-require `perl(Test::More)' (t/06spelling.t:6).
FIX: Build-require `perl(HTTP::Server::Simple::CGI)' (t/server.t:7).
FIX: Build-require `perl(Data::Dumper)' (t/server.t:12).

TODO: I can't see why perl(HTTP::Server::Simple::Authen) and perl(CGI) are build-required. The modules are used nowhere. Are they needed?

TODO: You can remove the BuildRoot tag and it's cleaning if your are not intent to build this package in EPEL-5. The actions are implicit in newer distributions.

TODO: Remove the %defattr from %files section. This is implicit in all distributions.

All tests pass. Ok.

TODO: Report a bug to upstream about undefined value (Use of uninitialized value in string eq at /builddir/build/BUILD/FusionInventory-Agent-Task-Deploy-2.0.0/blib/lib/FusionInventory/Agent/Task/Deploy/CheckProcessor.pm line 14.).


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

$ rpm -q -lv -p ../RPMS/noarch/perl-FusionInventory-Agent-Task-Deploy-2.0.0-2.fc18.noarch.rpm 
drwxr-xr-x    2 root    root                        0 kvě 29 07:40 /usr/share/doc/perl-FusionInventory-Agent-Task-Deploy-2.0.0
-rw-r--r--    1 root    root                     2250 dub  8 15:07 /usr/share/doc/perl-FusionInventory-Agent-Task-Deploy-2.0.0/Changes
-rw-r--r--    1 root    root                    17987 zář 20  2011 /usr/share/doc/perl-FusionInventory-Agent-Task-Deploy-2.0.0/LICENSE
-rw-r--r--    1 root    root                      725 bře 18 19:11 /usr/share/doc/perl-FusionInventory-Agent-Task-Deploy-2.0.0/README
drwxr-xr-x    2 root    root                        0 kvě 29 07:40 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy
-rw-r--r--    1 root    root                    16869 dub  8 15:08 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy.pm
drwxr-xr-x    2 root    root                        0 kvě 29 07:40 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/ActionProcessor
-rw-r--r--    1 root    root                     2243 dub  8 15:04 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/ActionProcessor.pm
drwxr-xr-x    2 root    root                        0 kvě 29 07:40 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/ActionProcessor/Action
-rw-r--r--    1 root    root                     2884 dub  8 15:04 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/ActionProcessor/Action/Cmd.pm
-rw-r--r--    1 root    root                      711 dub  8 15:04 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/ActionProcessor/Action/Copy.pm
-rw-r--r--    1 root    root                      470 dub  8 15:04 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/ActionProcessor/Action/Delete.pm
-rw-r--r--    1 root    root                      689 dub  8 15:04 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/ActionProcessor/Action/Mkdir.pm
-rw-r--r--    1 root    root                      702 dub  8 15:04 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/ActionProcessor/Action/Move.pm
-rw-r--r--    1 root    root                     2999 dub  8 15:04 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/CheckProcessor.pm
drwxr-xr-x    2 root    root                        0 kvě 29 07:40 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/Datastore
-rw-r--r--    1 root    root                     2487 dub  8 15:04 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/Datastore.pm
-rw-r--r--    1 root    root                     2437 dub  8 15:04 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/Datastore/WorkDir.pm
-rw-r--r--    1 root    root                     1899 dub  8 15:04 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/DiskFree.pm
-rw-r--r--    1 root    root                     4434 dub  8 15:04 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/File.pm
-rw-r--r--    1 root    root                      797 dub  8 15:04 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/Job.pm
-rw-r--r--    1 root    root                     5496 dub  8 15:04 /usr/share/fusioninventory/lib/FusionInventory/Agent/Task/Deploy/P2P.pm
-rw-r--r--    1 root    root                     1987 kvě 29 07:40 /usr/share/man/man3/FusionInventory::Agent::Task::Deploy.3pm.gz
The Fusion Perl modules are private. Non-standard location is Ok.
File permissions and layout is Ok.

$ rpm -q --requires -p ../RPMS/noarch/perl-FusionInventory-Agent-Task-Deploy-2.0.0-2.fc18.noarch.rpm |sort |uniq -c
      1 fusioninventory-agent >= 2.2.0
      1 perl(Archive::Extract)
      1 perl(base)
      1 perl(Compress::Zlib)
      1 perl(Cwd)
      2 perl(Digest::SHA)
      1 perl(English)
      1 perl(Exporter)
      1 perl(File::Basename)
      1 perl(File::Copy::Recursive)
      1 perl(File::Glob)
      1 perl(File::Path)
      1 perl(File::Spec)
      1 perl(HTTP::Request)
      1 perl(HTTP::Request::Common)
      1 perl(JSON)
      1 perl(LWP)
      1 perl(:MODULE_COMPAT_5.14.2)
      1 perl(Net::IP)
      1 perl(POE)
      1 perl(POE::Component::Client::Ping)
      1 perl(strict)
      1 perl(UNIVERSAL::require)
      1 perl(URI::Escape)
      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(Digest::SHA)' explicitly. It's auto-detected.
FIX: Run-require `perl(POE::Component::Client::TCP)' (lib/FusionInventory/Agent/Task/Deploy/P2P.pm:9).

$ rpm -q --provides -p ../RPMS/noarch/perl-FusionInventory-Agent-Task-Deploy-2.0.0-2.fc18.noarch.rpm |sort |uniq -c
      1 perl-FusionInventory-Agent-Task-Deploy = 2.0.0-2.fc18
The Fusion::* modules are private, thus filtered.
Binary provides are Ok.

$ resolvedeps rawhide ../RPMS/noarch/perl-FusionInventory-Agent-Task-Deploy-2.0.0-2.fc18.noarch.rpm 
Binary dependencies resolvable. Ok.

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


Please correct all `FIX' issues, consider fixing `TODO' items and provide new spec file.

Resolution: Package NOT approved.

Comment 3 Remi Collet 2012-05-29 18:15:03 UTC

(In reply to comment #2)
> TODO: Report a bug to upstream about undefined value (Use of uninitialized
> value in string eq at
> /builddir/build/BUILD/FusionInventory-Agent-Task-Deploy-2.0.0/blib/lib/
> FusionInventory/Agent/Task/Deploy/CheckProcessor.pm line 14.).

http://forge.fusioninventory.org/issues/1591

I will provide a new spec ASAP.

Comment 4 Remi Collet 2012-05-30 15:45:55 UTC

Thanks for the comments.
I'm still sorry for missing BR (explicit vs implicit...)

All FIX and most TODO applied in 
https://github.com/remicollet/remirepo/commit/adef8d5a0d1e7cf165bfd3fea227d14267ffa1ea
(I still prefer to keep old stuff, builroot, defattr, clean, ... until RHEL-5 EOL)

Koji scratch BUILD
http://koji.fedoraproject.org/koji/taskinfo?taskID=4114765

Comment 5 Remi Collet 2012-05-30 16:38:45 UTC

<off topic>

Petr, can you please give me how to you detect BR ? Have a script or does (find ... | /usr/lib/rpm/perl.req) could be enough ?

I think I also need to mostly rewrite the BR+R for fusioninventory-agent (I will be happy to have any comment, from you, on this package)

Comment 6 Petr Pisar 2012-05-31 08:14:51 UTC

Spec file changes:
--- perl-FusionInventory-Agent-Task-Deploy.spec.old     2012-05-29 07:05:23.025998958 -0400
+++ perl-FusionInventory-Agent-Task-Deploy.spec 2012-05-31 03:55:12.122006698 -0400
@@ -1,6 +1,6 @@
 Name:           perl-FusionInventory-Agent-Task-Deploy
 Version:        2.0.0
-Release:        2%{?dist}
+Release:        3%{?dist}
 Summary:        Software deployment support for FusionInventory Agent
 License:        GPLv2+
 Group:          Development/Libraries
@@ -10,24 +10,38 @@
 BuildRoot:      %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 BuildArch:      noarch
 BuildRequires:  perl >= 1:5.8.0
+BuildRequires:  perl(base)
 BuildRequires:  perl(inc::Module::Install)
 BuildRequires:  perl(Archive::Extract)
 BuildRequires:  perl(Archive::Tar)
+BuildRequires:  perl(Compress::Zlib)
+BuildRequires:  perl(Cwd)
+BuildRequires:  perl(Data::Dumper)
 BuildRequires:  perl(Digest::SHA)
+BuildRequires:  perl(Exporter)
 BuildRequires:  perl(File::Copy::Recursive)
+BuildRequires:  perl(File::Path)
+BuildRequires:  perl(File::Spec)
+BuildRequires:  perl(File::Which)
+BuildRequires:  perl(HTTP::Request)
+BuildRequires:  perl(HTTP::Request::Common)
+BuildRequires:  perl(HTTP::Server::Simple::CGI)
 BuildRequires:  perl(JSON)
+BuildRequires:  perl(LWP)
+BuildRequires:  perl(Net::IP)
+BuildRequires:  perl(POE)
 BuildRequires:  perl(POE::Component::Client::Ping)
+BuildRequires:  perl(POE::Component::Client::TCP)
 BuildRequires:  perl(Test::Compile)
 BuildRequires:  perl(Test::HTTP::Server::Simple)
+BuildRequires:  perl(Test::More)
+BuildRequires:  perl(UNIVERSAL::require)
 BuildRequires:  perl(URI::Escape)
 BuildRequires:  fusioninventory-agent >= 2.2.0
-%if 0%{?fedora} >= 14
-BuildRequires:  perl(HTTP::Server::Simple::Authen) perl(CGI)
-%endif

 Requires:       fusioninventory-agent >= 2.2.0
-Requires:       perl(Digest::SHA)
 Requires:       perl(POE::Component::Client::Ping)
+Requires:       perl(POE::Component::Client::TCP)
 Requires:       perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version))

 # RPM 4.8
@@ -88,6 +102,9 @@


 %changelog
+* Wed May 30 2012 Remi Collet <remi> - 2.0.0-3
+- fix BuildRequires/Requires from review #812587
+
 * Fri May 11 2012 Remi Collet <remi> - 2.0.0-2
 - filter private provides/requires


> FIX: Build- and run-require `perl(POE::Component::Client::TCP)'
> (lib/FusionInventory/Agent/Task/Deploy/P2P.pm:9).
+BuildRequires:  perl(POE::Component::Client::TCP)
+Requires:       perl(POE::Component::Client::TCP)
Ok.

> FIX: Build-require `perl(File::Which)' (Makefile.PL:13).
+BuildRequires:  perl(File::Which)
Ok.

> TODO: Build-require `perl(base)' (lib/FusionInventory/Agent/Task/Deploy.pm:8).
+BuildRequires:  perl(base)
Ok.

> FIX: Build-require `perl(LWP)' (lib/FusionInventory/Agent/Task/Deploy.pm:11).
+BuildRequires:  perl(LWP)
Ok.

> FIX: Build-require `perl(Compress::Zlib)' (lib/FusionInventory/Agent/Task/Deploy/Datastore/WorkDir.pm:7).
+BuildRequires:  perl(Compress::Zlib)
Ok.

> TODO: Build-require `perl(Exporter)' (lib/FusionInventory/Agent/Task/Deploy/DiskFree.pm:5).
+BuildRequires:  perl(Exporter)
Ok.

> FIX: Build-require `perl(UNIVERSAL::require)' (lib/FusionInventory/Agent/Task/Deploy/DiskFree.pm:10).
+BuildRequires:  perl(UNIVERSAL::require)
Ok.

> FIX: Build-require `perl(File::Spec)' (lib/FusionInventory/Agent/Task/Deploy/Datastore.pm:8).
+BuildRequires:  perl(File::Spec)
Ok.

> FIX: Build-require `perl(HTTP::Request::Common)' (lib/FusionInventory/Agent/Task/Deploy/P2P.pm:7).
+BuildRequires:  perl(HTTP::Request::Common)
Ok.

> FIX: Build-require `perl(Net::IP)' (lib/FusionInventory/Agent/Task/Deploy/P2P.pm:8).
+BuildRequires:  perl(Net::IP)
Ok.

> FIX: Build-require `perl(POE)' (lib/FusionInventory/Agent/Task/Deploy/P2P.pm:9).
+BuildRequires:  perl(POE)
Ok.

> FIX: Build-require `perl(Cwd)'
> (lib/FusionInventory/Agent/Task/Deploy/ActionProcessor.pm:6).
+BuildRequires:  perl(Cwd)
Ok.

> FIX: Build-require `perl(HTTP::Request)'
> (lib/FusionInventory/Agent/Task/Deploy/File.pm:10).
+BuildRequires:  perl(HTTP::Request)
Ok.

> FIX: Build-require `perl(Test::More)' (t/06spelling.t:6).
+BuildRequires:  perl(Test::More)
Ok.

> FIX: Build-require `perl(HTTP::Server::Simple::CGI)' (t/server.t:7).
+BuildRequires:  perl(HTTP::Server::Simple::CGI)

> FIX: Build-require `perl(Data::Dumper)' (t/server.t:12).
+BuildRequires:  perl(Data::Dumper)
Ok.

> > TODO: You can remove the BuildRoot tag and it's cleaning if your are not
> > intent to build this package in EPEL-5. The actions are implicit in newer
> > distributions.
> > TODO: Remove the %defattr from %files section. This is implicit in all
> > distributions.
> (I still prefer to keep old stuff, builroot, defattr, clean, ... until RHEL-5
> EOL)
Ok.

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

> TODO: Do not run-require `perl(Digest::SHA)' explicitly. It's auto-detected.
> FIX: Run-require `perl(POE::Component::Client::TCP)'
> (lib/FusionInventory/Agent/Task/Deploy/P2P.pm:9).
$ rpm -q --requires -p ../RPMS/noarch/perl-FusionInventory-Agent-Task-Deploy-2.0.0-3.fc18.noarch.rpm |sort |uniq -c                             1 fusioninventory-agent >= 2.2.0
      1 perl(Archive::Extract)
      1 perl(base)
      1 perl(Compress::Zlib)
      1 perl(Cwd)
      1 perl(Digest::SHA)
      1 perl(English)
      1 perl(Exporter)
      1 perl(File::Basename)
      1 perl(File::Copy::Recursive)
      1 perl(File::Glob)
      1 perl(File::Path)
      1 perl(File::Spec)
      1 perl(HTTP::Request)
      1 perl(HTTP::Request::Common)
      1 perl(JSON)
      1 perl(LWP)
      1 perl(:MODULE_COMPAT_5.14.2)
      1 perl(Net::IP)
      1 perl(POE)
      1 perl(POE::Component::Client::Ping)
      1 perl(POE::Component::Client::TCP)
      1 perl(strict)
      1 perl(UNIVERSAL::require)
      1 perl(URI::Escape)
      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.

$ resolvedeps rawhide ../RPMS/noarch/perl-FusionInventory-Agent-Task-Deploy-2.0.0-3.fc18.noarch.rpm 
Binary dependencies resolvable. Ok.

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

Package is in line with Fedora and Perl packaging guidelines.


Resolution: Package APPROVED.

Comment 7 Petr Pisar 2012-05-31 08:21:24 UTC

(In reply to comment #5)
> Petr, can you please give me how to you detect BR ? Have a script or does
> (find ... | /usr/lib/rpm/perl.req) could be enough ?
> 
I have no script. I just study META.yml and then "grep -HnrE '\<(use|require|parent|base|ISA)\>'" Makefile.PL, lib, t and inc. And of course I check the context of the used module. It can turn out that the module is just eval-ed optional functionality or something like that.

The /usr/lib/rpm/perl.req is not perfect too. But studying it's results is also helpful (I check the `rpm -q --requires -p' output).

Comment 8 Remi Collet 2012-05-31 12:33:09 UTC

Thanks for your time.


New Package SCM Request
=======================
Package Name: perl-FusionInventory-Agent-Task-Deploy
Short Description: Software deployment support for FusionInventory Agent
Owners: remi
Branches: 
InitialCC: perl-sig

Comment 9 Gwyn Ciesla 2012-05-31 13:14:50 UTC

Git done (by process-git-requests).