Bug 665005

Update:

Spec URL: http://corsepiu.fedorapeople.org/packages/perl-Server-Starter.spec
SRPM URL:
http://corsepiu.fedorapeople.org/packages/perl-Server-Starter-0.09-2.fc15.src.rpm

+ source files match upstream.  
    f3a89be749127dcf5b46b97befbfc916  Server-Starter-0.09.tar.gz

+ package meets naming and versioning guidelines.
+ specfile is properly named, is cleanly written and uses macros consistently.
+ summary is OK.
+ description is OK.
+ dist tag is present.
+ build root is OK.
+ license field matches the actual license.
    GPL+ or Artistic

+ license is open source-compatible.
+ license text not included upstream.
- latest version is being packaged.
    0.11 is available now

+ BuildRequires are proper.
+ compiler flags are appropriate.
+ %clean is present.
+ package builds in mock
    http://koji.fedoraproject.org/koji/taskinfo?taskID=2732539

+ package installs properly.
- rpmlint has no complaints:
    perl-Server-Starter.noarch: W: spelling-error Summary(en_US) Superdaemon -> Super daemon, Super-daemon, Superdominant
    perl-Server-Starter.noarch: W: spelling-error %description -l en_US superdaemon -> super daemon, super-daemon, superdominant
    perl-Server-Starter.noarch: W: spelling-error %description -l en_US commenctions -> commendations, commensuration, commendation
    perl-Server-Starter.noarch: W: spurious-executable-perm /usr/share/doc/perl-Server-Starter-0.09/start_server
    perl-Server-Starter.noarch: W: doc-file-dependency /usr/share/doc/perl-Server-Starter-0.09/start_server perl(Getopt::Long)
    perl-Server-Starter.noarch: W: doc-file-dependency /usr/share/doc/perl-Server-Starter-0.09/start_server /usr/bin/perl
    perl-Server-Starter.noarch: W: doc-file-dependency /usr/share/doc/perl-Server-Starter-0.09/start_server perl(Pod::Usage)
    perl-Server-Starter.src: W: spelling-error Summary(en_US) Superdaemon -> Super daemon, Super-daemon, Superdominant
    perl-Server-Starter.src: W: spelling-error %description -l en_US superdaemon -> super daemon, super-daemon, superdominant
    perl-Server-Starter.src: W: spelling-error %description -l en_US commenctions -> commendations, commensuration, commendation
    perl-Server-Starter.src:67: E: files-attr-not-set
    perl-Server-Starter.src:68: E: files-attr-not-set
    perl-Server-Starter-start_server.noarch: W: summary-not-capitalized C perl-Server-Starter start_server script

- final provides and requires are sane:
    $ rpm -qp --provides perl-Server-Starter-0.09-2.fc15.noarch.rpm 
    perl(Server::Starter) = 0.09
    perl-Server-Starter = 0.09-2.fc15

    $ rpm -qp --provides perl-Server-Starter-start_server-0.09-2.fc15.noarch.rpm 
    perl-Server-Starter-start_server = 0.09-2.fc15

=
    $ rpm -qp --requires perl-Server-Starter-0.09-2.fc15.noarch.rpm 
    /usr/bin/perl  
    perl >= 0:5.008
    perl(:MODULE_COMPAT_5.12.2)  
    perl(Carp)  
    perl(Exporter)  
    perl(Fcntl)  
    perl(Getopt::Long)  
    perl(IO::Handle)  
    perl(IO::Socket::INET)  
    perl(List::MoreUtils)  
    perl(POSIX)  
    perl(Pod::Usage)  
    perl(Proc::Wait3)  
    perl(Scope::Guard)  
    perl(Server::Starter)  
    perl(strict)  
    perl(warnings)  

    $ rpm -qp --requires ./perl-Server-Starter-start_server-0.09-2.fc15.noarch.rpm |sort
    perl(Getopt::Long)  
    perl(Pod::Usage)  
    perl(Server::Starter)  
    perl(strict)  
    perl(warnings)  

