[Bug 217275] New: Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=217275

Summary: Review Request: ocsinventory-client - Open Computer and
                    Software Inventory Next Generation client
           Product: Fedora Extras
           Version: devel
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: normal
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]

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.s...
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.

[Bug 217275] Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=217275

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

------- Additional Comments From [e-mail]  2006-11-26 07:04 EST -------
* 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

[Bug 217275] Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=217275

------- Additional Comments From [e-mail]  2006-11-26 08:26 EST -------
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.s...

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

[Bug 217275] Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=217275

------- Additional Comments From [e-mail]  2006-11-26 17:34 EST -------
* %{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)

[Bug 217275] Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=217275

------- Additional Comments From [e-mail]  2006-11-27 01:34 EST -------
> 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.s...

* Mon Nov 27 2006 Remi Collet  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...

[Bug 217275] Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=217275

------- Additional Comments From [e-mail]  2006-11-27 03:04 EST -------
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.

[Bug 217275] Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=217275

------- Additional Comments From [e-mail]  2006-11-27 09:30 EST -------
(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.

[Bug 217275] Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=217275

------- Additional Comments From [e-mail]  2006-11-28 13:26 EST -------
> 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.s...

* Tue Nov 28 2006 Remi Collet  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)

[Bug 217275] Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=217275

------- Additional Comments From [e-mail]  2006-11-28 16:13 EST -------
(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-a5931a7372c4a0006571...

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.s...
> Tue Nov 28 2006 Remi Collet  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.

[Bug 217275] Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=217275

------- Additional Comments From [e-mail]  2006-11-29 13:34 EST -------
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.s...

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

[Bug 217275] Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=217275

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|[e-mail]    |[e-mail]
OtherBugsDependingO|163776                      |163779
              nThis|                            |

------- Additional Comments From [e-mail]  2006-11-29 17:09 EST -------
* 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

[Bug 217275] Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: ocsinventory-client - Open Computer and Software Inventory Next Generation client

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=217275

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE