Bug 331531

The version/release of the package isn't right, have a look at
http://fedoraproject.org/wiki/Packaging/NamingGuidelines#head-cfd71146dbb6f00cec9fe3623ea619f843394837

Regarding the name, why not simply ltspswapd?

For the source url, it is almost right, but please have 
a look at 
http://fedoraproject.org/wiki/Packaging/SourceURL
for an even better style.

I don't think that %{_localstatedir}/opt/ is the right var directory 
for the swap files. Maybe %{_localstatedir}/lib/ or %{_localstatedir}/spool
(with appropriate subdirectories), but /opt in general should always
be left empty by rpm installations.

You should use the -p switch to install, to keep timestamps, like
install -p -m 0644 %{SOURCE1} ${RPM_BUILD_ROOT}/%{_sysconfdir}/sysconfig/ltspswapd

The Source3 file is a bit strange. Isn't it a remnant of something
else?

Suggestion:
the %{_*dir} marcos already have a leading /, so it is not needed
in ${RPM_BUILD_ROOT}/%{_sysconfdir}/sysconfig/, or in 
/%{_sysconfdir}/sysconfig/ltspswapd

Also ltspswapd.init lacks a lsb init header.

Updated based on your comments

Spec URL:
http://k12linux.mesd.k12.or.us/K12LTSP/source/ltspswapd/ltspswapd.spec
SPRM URL:
http://k12linux.mesd.k12.or.us/K12LTSP/source/ltspswapd/ltspswapd-0-0.1.20071016cvs.src.rpm

You should change the LTSPSWAPD_FILEPATH in ltspswapd.sysinit,
something along
sed -e 's;LTSPSWAPD_FILEPATH=.*;LTSPSWAPD_FILEPATH=%{swapdir};' %{SOURCE1} >
${RPM_BUILD_ROOT}%{_sysconfdir}/sysconfig/ltspswapd

(and if you are pedantic about timestamps, like I do)
touch -r %{SOURCE1} ${RPM_BUILD_ROOT}%{_sysconfdir}/sysconfig/ltspswapd

No need for /sbin/service in Requires(post)

In the cvs checkout command there should be a switch that makes
sure that the cvs version is the same one than the one in the rpm.

%{_localstatedir}/lib/ltsp/ is unowned.

Does ltspswapd really requires nbd? It looks like it has its own
nbd-server implementation.

Now that there is a central configuration for ltsp5, I think
that that package should use it too. It means that the
config file won't be there is ltsp-server isn't installed, but 
this should never happen.

Spec URL:
http://k12linux.mesd.k12.or.us/K12LTSP/source/ltspswapd/ltspswapd.spec
SPRM URL:
http://k12linux.mesd.k12.or.us/K12LTSP/source/ltspswapd/ltspswapd-0-0.3.20071016cvs.fc8.src.rpm

- source /etc/ltsp5.conf to find swap directory
- require /etc/ltsp5.conf
- move /var/opt/ltsp5 to /var/lib/ltsp5

I think that it is wrong to require /etc/ltsp.conf. A user
may want to use ltspswapd alone. Therefore, I think that 
%{_sysconfdir}/sysconfig/ltspswapd should still used, but only 
if there is no /etc/ltsp5.conf. And it should be said in a
comment in %{_sysconfdir}/sysconfig/ltspswapd.

The corresponding code in init file would be

# Source ltspswapd configuration.
[ -f /etc/sysconfig/ltspswapd ] && . /etc/sysconfig/ltspswapd
[ -f /etc/ltsp5.conf ] && . /etc/ltsp5.conf

A dot is missing at the end of the %description

a -r or similar for the cvs command is also still missing.

Only a suggestion, but in general the %define are put at
the very beginning of the spec file. 

Spec URL:
http://k12linux.mesd.k12.or.us/K12LTSP/source/ltspswapd/ltspswapd.spec
SRPM URL:
http://k12linux.mesd.k12.or.us/K12LTSP/source/ltspswapd/ltspswapd-0-0.4.20071019cvs.fc8.src.rpm

- source /etc/sysconfig/ltspswapd, then /etc/ltsp5.conf in initscript
- read-only checkout from CVS
- fix description, move %%define to top

(In reply to comment #8)

> - read-only checkout from CVS

It is not what I asked for, I asked for the -r option
of checkout:
        -r rev  Check out revision or tag. (implies -P) (is sticky)
You can also use -D.

-added "-D <date>" as per comment #9
-updated config file location to match changes in ltsp-server

Spec URL:
http://k12linux.mesd.k12.or.us/K12LTSP/source/ltspswapd/ltspswapd.spec
SRPM URL:
http://k12linux.mesd.k12.or.us/K12LTSP/source/ltspswapd/ltspswapd-0-0.5.20071023cvs.fc8.src.rpm

You should remove the dependency on  /etc/ltsp5/ltsp.conf such 
that the package may be installed on its own. And in my
opinion the comment in sysconfig/ltspswapd should be along

# 
# the LTSP_SWAP_DIR variable is preferably set in /etc/ltsp5/ltsp.conf 
# to have a central config file. If you don't want to use /etc/ltsp5/ltsp.conf 
# you can also set this variable in this file
#

%{_localstatedir}/lib is already owned by the filesystem package.

You could also add LTSP_SWAP_DIR in a comment in sysconfig/ltspswapd,
like

# LTSP_SWAP_DIR=/var/lib/ltsp5/swapfiles/

The following lines are not useful if the comment is not there:
sed -e 's;LTSPSWAPD_FILEPATH=.*;LTSPSWAPD_FILEPATH=%{swapdir};' %{SOURCE1} >
${RPM_BUILD_ROOT}%{_sysconfdir}/sysconfig/ltspswapd
touch -r %{SOURCE1} ${RPM_BUILD_ROOT}%{_sysconfdir}/sysconfig/ltspswapd

And if the comment is there, LTSPSWAPD_FILEPATH should be 
changed to LTSP_SWAP_DIR

nbdswapd is provided in the upstream ltsp repository, which builds into
sub-packages ltsp-client and ltsp-server.  We should verify that the upstream
nbdswapd suits our needs, then cancel this review.

OK, it is pretty clear that we will use upstream's script integrated into
ltsp-server instead.  Closing this as duplicate against ltsp-server.


*** This bug has been marked as a duplicate of 331731 ***