+ %check is present and all tests pass.
    PERL_DL_NONLAZY=1 /usr/bin/perl "-MExtUtils::Command::MM" "-e" "test_harness(0, 'inc', 'blib/lib', 'blib/arch')" t/00-base.t t/01-starter.t t/02-startfail.t
    t/00-base.t ....... ok
    start_server (pid:24883) starting now...
    starting new worker 24886
    received HUP, spawning a new worker
    starting new worker 25823
    new worker is now running, sending TERM to old workers:24886
    old worker 24886 died, status:0
    received TERM, sending TERM to all workers:25823
    worker 25823 died, status:0
    exitting
    start_server (pid:30769) starting now...
    starting new worker 30770
    received HUP, spawning a new worker
    starting new worker 32375
    new worker is now running, sending USR1 to old workers:30770
    old worker 30770 died, status:0
    received TERM, sending TERM to all workers:32375
    worker 32375 died, status:0
    exitting
    t/01-starter.t .... ok
    start_server (pid:5389) starting now...
    starting new worker 5390
    new worker 5390 seems to have failed to start, exit status:256
    starting new worker 5690
    received HUP, spawning a new worker
    starting new worker 6885
    new worker 6885 seems to have failed to start, exit status:256
    starting new worker 6886
    new worker 6886 seems to have failed to start, exit status:256
    starting new worker 7656
    new worker is now running, sending TERM to old workers:5690
    old worker 5690 died, status:0
    received TERM, sending TERM to all workers:7656
    worker 7656 died, status:0
    exitting
    t/02-startfail.t .. ok
    All tests successful.
    Files=3, Tests=38, 28 wallclock secs ( 0.02 usr  0.01 sys +  0.59 cusr  0.15 csys =  0.77 CPU)
    Result: PASS

+ no shared libraries are added to the regular linker search paths.
+ owns the directories it creates.
+ doesn't own any directories it shouldn't.
+ no duplicates in %files.
+ file permissions are appropriate.
+ no generically named files
+ code, not content.
+ documentation is small, so no -doc subpackage is necessary.
+ %docs are not necessary for the proper functioning of the package.


There's a few minor problems:

- newer version 0.11 is available
- start_server script shouldn't be in main package's %doc
- %files start_server section needs to set %defattr(-,root,root,-)
- spelling in description needs fixing (commenctions -> connections; superdaemon is okay for me)

And I'm not entirely convinced that start_server needs its own sub-package, why not just include it in the main package?

And if you keep the sub-package, I think it should explicitly require perl-Server-Starter = %{version}-%{release}

(In reply to comment #2)

> - newer version 0.11 is available
Updated.

> - start_server script shouldn't be in main package's %doc
Fixed (cf. below)

> - %files start_server section needs to set %defattr(-,root,root,-)
Fixed.

> - spelling in description needs fixing (commenctions -> connections;
> superdaemon is okay for me)
cpanspec pulling in upstream text :) Fixed

> And I'm not entirely convinced that start_server needs its own sub-package,
> why not just include it in the main package?

I package it separately, because
a) I consider /usr/bin/start_server to be a very poor choice of script naming, which is causing confusions related to start-stop-daemon etc.

Initially, I had tried not to ship it and to move it into %doc instead, until I subsequently found it is used by another other package. start_server's presence in %doc was a remnant of these experiments.

ATM, I am tempted to go a step further: Rename the sub package into "start_server" instead of "perl-Start-Server-start_server" ;)

b) I wanted to reduce the deps between perl-Start-Server package and other packages, because
-  So far, start_server is not used by an run-time package and is only used by the testsuite of another package which is currently waiting for review.

- /usr/bin/start_server pulls in additional deps, which are not being used by perl-Start-Server itself [ATM all these additional deps happen to be provided by the core perl package, so this argument is mostly moot]

