Bug 435593

Patrice, i put you in CC as you have reviewed ocsinventory-client which is
obsoleted by this new package (#217275).

I used this new agent in "production" for more than 1 year and I like to push
this "official" OCS new client in Fedora and EPEL.

It will be great to have it on the repo at the same time than official
annoucement on OCS web site.

Regards.

Missing BR perl(Digest::MD5) and perl(File::Temp). 

You should not use fedora in the README file names, but instead
something that can be in any derived package, so something like
distribution (and in my opinion it is better to avoid writing 
Fedora as much as possible because of potential trademark issues).

Obsoletes: ocsinventory-client <= 1.01
is right, but
Provides:  ocsinventory-client = %{version}-%{release}
is wrong since version is 0.0.8.2, so it provides a version older 
than the obsoleted version. I am not completly sure what to do in 
that case. I see 2 possibilities
* have 
Provides:  ocsinventory-client = 1.02
# use the following line when version is strictly above 1.01
# ocsinventory-client = %{version}-%{release}

* no uncommented Provides but the following comment:
# as soon as version is strictly above 1.01 uncomment the following line 
# ocsinventory-client = %{version}-%{release}



The cvs -z3 co command should use a date or a tag such that the 
same code is checked out in the future.

Shouldn't postinst.pl be installed under _sbindir (with a better
name)?

Yes i forget postinst.pl

I think i should simply remove it as .cfg is provided and the use of this script
may conflict with rpm stuff (mainly the cron.d != cron.hourly).

About ocsinventory-client, i'm thinking of using epoch (i must check if this is ok)
ocsinventory-client = 1:%{version}-%{release}

I haven't add perl(Digest::MD5) and perl(File::Temp) because provided by perl
main packages, but i will add it (in case of a package split).

Thank's for the quick feedback.

(In reply to comment #4)
> Yes i forget postinst.pl
> 
> I think i should simply remove it as .cfg is provided and the use of this script
> may conflict with rpm stuff (mainly the cron.d != cron.hourly).

Indeed. Or patch it if it is worth it. 

> About ocsinventory-client, i'm thinking of using epoch (i must check if this
is ok)
> ocsinventory-client = 1:%{version}-%{release}

It is possible, but it would be better, in my opinion to avoid
epochs. 

Spec URL: http://remi.fedorapeople.org/ocsinventory-agent.spec
SRPM URL:
http://remi.fedorapeople.org/ocsinventory-agent-0.0.8.2-0.2.20080302.fc8.src.rpm

- Fix CVS command in comment
- Fix BR
- Provides:  ocsinventory-client = 1.02
- rename to README.RPM (as it provides information for RPM customisation)

It is a bit strange to use svn in the tarball name when it is a
cvs snapshot.

Otherwise I checked the source, it is the same.

In the description, I believe it should be 'provides':

%{name} provides the client for Linux (Unified Unix Agent).

* rpmlint is silent
* follow packaging guidelines
* sane provides and requires
* %files section right
* free software, license included


APPROVED. But please consider renaming svn to cvs.

Also in the first comment, it should be README.RPM instead of
README.fedora.

Thanks for the review.


New Package CVS Request
=======================
Package Name: ocsinventory-agent
Short Description: Open Computer and Software Inventory Next Generation client
Owners: remi
Branches: F-8, F-7, EL-5, EL-4
InitialCC: 
Cvsextras Commits:yes

After some thinking and testing, I think there is something
wrong with enabling the cron job automatically. 
A user may want to use the software without having it run 
automatically.

You can either isolate the cron file in a subpackage, or have
a file in /etc/sysconfig sourced in the cron script with
a variable which value has to be changed for ocs to be run
in te cron script. In that case you can also do a subpackage 
with a /etc/sysconfig/ocs... file with the variable set.

In fact i work with upstream to merge the old "client" config file in the main
new .cfg and to avoid having 2 config files.

One proposal was to have a "mode" option in the config which can be set to :
none, hourly, daily, weekly or service. But this is wasn't accepted.

I will propose, for the RPM to enable the cron when "server" is configured.

So after installation, mode is "local", so cron is installed but deactivated.

Also, add comments in the config file.

See : 
Spec URL: http://remi.fedorapeople.org/ocsinventory-agent.spec
SRPM URL:
http://remi.fedorapeople.org/ocsinventory-agent-0.0.8.2-0.3.20080302.fc8.src.rpm

What about this "simple" solution ?

clearning cvs flag here until this is re-approved. 

I think that it makes sense to have a different config file for
the cron file and for ocsinventory-agent itself. I will try to do 
a patch tomorrow that does a file for ocs cron file and set 
the file content according to %ocsserver.

OK.

Probably we can't take the old launcher in ocsinventory-client, change path to 
/etc/sysconfig/ocsinventory-agent and add a OCSMODE option (none, cron,
daemon...). In a future release we'll can also add an daemon mode (not enough
mature in this upstream release)

Having OCSPAUSE (wait parameter) as an option (back) will be usefull too.

I'm also thinking a way to give multiple servers in this config (usefull in some
case to send inventory to more than 1 server, and new tree in /var/lib allow
it). Well in this case the provided .cfg will become unused, except for manual
inventory.

I will be AFK for the next 2 days, I will work on this Wednesday evening.

Regards

Spec URL: http://remi.fedorapeople.org/ocsinventory-agent.spec
SRPM URL:
http://remi.fedorapeople.org/ocsinventory-agent-0.0.8.2-0.4.20080305.fc8.src.rpm
Mock log: http://remi.fedorapeople.org/ocsinventory-agent-build.log

Changes :
- update to 2008-03-05
- add /etc/sysconfig/ocsinventory-agent config file for cron job

I think that the local and server config in the cfg file should not
be overriden by the sysconfig file in the default case. But it 
could be possible to override them, especially if you have multiple
config in the sysconfig file. 

I propose patches for the spec file and the cron script to do that. 

In the spec diff, I also added a sed substitution on the cron script
to use the rpm macros (and added a -p to an install).

Created attachment 297152 [details]
set the local and server options only if they are set

also this is a bash script, it uses a lot of bashisms.

Created attachment 297153 [details]
set server or local in the .cfg file, use rpm macros for cron script

> bashisms
I hope this is not copyrighted.. I love this term ;)

Spec URL: http://remi.fedorapeople.org/ocsinventory-agent.spec
SRPM URL:
http://remi.fedorapeople.org/ocsinventory-agent-0.0.8.2-0.5.20080305.fc8.src.rpm

- patches included
- remove "local" from .cfg (because priority is higher than "server" option from
command line)

Actually, running from command line try to contact "hardcoded" server.
I will work with upstream to have command line option "server" working if
"local" option in config file.

You should remove /home/extras/SOURCES/ in the generate tarball
comment.

Any reapproved.

New Package CVS Request
=======================
Package Name: ocsinventory-agent
Short Description: Open Computer and Software Inventory Next Generation client
Owners: remi
Branches: F-8, F-7, EL-5, EL-4
InitialCC: 
Cvsextras Commits:yes

cvs done.