Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.

Bug 217275

Summary: Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client
Product: [Fedora] Fedora Reporter: Remi Collet <fedora>
Component: Package ReviewAssignee: Patrice Dumas <pertusus>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: pertusus
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2006-12-02 08:06:32 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:    
Bug Blocks: 163779    

Description Remi Collet 2006-11-26 10:34:38 UTC
Spec URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client.spec
SRPM URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client-1.0-0.1.RC3.fc7.src.rpm
Mock Log: http://remi.collet.free.fr/rpms/extras/ocsinventory-client-build.log
Description: 
Open Computer and Software Inventory Next Generation is an application
designed to help a network or system administrator keep track of computer
configuration and software installed on the network.

It also allows deploying softwares, commands or files on Windows and
Linux client computers.

ocsinventory-client provide the client agent for Linux.

---
Patch used have be send upstream.

rpmlint warning due to DEVIDE_ID generation (using date):
W: ocsinventory-client percent-in-%post

To test simply run "ocsinventory-client.pl -xml" and read /var/log/ocsinventory-client/xxx.ocs

I also work on ocsinventory-*-server (lot of more job)

I don't know if i should create a separate perl-Ocsinventory-Agent as this extension is only use by this client.

Comment 1 Patrice Dumas 2006-11-26 12:04:34 UTC
* I think that in the comment explaining what ocstag and ocsserver
I think that you should put a reference to the README. Something along

# Can, optionaly, be defined at build time (see README)

* instead of dos2unix, you can use
sed -i 's/\r//'

And similarly you can use sed instead perl to do the simple 
substitution, like

sed -i -e 's|PATH_TO_LOG_DIRECTORY|%{_localstatedir}/log/%{name}|'
logrotate.ocsinventory-client

* I think that the README.fedora should be split in a README.fedora
and README.fedora.fr, tagged like
%lang(fr) %doc README.fedora.fr

Comment 2 Remi Collet 2006-11-26 13:26:50 UTC
Patrice, Thanks for the comments

Spec URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client.spec
SRPM URL:
http://remi.collet.free.fr/rpms/extras/ocsinventory-client-1.0-0.2.RC3.fc7.src.rpm

%changelog
* Sun Nov 26 2006 Remi Collet <Fedora> 1.0-0.2.RC3
- replace perl and dos2unix by sed
- split README.fedora


Comment 3 Patrice Dumas 2006-11-26 22:34:38 UTC
* %{buildroot} is used everywhere, so I guess $RPM_OPT_FLAGS 
should be changed to %{optflags}. 

* there are some unowned directory issues, %{_sysconfdir}/logrotate.d/
and %{_sysconfdir}/cron.daily/ (owned by logrotate and crontabs). There
was a thread about that issue on fedora-devel-list, I have no particular
opinion on what is the right solution.

* from looking in the script, seems like there is a missing 
Requires: perl(HTTP::Request)



Comment 4 Remi Collet 2006-11-27 06:34:06 UTC
> there are some unowned directory issues,

Yes. I don't have real opinion too.
logrotate and cron are installed on must system. I don't like to add an require
for them as alternatives exists and "virtual-requires" not.

Spec URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client.spec
SRPM URL:
http://remi.collet.free.fr/rpms/extras/ocsinventory-client-1.0-0.3.RC3.fc7.src.rpm

* Mon Nov 27 2006 Remi Collet <Fedora> 1.0-0.3.RC3
- $RPM_OPT_FLAGS changed to %%{optflags}. 
- add not detected Requires perl(HTTP::Request)

P.S. I will be away for the next 2 days...

Comment 5 Remi Collet 2006-11-27 08:04:51 UTC
I will also change 
  BuildRequires: dmidecode
  Requires: dmidecode
to :
  BuildRequires: %{_sbindir}/dmidecode
  Requires:  %{_sbindir}/dmidecode

To allow build on older version (RHEL and FC) as dmidecode is provided by
kernel-utils.