(In reply to comment #3)
> And if you keep the sub-package, I think it should explicitly require
> perl-Server-Starter = %{version}-%{release}
Hmm, I don't see much need to do so

* start_server actually is an application, which only happens to be bundled with the Server-Starter-tarball, but otherwise is only loose tied to perl(Server::Starter), just like any other arbitrary perl-application.

* start_server originates from the same tarball as perl(Server::Starter), so it will be updated at the same time.

Update:

Spec URL: http://corsepiu.fedorapeople.org/packages/perl-Server-Starter.spec
SRPM URL:
http://corsepiu.fedorapeople.org/packages/perl-Server-Starter-0.11-1.fc15.src.rpm

(In reply to comment #4)
> (In reply to comment #2)
> 
> > - newer version 0.11 is available
> Updated.

new source files match upstream:
    037d75831a23ca76cd306d678b20332e  Server-Starter-0.11.tar.gz

new package builds in mock:
    http://koji.fedoraproject.org/koji/taskinfo?taskID=2732540

> > - start_server script shouldn't be in main package's %doc
> Fixed (cf. below)

ack
 
> > - %files start_server section needs to set %defattr(-,root,root,-)
> Fixed.

ack


> > - spelling in description needs fixing (commenctions -> connections;
> > superdaemon is okay for me)
> cpanspec pulling in upstream text :) Fixed

ack

 
> > And I'm not entirely convinced that start_server needs its own sub-package,
> > why not just include it in the main package?
> 
> I package it separately, because
> a) I consider /usr/bin/start_server to be a very poor choice of script naming,
> which is causing confusions related to start-stop-daemon etc.
> 
> Initially, I had tried not to ship it and to move it into %doc instead, until I
> subsequently found it is used by another other package. start_server's presence
> in %doc was a remnant of these experiments.
> 
> ATM, I am tempted to go a step further: Rename the sub package into
> "start_server" instead of "perl-Start-Server-start_server" ;)
> 
> b) I wanted to reduce the deps between perl-Start-Server package and other
> packages, because
> -  So far, start_server is not used by an run-time package and is only used by
> the testsuite of another package which is currently waiting for review.
> 
> - /usr/bin/start_server pulls in additional deps, which are not being used by
> perl-Start-Server itself [ATM all these additional deps happen to be provided
> by the core perl package, so this argument is mostly moot]

OK. I think the existing sub-package name is better, though. "start_server" seems a little too generic. Since it's not really intended as an end-user application, it can hide away in perl- package namespace.

> (In reply to comment #3)
> > And if you keep the sub-package, I think it should explicitly require
> > perl-Server-Starter = %{version}-%{release}
> Hmm, I don't see much need to do so
> 
> * start_server actually is an application, which only happens to be bundled
> with the Server-Starter-tarball, but otherwise is only loose tied to
> perl(Server::Starter), just like any other arbitrary perl-application.
> 
> * start_server originates from the same tarball as perl(Server::Starter), so it
> will be updated at the same time.

According to guidelines on requiring base package, "It is almost always better to over specify the version, so it's best practice to just use a fully versioned dependency: Requires: %{name} = %{version}-%{release}". Otherwise its possible to "yum update perl-Server-Starter" and end up with new module and older start_service script.

APPROVED (with explicit requires in sub-package).

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #2)
> > 
> > > - newer version 0.11 is available

> > ATM, I am tempted to go a step further: Rename the sub package into
> > "start_server" instead of "perl-Start-Server-start_server" ;)
 
> OK. I think the existing sub-package name is better, though. "start_server"
> seems a little too generic.
Correct, this would expose the quality of upstream's name choice to users.

> > (In reply to comment #3)
> > > And if you keep the sub-package, I think it should explicitly require
> > > perl-Server-Starter = %{version}-%{release}
> > Hmm, I don't see much need to do so
> > 
> > * start_server actually is an application, which only happens to be bundled
> > with the Server-Starter-tarball, but otherwise is only loose tied to
> > perl(Server::Starter), just like any other arbitrary perl-application.
> > 
> > * start_server originates from the same tarball as perl(Server::Starter), so it
> > will be updated at the same time.
> 
> According to guidelines on requiring base package, "It is almost always better
> to over specify the version, so it's best practice to just use a fully
> versioned dependency: Requires: %{name} = %{version}-%{release}". Otherwise its
Can we have some common sense, please? "almost better" != "is better".

> possible to "yum update perl-Server-Starter" and end up with new module and
> older start_service script.
This would only do harm if perl(Server::Starter)'s API changes.

Openly said, if the start_server script was packaged as a separate source tarball, we wouldn't be discussing this topic at all.

> APPROVED (with explicit requires in sub-package).
I'll do so under explict *protest*.

Anyway, thanks for the review.

New Package SCM Request
=======================
Package Name: perl-Server-Starter
Short Description: Superdaemon for hot-deploying server programs
Owners: corsepiu
Branches: f13 f14
InitialCC: perl-sig

Git done (by process-git-requests).

perl-Server-Starter-0.11-2.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/perl-Server-Starter-0.11-2.fc13

perl-Server-Starter-0.11-2.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/perl-Server-Starter-0.11-2.fc14

perl-Server-Starter-0.11-2.fc13 has been pushed to the Fedora 13 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update perl-Server-Starter'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/perl-Server-Starter-0.11-2.fc13

perl-Server-Starter-0.11-2.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

perl-Server-Starter-0.11-2.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.