Bug 230275

After a first look on your spec file:
------

* The packager tag (even commented) SHOULDN'T be in spec file.

* Buildroot tag SHOULD be
http://fedoraproject.org/wiki/Packaging/Guidelines?highlight=%28Packaging%29#head-f196e7b2477c2f5dd97ef64e8eacddfb517f1aa1

* Some of BuildRequires are redundant as they already installed from development   
  tools and based installed packages (such as gcc, automake, autoconf, patch,  
  libtoll and gcc-c++) all can be drop.

* Requies: i don't really sure that kernel version can be added as requires 
  especially the kernel 2.6 is already included as default kernel version for 
  FC6 and more.

* vendor tag SHOULDN'T be use in spec file.

* Group tag of %package -n %{lib_name} SHOULD be System Environment/Librairies
  insteas of System Environment/Daemons.

* Summary tag SHOULD be revisited.
  (such as %{name} library)

* BR and Requires for -libs and -libs-devel packages: same things as above.

* Description -n %{lib_name} SHOULD be revisited
 (such as, %{lib_name} contains librairies files for %{name})

* Summary tag of %package -n %{lib_name}-devel SHOULD be revisited.
  (such as development librairies for %{lib_name}.

* Group tag of %package -n %{lib_name}-devel SHOULD be
  "Development/Librairies instead" instead of "System Environment/Daemons"

* url tag in -devel isn't necessary can be remove.

* -devel Requires is missing and it SHOULDN't be.
  SHOULD be present : Requires:   %{lib_name} = %{version}-%{release}

* %description of -devel package SHOULDN be something like :
  "The %{name}-devel package contains libraries and header files for
  developing applications that use %{name}." instead.

----
%prep
----

* The use of iconv can be improve by using pushd to enter and popd to exit  
  directories. This fact can reduce the complexity of the iconv command  
 (including option and redirection command).

Here is a way that you can use:
-----------------------------
pushd bin
for i in varnishlog varnishtop varnishd \
  varnishstat varnishhist varnishncsa ; do
   iconv -f iso-8859-2 -t utf-8 $i/$i.1 > $i/$i.1.utf8
   rm -f $i/$i.1 && mv $i/$i.1.utf8 $i/$i.1
done
popd
iconv -f iso-8859-2 -t utf-8 man/vcl.7 > man/vcl.7.utf8
rm -f man/vcl.7 && mv man/vcl.7.utf8 man/vcl.7
-----------------------------

----
%build
----

*  autogen.sh script SHOULDN'T be use if the configure file is present and work.

* From make commande, using  %{?_smp_mflags} can improve the procedure and 
  doesn't affect no-smp.

----
%install
----

* the %{makeinstall} should not be used if the "make install
DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p""
  functions and it is the case.
see:
http://fedoraproject.org/wiki/Packaging/Guidelines?highlight=%28Packaging%29#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002

* From strip command, you SHOULD comment the use of and add -p option
  to keep timstamps to files.
----
The use of:
----
mkdir -p %{buildroot}%{_docdir}/%{name}-%{version}

and

%{__install} -m 0644 INSTALL %{buildroot}%{_docdir}/%{name}-%{version}/INSTALL
%{__install} -m 0644 LICENSE %{buildroot}%{_docdir}/%{name}-%{version}/LICENSE
%{__install} -m 0644 README %{buildroot}%{_docdir}/%{name}-%{version}/README
%{__install} -m 0644 ChangeLog %{buildroot}%{_docdir}/%{name}-%{version}/ChangeLog
%{__install} -m 0644 redhat/README.redhat
%{buildroot}%{_docdir}/%{name}-%{version}/README.redhat

and

%doc %{_docdir}/%{name}-%{version}/INSTALL
%doc %{_docdir}/%{name}-%{version}/LICENSE
%doc %{_docdir}/%{name}-%{version}/README
%doc %{_docdir}/%{name}-%{version}/README.redhat
%doc %{_docdir}/%{name}-%{version}/ChangeLog

----
SHOULDN'T be used.
Docs SHOULD be add in %doc in %files section such as "%Doc README COPYING
ChangeLog".
It's automaticaly installed in %{_docdir}/%{name}-%{version}.

* You don't need to explicit write each files in %files section.
  use %{_sbindir}/ instead.
  use %{_mandir}/man1/*.1.gz instead.
  use %{_mandir}/man7/*.7.gz instead.
  use %{_libdir}/*.so.* instead.
  use %{_libdir}/*.so instead.

* "*.la" files SHOULDN'T be include in -devel subpackage.
 if for some reason its really require ( for full work), You should
 add -statics subpackage.

Can be remove by using:
find $RPM_BUILD_ROOT/%{_libdir}/ -name '*.la' -exec rm -f {} ';' 


* Scriptlets seems to miss some options and arguments.
  see :
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets?highlight=%28script%29

* "/etc" can be replace by its macros %{_sysconfdir}.

----
%changelog
----

please add a empty line between each of them.

-----

reflexion : use should think about the naming of -libs and -libs-devel subpackag
            by renaming subpackage as follow, lib%{name} and lib%{name}-devel.

typo: it's not use should think about...
      it's, you should think about...

typo1: Docs should be add... -> Docs should be added...

typo2: no-smp -> no-smp-machine

typo3: The use of iconv can be improve... -> The use of iconv can be improved...

The short story: Fixed according to comments below. New spec/src.rpm :

http://users.linpro.no/ingvar/varnish/fedora-extras-commit/varnish.spec
http://users.linpro.no/ingvar/varnish/fedora-extras-commit/varnish-1.0.3-4.src.rpm

Comments and answers:

> * The packager tag (even commented) SHOULDN'T be in spec file.

ok, removed
 
> * Buildroot tag SHOULD be
>
http://fedoraproject.org/wiki/Packaging/Guidelines?highlight=%28Packaging%29#head-f196e7b2477c2f5dd97ef64e8eacddfb517f1aa1

ok, changed to %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
 
> * Some of BuildRequires are redundant as they already installed from
>   development tools and based installed packages (such as gcc,
>   automake, autoconf, patch, libtoll and gcc-c++) all can be drop.

I guess you mean libtool. ok, fixed.

> * Requies: i don't really sure that kernel version can be added as
>   requires especially the kernel 2.6 is already included as default
>   kernel version for FC6 and more.

Kernel version can be added as requires, a least technically, rpm and
rpmbuild allow that. The package should also build on other
distributions. There are people that have tried to build varnish from
a source rpm on RHEL3. The daemon will build, but not work on kernel
<2.6.
 
> * vendor tag SHOULDN'T be use in spec file.

ok, removed
 
> * Group tag of %package -n %{lib_name} SHOULD be System
>   Environment/Librairies insteas of System Environment/Daemons.

ok, fixed.

> * Summary tag SHOULD be revisited.
>   (such as %{name} library)

ok, fixed

> * BR and Requires for -libs and -libs-devel packages: same things as
>   above.

You need to specify buildroot for the subpackages as well? ok, fixed.
 
> * Description -n %{lib_name} SHOULD be revisited (such as,
>   %{lib_name} contains librairies files for %{name})

It's there, but I have trimmed it a bit.

> * Summary tag of %package -n %{lib_name}-devel SHOULD be revisited.
>   (such as development librairies for %{lib_name}.

It's there, but I trimmed it a bit too.
 
> * Group tag of %package -n %{lib_name}-devel SHOULD be
>   "Development/Librairies instead" instead of "System
>   Environment/Daemons"

ok, fixed

> * url tag in -devel isn't necessary can be remove.

ok, removed

> * -devel Requires is missing and it SHOULDN't be.
>   SHOULD be present : Requires:   %{lib_name} = %{version}-%{release}

Eh? At the moment, it's like this:
Requires: ncurses kernel >= 2.6.0  %{lib_name} = %version-%{release}
 
> * %description of -devel package SHOULDN be something like :
>   "The %{name}-devel package contains libraries and header files for
>   developing applications that use %{name}." instead.

Well, there is no varnish-devel. Just a varnish-libs-devel, that
contains .a, .la and .so files. So my new description "Development
libraries for %{name}" should be enough, I guess.
 

> %prep
> * The use of iconv can be improve by using pushd to enter and popd to exit  
>   directories. This fact can reduce the complexity of the iconv command  
>  (including option and redirection command).
> 
> Here is a way that you can use:
> -----------------------------
> pushd bin
> for i in varnishlog varnishtop varnishd \
>   varnishstat varnishhist varnishncsa ; do
>    iconv -f iso-8859-2 -t utf-8 $i/$i.1 > $i/$i.1.utf8
>    rm -f $i/$i.1 && mv $i/$i.1.utf8 $i/$i.1
> done
> popd
> iconv -f iso-8859-2 -t utf-8 man/vcl.7 > man/vcl.7.utf8
> rm -f man/vcl.7 && mv man/vcl.7.utf8 man/vcl.7
> -----------------------------

I may argue that this is less complex, but ok, fixed.
 
> %build
> *  autogen.sh script SHOULDN'T be use if the configure file is
>    present and work.

Ah, that's a leftover from working with the svn version. Thanks, fixed.
 
> * From make commande, using %{?_smp_mflags} can improve the
>   procedure and doesn't affect no-smp.

I used to have this, but it killed the build my home computer (maybe
an unstable cpu?). The sources are not large, so compilation time is
not an issue, I think. Now, -jN seems to work on other smp machines,
so ok, added.
 
> %install
> * the %{makeinstall} should not be used if the "make install
> DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p""
>   functions and it is the case.
> see:
>
http://fedoraproject.org/wiki/Packaging/Guidelines?highlight=%28Packaging%29#head-fcaf3e6fcbd51194a5d0dbcfbdd2fcb7791dd002

ok, fixed

> * From strip command, you SHOULD comment the use of and add -p option
>   to keep timstamps to files.

ok, fixed

> The use of:
> mkdir -p %{buildroot}%{_docdir}/%{name}-%{version}
> %{__install} -m 0644 INSTALL %{buildroot}%{_docdir}/%{name}-%{version} (...)
> %doc %{_docdir}/%{name}-%{version}/ (...)
>
> SHOULDN'T be used.
> Docs SHOULD be add in %doc in %files section such as "%Doc README
> COPYING ChangeLog".
> It's automaticaly installed in %{_docdir}/%{name}-%{version}.

ok, fixed

> * You don't need to explicit write each files in %files section.
>   use %{_sbindir}/ instead.
>   use %{_mandir}/man1/*.1.gz instead.
>   use %{_mandir}/man7/*.7.gz instead.
>   use %{_libdir}/*.so.* instead.
>   use %{_libdir}/*.so instead.

ok, fixed

> * "*.la" files SHOULDN'T be include in -devel subpackage.
>  if for some reason its really require ( for full work), You should
>  add -statics subpackage.
> 
> Can be remove by using:
> find $RPM_BUILD_ROOT/%{_libdir}/ -name '*.la' -exec rm -f {} ';' 

ok, removed

> * Scriptlets seems to miss some options and arguments. see :
> http://fedoraproject.org/wiki/Packaging/ScriptletSnippets?highlight=%28script%29

ok, now only service stop and chkconfig --del at uninstall.

> * "/etc" can be replace by its macros %{_sysconfdir}.

ok, fixed

> %changelog
> please add a empty line between each of them.

ok, fixed.

> reflexion : use should think about the naming of -libs and
> -libs-devel subpackag by renaming subpackage as follow, lib%{name}
> and lib%{name}-devel.

I used to do that, but was told by someone that the Fedora/RedHat
standards was name, name-libs, name-devel and/or name-libs-devel. What
is the correct answer? I found no specific information on this at
http://fedoraproject.org/wiki/Packaging/NamingGuidelines.

Hey Ingvar. 

I'd be happy to review this package and sponsor you... do you have any other
packages pending that you would like to submit? 

Look for a formal review of this in a while here. 

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
See below - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (BSD-like)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
f942bfa029be8a9af9692a43bd04158c  varnish-1.0.3.tar.gz
f942bfa029be8a9af9692a43bd04158c  varnish-1.0.3.tar.gz.1
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install

OK - Headers/static libs in -devel subpackage.
OK - Spec has needed ldconfig in post and postun
OK - .so files in -devel subpackage.
OK - -devel package Requires: %{name} = %{version}-%{release}
OK - .la files are removed.

OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
OK - final provides and requires are sane.

SHOULD Items:

OK - Should build in mock.
OK - Should build on all supported archs
OK - Should function as described.
OK - Should have subpackages require base package with fully versioned depend.
See below - Should have dist tag
OK - Should package latest version

Issues:

1. You might want to use the %{?dist} tag. It makes things easier when you
are building the same version/release for multiple branches.
http://fedoraproject.org/wiki/Packaging/DistTag

2. You should not strip the binaries and libraries... that makes the rpm debuginfo
package empty. Remove the 'strip -p' lines in install.
rpmlint says:
E: varnish-debuginfo empty-debuginfo-package

3. Don't use $RPM_BUILD_ROOT and %{buildroot}. Pick one style and use just that one.

4. You might use for the Source0 url something like:
http://downloads.sourceforge.net/varnish/varnish-%{version}.tar.gz
Not a blocker, but nice to not point to a specific mirror.

5. You shouldn't need to specify Buildroot for each of the subpackages. It should
pick that up from the top. I think the comment above was about "BuildRequires", not
"Buildroot". Also, no need for another URL tag for the subpackages.

6. Do you need to ship static libs? They are discouraged in Fedora for a
variety of reasons. Do you know anything that links to just the static libs?
Or can we remove them for now and readd a -static subpackage later if someone
requests?

7.  Subpackages shouldn't also need to duplicate all the doc files:
%doc INSTALL LICENSE README ChangeLog redhat/README.redhat
Perhaps they should just be in the main package since the others require it?
Also, no need to include the INSTALL file, since people reading here will be
installing via the rpm. ;)

8. Does the description in the main package need to have all the links and
copyright info? The main url should be available via the URL tag, so not sure
if it's worth repeating there. Not sure we should also be pointing to
commercial support information either. They should be able to find that
off the main URL?

9. Should remove the
/sbin/chkconfig --list varnish
in %post. rpms shouldn't have any output when installing.
Also missing some requires for the init script handling, suggest:

Requires(post): /sbin/chkconfig
Requires(preun): /sbin/chkconfig
Requires(preun): /sbin/service

10. You need to own the /etc/varnish directory...
%dir %{_sysconfdir}/varnish/

The short story:

New version of the specfile:
http://users.linpro.no/ingvar/varnish/fedora-extras-commit/varnish.spec
New source rpm:
http://users.linpro.no/ingvar/varnish/fedora-extras-commit/varnish-1.0.3-5.src.rpm

The details:

* Kevin Fenzi
> 1. You might want to use the %{?dist} tag.

OK, added the dist tag in Relase

> 2. You should not strip the binaries and libraries...
> that makes the rpm debuginfo
> package empty. Remove the 'strip -p' lines in install.
> rpmlint says:
> E: varnish-debuginfo empty-debuginfo-package

Ah, I had a %debug_package %{nil} in my .rpmmacros, som I didn't catch
this one with rpmlint. Fixed.

> 3. Don't use $RPM_BUILD_ROOT and %{buildroot}.

OK, fixed.

> 4. You might use for the Source0 url something like:
> http://downloads.sourceforge.net/varnish/varnish-%{version}.tar.gz
> Not a blocker, but nice to not point to a specific mirror.

OK, fixed.

> 5. You shouldn't need to specify Buildroot for each of the
> subpackages. It should pick that up from the top. I think the
> comment above was about "BuildRequires", not "Buildroot". Also, no
> need for another URL tag for the subpackages.

OK, fixed

> 6. Do you need to ship static libs? They are discouraged in Fedora
> for a variety of reasons. Do you know anything that links to just
> the static libs?  Or can we remove them for now and readd a -static
> subpackage later if someone requests?

Yep, removed.

> 7.  Subpackages shouldn't also need to duplicate all the doc files:
> %doc INSTALL LICENSE README ChangeLog redhat/README.redhat

OK, added only the LICENCE file to avoid the "no documentation"
warning from rpmlint.

> Perhaps they should just be in the main package since the others require it?

Yep, that sounds reasonable

> Also, no need to include the INSTALL file, since people reading here will be
> installing via the rpm. ;)

That is not necessarily true. The INSTALL file may contain information
which could inspire users to compile a version of varnish on their
own. Which is not a bad thing. I kept the INSTALL file.

> 8. Does the description in the main package need to have all the
> links and copyright info? 

It's just a dump of the README, so no problem in trimming it down a
bit.

> 9. Should remove the
> /sbin/chkconfig --list varnish
> in %post. rpms shouldn't have any output when installing.  Also
> missing some requires for the init script handling, suggest:
>
> Requires(post): /sbin/chkconfig
> Requires(preun): /sbin/chkconfig
> Requires(preun): /sbin/service

OK, fixed as proposed.

> 10. You need to own the /etc/varnish directory...
> %dir %{_sysconfdir}/varnish/

OK, fixed

1. ok, looks good.
2. ok. looks good.
3. ok. looks good.
4. ok. looks good.
5. ok. looks good.

6. ok on static libs, but did you really want to disable the dynamic
libs as well? Does anything known link against the varnish dynamic libs?
Is there a reason to not ship them?

7. ok. looks good. You shouldn't need the LICENSE in all the subpackages,
but it's not a blocker if you want to do so. The rpmlint warning about
no docs can be ignored for devel subpackages.

If you really want to ship the INSTALL thats fine I would think.

8. ok. looks good.

9. ok. looks good.

10. ok. looks good.

So the only final question I see here is if the devel package should be
shipped or not. It's only the static libs that are reccomended against,
not the dynamic ones.

* Kevin Fenzi
> 6. ok on static libs, but did you really want to disable the dynamic
> libs as well? Does anything known link against the varnish dynamic libs?
> Is there a reason to not ship them?

The dynamic libraries are there allright. I just removed the ".so"
symlinks that points to them. These symlinks are usually part of the
-devel package in redhat-based systems, it I have understood
correctly. Varnish itself links to the main-versioned symlink.

$ ls -l /usr/lib64/*varnish*so
lrwxrwxrwx  1 root root 22 feb 28 14:17 /usr/lib64/libvarnishapi.so ->
libvarnishapi.so.0.0.0
lrwxrwxrwx  1 root root 19 feb 28 14:17 /usr/lib64/libvarnish.so ->
libvarnish.so.0.0.0

$ ldd /usr/sbin/varnishd  | grep usr | cut -d '(' -f 1
        libvarnish.so.0 => /usr/lib64/libvarnish.so.0
        libvcl.so.0 => /usr/lib64/libvcl.so.0

So for varnish itself, the .so symlinks are not necessary, though they
might be valuable if anybody want to develop things on top of varnish
later. Thus, they might belong in a future -devel package.

> 7. ok. looks good. You shouldn't need the LICENSE in all the
> subpackages, but it's not a blocker if you want to do so. The
> rpmlint warning about no docs can be ignored for devel subpackages.

I get this warm fuzzy feeling when I get no errors or warnings, you
see. Also note that "all the subpackages" at the moment counts one :-)

> If you really want to ship the INSTALL thats fine I would think.

Good

> So the only final question I see here is if the devel package should
> be shipped or not. 

Well, I don't know of anyone using varnish technology for anything but
varnish, so for an initial release, I'd say it's not necessary. A
devel package should perhaps include some header files and some
hacking starting point docs too. If we can push it for later, I'll ask
some of the developers for this.

Ingvar

>I get this warm fuzzy feeling when I get no errors or warnings, you
>see. Also note that "all the subpackages" at the moment counts one :-)

indeed. :)

>Well, I don't know of anyone using varnish technology for anything but
>varnish, so for an initial release, I'd say it's not necessary. A
>devel package should perhaps include some header files and some
>hacking starting point docs too. If we can push it for later, I'll ask
>some of the developers for this.

ok. Yeah, if you could talk to upstream about making a more fully featured devel
package before shipping it, that would be great. 

I see no further blockers, so this package is APPROVED, and I would be happy
to sponsor you. You can continue the process from: 
http://fedoraproject.org/wiki/PackageMaintainers/Join#head-a601c13b0950a89568deafa65f505b4b58ee869b

For having packaged and used varnish, just a few comments :
- The %lib_name doesn't seem very useful, and having used plain "libs" instead
of "-n %{lib_name}" for the sub-package would make things clearer. Also, the
"future" devel package would be named wrong since it would be "varnish-libs-devel".
- Some brackets are used inconsistently ("%version-%{release}").
- A condrestart should probably be added in %postun, as it makes sense to
restart varnishd after an update.
- The .gz extensions in %files for the man pages are wrong, you should use
something like "*.1*" instead, for people who rebuild with uncompressed or
bzip2ed man pages.
- You could spare a lot of "mkdir -p" by using "install -D".
- The "--sbindir=/usr/sbin" on the %configure line is redundant.
- The iteration for the UTF-8 conversion would be best done with a glob, i.e.
"for i in bin/*/*.1", as it'll be less subject to break if any programs are
added or removed.
- I would personally add a comment above the "Requires: gcc" line to explain
that varnish *really* needs a C compiler at runtime by design because of its VCL
files.
- The explicit requirements on "ncurses" should be removed, as it's wrong to
have it (wouldn't allow for a compat-ncurses to work right).
- The kernel requirement should probably be removed from the libs package,
unless they are the ones requiring 2.6 specific features (but I think it's only
the daemon).

rpmlint's output is valuable, but having it empty unfortunately doesn't
necessarily mean that the package is perfect!

Ingvar: Can you address the items from Comment #10?

>rpmlint's output is valuable, but having it empty unfortunately doesn't
>necessarily mean that the package is perfect!

Very true. :) 
I think some of the suggestions are minor/cosmetic, but they all make sense to me. 

Thanks for the input Matthias!

The short story:

Updated specfile: 
http://users.linpro.no/ingvar/varnish/fedora-extras-commit/varnish.spec

Updated source rpm:
http://users.linpro.no/ingvar/varnish/fedora-extras-commit/varnish-1.0.3-6.src.rpm


Details and comments:

* Matthias Saou
> - The %lib_name doesn't seem very useful, and having used plain
>   "libs" instead of "-n %{lib_name}" for the sub-package would make
>   things clearer. Also, the "future" devel package would be named
>   wrong since it would be "varnish-libs-devel".

The macro has been hanging around since I experimented with names and
an old mandrake-ish rpmlint. Agreed and fixed.

> - Some brackets are used inconsistently ("%version-%{release}").

Ah, thanks. Fixed.

> - A condrestart should probably be added in %postun, as it makes
>   sense to restart varnishd after an update.

Yup, added.

> - The .gz extensions in %files for the man pages are wrong, you should use
>   something like "*.1*" instead, for people who rebuild with uncompressed or
>   bzip2ed man pages.

Fixed

> - You could spare a lot of "mkdir -p" by using "install -D".

Fixed

> - The "--sbindir=/usr/sbin" on the %configure line is redundant.

No, it's not. At least, not unless this has been fixed in upstream
very, recently. In 1.0 it was needed.

> - The iteration for the UTF-8 conversion would be best done with a
>   glob, i.e.  "for i in bin/*/*.1", as it'll be less subject to break
>   if any programs are added or removed.

...and it became simpler and shorter too. Fixed.

> - I would personally add a comment above the "Requires: gcc" line to
>   explain that varnish *really* needs a C compiler at runtime by
>   design because of its VCL files.

Point. Fixed.

> - The explicit requirements on "ncurses" should be removed, as it's wrong to
>   have it (wouldn't allow for a compat-ncurses to work right).

Right. Removed, and let rpm find the correct deps by itself.

> - The kernel requirement should probably be removed from the libs
>   package, unless they are the ones requiring 2.6 specific features
>   (but I think it's only the daemon).

Yup, that's correct. Fixed.
 
> rpmlint's output is valuable, but having it empty unfortunately doesn't
> necessarily mean that the package is perfect!

But of course. Thanks for the input. I also stole some of your init-
and config file items :-)

Ingvar

Those changes look mostly good, however: 

The condrestart, shouldn't: 
  /sbin/service condrestart > /dev/null 2>/dev/null
be:
  /sbin/service varnish condrestart > /dev/null 2>/dev/null

and it shouldn't be in the -libs subpackage, but the main package? 

Other than that I see no issues... Matthias ?

Ugh, fixec. src.rpm and spec in the same location.

That looks fixed to me...

I think this should be good to go now, unless Matthais sees any further places
for improvement? 

Just another quick look, and only minor points :
- Instead of using "-n varnish-libs" (and -devel), plain "libs" is shorter and
more often used.
- I maintain that --sbindir=/usr/sbin is redundant ;-)

All the rest looks much better.

Happy May 17, it's the Norwegian Constitution Day!

varnish-1.0.4 is on the stairs, banging at the door. Adding 1.0.3 to Fedora, and
then upgrade directly afterwards seems a little unnecessary, so I'll wait a few
days before the final commit. Till then, please have a look at this build, based
on the svn 1.0 branch. It's pretty close to what the final 1.0.4 will look like. 

Changes in the rpm package are as follows. Comments are very welcome.

* The included vcl config has changed name (upstream change)
* varnishlog is now able to daemonize properly, has got an init script and a 
  logrotate script
* The sysconfig file has changed a bit (to the better), and is now more or less
  in sync with the Debian defaults file. Varnish' init script is changed to 
  reflect this.
* pidfile support in the init scripts and other cosmetic changes

src.rpm:
http://users.linpro.no/ingvar/varnish/fedora-extras-commit/varnish-1.0.svn-20070517.src.rpm

specfile: http://users.linpro.no/ingvar/varnish/fedora-extras-commit/varnish.spec

Ingvar

New Package CVS Request
=======================
Package Name: varnish
Short Description: Varnish is a high-performance HTTP accelerator
Owners: ingvar
Branches: FC-6 F-7

varnish-1.0.4 released. Package updated. See comment #17 for changes in the rpm
packages.

src.rpm:
http://users.linpro.no/ingvar/varnish/fedora-extras-commit/varnish-1.0.4-2.src.rpm

specfile: http://users.linpro.no/ingvar/varnish/fedora-extras-commit/varnish.spec

Package Change Request
======================
Package Name: Varnish
[New Branches: ] EL-4 EL-5

As 2.0-beta1 is out, and I've got a lot of requests for branching for epel, I'd like to do that now. The rawhide package compiles cleanly and works well on el4 and el5.

Ingvar

cvs done.