Comment 6 Patrice Dumas 2006-11-27 14:30:44 UTC
(In reply to comment #4)
> > there are some unowned directory issues,
> 
> Yes. I don't have real opinion too.
> logrotate and cron are installed on must system. I don't like to add an require
> for them as alternatives exists and "virtual-requires" not.

In that case you can add a Requires on the directories, or have
ocsinventory-client own those directories, until they get owned by
the filesystem package.

(In reply to comment #5)
> I will also change 
>   BuildRequires: dmidecode
>   Requires: dmidecode
> to :
>   BuildRequires: %{_sbindir}/dmidecode
>   Requires:  %{_sbindir}/dmidecode
> 
> To allow build on older version (RHEL and FC) as dmidecode is provided by
> kernel-utils.

Fine for me.


Comment 7 Remi Collet 2006-11-28 18:26:20 UTC
> In that case you can add a Requires on the directories, or have
> ocsinventory-client own those directories, until they get owned by
> the filesystem package.

Is this a MUST for the review ?
I look at some core package and don't see anything...

Some improvement to have ocsinventory-client build and work on RHEL and older FC
(i really need this at work).

Spec URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client.spec
SRPM URL:
http://remi.collet.free.fr/rpms/extras/ocsinventory-client-1.0-0.4.RC3.fc7.src.rpm

* Tue Nov 28 2006 Remi Collet <Fedora> 1.0-0.4.RC3
- requires %%{_sbindir}/dmidecode (kernel-utils in FC3) rather than dmidecode 
- requires perl(:MODULE_COMPAT) only on Fedora (not provided on RHEL3)
- patch improved (Fedora is RPM based)


Comment 8 Patrice Dumas 2006-11-28 21:13:59 UTC
(In reply to comment #7)
> > In that case you can add a Requires on the directories, or have
> > ocsinventory-client own those directories, until they get owned by
> > the filesystem package.
> 
> Is this a MUST for the review ?

Yes. I make exception for /usr/share/icons/hicolor. 

In the packaging guidelines it is at
http://fedoraproject.org/wiki/Packaging/Guidelines#head-a5931a7372c4a00065713430984fa5875513e6d4

And in that case I think it is relevant, since it materializes
whether you want to have a real dependency on logrotate/cron
or make logrotate/cron optional - which means that the package
owns the directory.

> I look at some core package and don't see anything...

Most core packages haven't gone through the guidelines and lacks the
fedora extras packages quality.

> Some improvement to have ocsinventory-client build and work on RHEL and older FC
> (i really need this at work).

With EPEL, this is indeed something acceptable or even encouraged. 
Besides in that case it doesn't really hurt legibility.

> Spec URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client.spec
> SRPM URL:
> http://remi.collet.free.fr/rpms/extras/ocsinventory-client-1.0-0.4.RC3.fc7.src.rpm
> 
> * Tue Nov 28 2006 Remi Collet <Fedora> 1.0-0.4.RC3
> - requires %%{_sbindir}/dmidecode (kernel-utils in FC3) rather than dmidecode 
> - requires perl(:MODULE_COMPAT) only on Fedora (not provided on RHEL3)
> - patch improved (Fedora is RPM based)

Apart from directory owning, seems right. Notice that I didn't 
assign the bug to myself, so a reviewer accepting unowned 
directories for those directories can step up.

Comment 9 Remi Collet 2006-11-29 18:34:28 UTC
Spec URL: http://remi.collet.free.fr/rpms/extras/ocsinventory-client.spec
SRPM URL:
http://remi.collet.free.fr/rpms/extras/ocsinventory-client-1.0-0.5.RC3.fc7.src.rpm

%changelog
* Wed Nov 29 2006 Remi Collet <Fedora> 1.0-0.5.RC3
- Requires %%{_sysconfdir}/logrotate.d and %%{_sysconfdir}/cron.daily
- define perl_vendorlib on non-fedora (for RHEL3)



Comment 10 Patrice Dumas 2006-11-29 22:09:00 UTC
* rpmlint gives an ignorable warning, % in post is used in date
  invocation:
W: ocsinventory-client percent-in-%post
* the tarball name cannot be used, the name chosen is relevant,
  and used in other distros.
* free software, license included
* follow packaging guidelines
* source match upstream:
34edd057f1937245d06c3515c0ff50ad  OCSNG_LINUX_AGENT_1.0RC3.tar.gz
* sane provides:
Provides: config(ocsinventory-client) = 1.0-0.5.RC3 perl(Ocsinventory::Agent)
perl(Ocsinventory::Agent::Common) perl(Ocsinventory::Agent::Option::Download)
perl(Ocsinventory::Agent::Option::Ipdiscover)
perl(Ocsinventory::Agent::Option::Update)
* buildrequires right
* %files section right


APPROVED