Bug 252257

Shouldn't these modules also be included as build requires and/or requires? :
Warning: prerequisite Gearman::Client 1.07 not found.
Warning: prerequisite Gearman::Client::Async 0.93 not found.
Warning: prerequisite Gearman::Server 1.08 not found.
Warning: prerequisite Net::Netmask 0 not found.

Oops, fixed this a while ago.
New version is at
http://rubenkerkhof.com/packages/perl-MogileFS-Server-2.17-2.fc7.src.rpm

I would also like to see this added. Thanks!

Not sure why I never looked at this one.  Unfortunately it fails to build for
me, because the mogstored-shutdown test fails:

Failed Test            Stat Wstat Total Fail  Failed  List of Failed
-------------------------------------------------------------------------------
t/mogstored-shutdown.t  255 65280     4    5 125.00%  2-4

The output from the failing test is:
(No info could be read for "-p": geteuid()=7225 but you should be root.)
(Not all processes could be identified, non-owned process info
 will not be shown, you would have to be root to see it all.)
real_pid = 24068

#   Failed test 'got mgmt connection'
#   in t/mogstored-shutdown.t at line 41.
IO::Socket::INET: connect: Connection refused   ...propagated at t/mogstored-shutdown.t line 41.

No clue what's going on here; mock builds don't run as root so I doubt this
test is going to work properly.  I'd suggest just deleting
t/mogstored-shutdown in %check, unless you think you can patch it to do
something useful.

I went ahead and deleted that test so I could look at the rest of the package.
rpmlint says:
  mogstored-backend-apache.noarch: W: no-documentation
  mogstored-backend-lighttpd.noarch: W: no-documentation
  mogstored-backend-perlbal.noarch: W: no-documentation
which are all OK.

I note that mogilefsd includes a license note:
  "Same terms as Perl itself.  Artistic/GPLv2, at your choosing."
This is a bit confusing, because the two statements are not the same.  I'm
going to have to assume that the second statement is merely a confused
explanation and that the first actually states the reality, but as usual you
should actually talk to them and ask them what they intend.

I'm a bit confused about the name of this package; I'm thinking that it would
make more sense to just call the SPRM "mogilefsd" or "mogilefs-server".
There's certainly no reason to name it with the mixed case since even on CPAN
it's named in lower case.

Should the perlbal backend have some sort of dependency on perlbal?  It
doesn't seem to have one currently.

Checklist:
* source files match upstream:
   05ce0dcd8973dee68f9cf28e9111fa639a62eaea8836099717381de237ff0aee  
   mogilefs-server-2.17.tar.gz
? I'm unsure about the proper name of this package.
* specfile is properly named, is cleanly written and uses macros consistently.
* summaries are OK.
* descriptions are OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text not included upstream.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
X package fails to build in mock unless I remove one of the tests.
* package installs properly
* rpmlint has acceptable complaints.
? final provides and requires are sane:
  mogilefsd-2.17-2.fc9.noarch.rpm
   config(mogilefsd) = 2.17-2.fc9
   perl(Mgd)
   perl(MogileFS)
   perl(MogileFS::Class)
   perl(MogileFS::Config)
   perl(MogileFS::Connection::Client)
   perl(MogileFS::Connection::Mogstored)
   perl(MogileFS::Connection::Worker)
   perl(MogileFS::DevFID)
   perl(MogileFS::Device)
   perl(MogileFS::DeviceState)
   perl(MogileFS::Domain)
   perl(MogileFS::Exception)
   perl(MogileFS::FID)
   perl(MogileFS::HTTPFile)
   perl(MogileFS::Host)
   perl(MogileFS::IOStatWatch::Client)
   perl(MogileFS::IOStatWatcher)
   perl(MogileFS::ProcManager)
   perl(MogileFS::RebalancePolicy)
   perl(MogileFS::RebalancePolicy::DrainDevices)
   perl(MogileFS::RebalancePolicy::PercentFree)
   perl(MogileFS::RebalancePolicy::Random)
   perl(MogileFS::ReplicationPolicy)
   perl(MogileFS::ReplicationPolicy::MultipleHosts)
   perl(MogileFS::ReplicationRequest)
   perl(MogileFS::Server) = 2.17
   perl(MogileFS::Store)
   perl(MogileFS::Store::MySQL)
   perl(MogileFS::Sys)
   perl(MogileFS::Util)
   perl(MogileFS::Worker)
   perl(MogileFS::Worker::Delete)
   perl(MogileFS::Worker::Fsck)
   perl(MogileFS::Worker::Monitor)
   perl(MogileFS::Worker::Query)
   perl(MogileFS::Worker::Reaper)
   perl(MogileFS::Worker::Replicate)
   mogilefsd = 2.17-2.fc9
  =
   /bin/bash
   /bin/sh
   /sbin/chkconfig
   /sbin/service
   /usr/bin/perl
   config(mogilefsd) = 2.17-2.fc9
   initscripts
   perl(Carp)
   perl(DBD::mysql)
   perl(DBI)
   perl(DBI) >= 1.44
   perl(Danga::Socket)
   perl(Danga::Socket) >= 1.56
   perl(Exporter)
   perl(File::Basename)
   perl(File::Copy)
   perl(File::Path)
   perl(Getopt::Long)
   perl(IO::Socket)
   perl(IO::Socket::INET)
   perl(LWP::UserAgent)
   perl(List::Util)
   perl(MogileFS::Class)
   perl(MogileFS::Config)
   perl(MogileFS::Connection::Client)
   perl(MogileFS::Connection::Mogstored)
   perl(MogileFS::Connection::Worker)
   perl(MogileFS::DevFID)
   perl(MogileFS::Device)
   perl(MogileFS::DeviceState)
   perl(MogileFS::Domain)
   perl(MogileFS::Exception)
   perl(MogileFS::FID)
   perl(MogileFS::HTTPFile)
   perl(MogileFS::Host)
   perl(MogileFS::IOStatWatcher)
   perl(MogileFS::ProcManager)
   perl(MogileFS::RebalancePolicy::DrainDevices)
   perl(MogileFS::ReplicationPolicy::MultipleHosts)
   perl(MogileFS::ReplicationRequest)
   perl(MogileFS::Server)
   perl(MogileFS::Store)
   perl(MogileFS::Store::MySQL)
   perl(MogileFS::Sys)
   perl(MogileFS::Util)
   perl(MogileFS::Worker::Delete)
   perl(MogileFS::Worker::Fsck)
   perl(MogileFS::Worker::Monitor)
   perl(MogileFS::Worker::Query)
   perl(MogileFS::Worker::Reaper)
   perl(MogileFS::Worker::Replicate)
   perl(Net::Netmask)
   perl(POSIX)
   perl(Socket)
   perl(Symbol)
   perl(Sys::Hostname)
   perl(Sys::Syscall) >= 0.22
   perl(Sys::Syslog)
   perl(Time::HiRes)
   perl(base)
   perl(constant)
   perl(fields)
   perl(lib)
   perl(overload)
   perl(strict)
   perl(vars)
   perl(warnings)
   shadow-utils

  mogstored-2.17-2.fc9.noarch.rpm
   perl(Mogstored::ChildProcess)
   perl(Mogstored::ChildProcess::DiskUsage)
   perl(Mogstored::ChildProcess::FIDSizes)
   perl(Mogstored::ChildProcess::IOStat)
   perl(Mogstored::FIDStatter)
   perl(Mogstored::HTTPServer)
   perl(Mogstored::SideChannelClient)
   perl(Mogstored::SideChannelListener)
   mogstored = 2.17-2.fc9
  =
   /bin/bash
   /bin/sh
   /sbin/chkconfig
   /sbin/service
   /usr/bin/perl
   initscripts
   perl(Carp)
   perl(Errno)
   perl(FindBin)
   perl(Gearman::Client) >= 1.06
   perl(Gearman::Client::Async) >= 0.93
   perl(Gearman::Server) >= 1.08
   perl(Gearman::Worker)
   perl(Getopt::Long)
   perl(IO::Socket::INET)
   perl(Mogstored::HTTPServer)
   perl(Mogstored::SideChannelClient)
   perl(Mogstored::SideChannelListener)
   perl(POSIX)
   perl(Perlbal) >= 1.53
   perl(Storable)
   perl(base)
   perl(fields)
   perl(lib)
   perl(strict)
   perl(warnings)
   shadow-utils

  mogstored-backend-apache-2.17-2.fc9.noarch.rpm
   perl(Mogstored::HTTPServer::Apache)
   mogstored-backend-apache = 2.17-2.fc9
  =
   httpd
   perl(File::Temp)
   perl(Mogstored::HTTPServer)
   perl(base)
   perl(strict)

  mogstored-backend-lighttpd-2.17-2.fc9.noarch.rpm
   perl(Mogstored::HTTPServer::Lighttpd)
   mogstored-backend-lighttpd = 2.17-2.fc9
  =
   lighttpd
   perl(File::Temp)
   perl(Mogstored::HTTPServer)
   perl(base)
   perl(strict)

? mogstored-backend-perlbal-2.17-2.fc9.noarch.rpm
   perl(Mogstored::HTTPServer::Perlbal)
   mogstored-backend-perlbal = 2.17-2.fc9
  =
   perl(Fcntl)
   perl(Mogstored::HTTPServer)
   perl(POSIX)
   perl(base)
   perl(strict)

X %check is present but fails; if I remove one test, I get:
   All tests successful, 6 tests skipped.
   Files=9, Tests=25,  1 wallclock secs ( 1.05 cusr +  0.21 csys =  1.26 CPU)
  The skipped tests require a running mysql server and a configuration to
   access it.

* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets look OK (user/group addition, initscripts)
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

Hi Jason,

I can't reproduce the failing test in mock.
What I think is that there's too little time between starting up mogstored (which succeeds, because the 
pid is echoed) and the connect call.

Increasing the timeout is not the solution, so I've deleted the test.

I've also renamed the package to mogilefs-server, and added a Requires for perlbal to the perlbal 
backend.

New version:
http://ruben.fedorapeople.org/mogilefs-server.spec
http://ruben.fedorapeople.org/mogilefs-server-2.17-3.fc8.src.rpm

Odd that you can't reproduce it.  My builder is has 2xQuad core 3.0GHz Xeons and
16GB of RAM, and everything is building in tmpfs so there is essentially no IO
overhead.  Maybe that's what's going on.

Everything looks good to me now, except that the mogstored-backend-perlbal
package won't install because there's no package named "perlbal".  I think the
actual package is capitalized ("Perlbal").

That's a trivial fix, though; you can take care of it when you check in.

APPROVED

> Odd that you can't reproduce it.  My builder is has 2xQuad core 3.0GHz Xeons and
> 16GB of RAM, and everything is building in tmpfs so there is essentially no IO
> overhead.  Maybe that's what's going on.

I'm building on a much slower machine than that, so you would've expected the test on my system to 
fail, instead of yours :-)
Good reason to not run it I guess.

I've fixed the typo, and removed perl(Perlbal) from the mogstored requirements as well.

Thanks for the review!

New Package CVS Request
=======================
Package Name: mogilefs-server
Short Description: Server part of the MogileFS distributed filesystem
Owners: ruben
Branches: F-7 F-8 EL-4 EL-5

cvs done.

/me thinks this package's package name is wrong.

I should probably be renamed into perl-mogilefe-server, to comply to Fedora's
conventions on CPAN dists 

(This package is a CPAN dist, c.f.
http://search.cpan.org/dist/mogilefs-server)

I disagree with Ralf, and do not believe that the package should be renamed. 
This is an application, not a Perl module, and we do not require that
applications be named after the language they happen to be written in,
regardless of there the source code might be hosted.

My guess is that the guidelines were written with perl modules in mind.
mogilefs-server contains 2 standalone daemons, and isn't meant to be included in other perl programs.

So in this case, mogilefs-server is probably more clear, but both names have its pro's and con's.

(In reply to comment #12)
> My guess is that the guidelines were written with perl modules in mind.
The guidelines were written with CPAN distributions (distribution != module) in
mind.

> mogilefs-server contains 2 standalone daemons, and isn't meant to be included
in other perl programs.
This doesn't matter. 

What matters is the package installing its modules into the perl-vendor-tree, is
it  being distributed via CPAN (it is update-able via cpan), and is ease of
maintainer's (using cpancheck) and user's life (Making this package's relation
to CPAN obvious).

Also, we've had precedences where package had been forced to be renamed to using
the perl-<distribution> convention. I don't see why this package should be an
exception.

> So in this case, mogilefs-server is probably more clear, but both names 
> have its pro's and con's.
There would be nothing wrong in letting this package "Provide: mogilefs-server".

It's all about ease of life, but ... 

I'd actually agree with Ralf here. This should be renamed to
perl-mogilefs-server, since it is a CPAN dist.

And I still must disagree.  Unless we're going to start renaming projects
because they're hosted at sourceforge.

Hmm, I'm wondering what to do now since my reviewers disagree :-)
I don't have a strong opinion about this. Of course guidelines are there to be followed, but they're not the 
law either.

Maybe the packaging commitee should have a discussion about this?

(In reply to comment #15)
> And I still must disagree.  Unless we're going to start renaming projects
> because they're hosted at sourceforge.
That's irrelevant. 

This is a CPAN distributed distribution
http://search.cpan.org/dist/mogilefs-server/
=> "cpan update" will pull them in.
=> it is being installed under the perl installation hierarchy.
=> its modules are being searched under the standard perl module directories
=> it can be used by other perl modules.

It's not any different from any other CPAN distributions, it's the same as for
why all perl packages are being named perl-<distro>

It's the same as for why perl's "gettext"
http://search.cpan.org/dist/gettext/
is called perl-gettext (cf. it's review. This particular package had been
renamed for exactly the same rationale)

Don't get me wrong, I am not opposed to add a 
"Provides: moglilefs-server = <package-version>" 
to the corresponding package.


(In reply to comment #16)
> Hmm, I'm wondering what to do now since my reviewers disagree :-)
> I don't have a strong opinion about this.
I do.

> Maybe the packaging commitee should have a discussion about this?
Tibbs, Spot and I are package committee members. Ville (another FPC member) had
enforced this rule in the past.

I continue to disagree.  An application that happens to include Perl modules
does not need to be named perl-* any more than an application written in any
other language would.  If there were reason to split the modules from the
application then of course the package with the modules would of course have the
usual naming convention for Perl modules.  It may be that that's the case.

See also Spamassassin, in the distro for some time now.  It's clearly an
application, having an initscript, config files and executables, but it happens
to include a whole bunch of Perl modules.  (And it's not the only one; Perlbal
is another.)  Frankly I'd expect that any well-written Perl application would
include some modules.  But now I fully expect Ralf to launch into another tirade
to try to get Spamassassin and Perlbal renamed as well.  Such is life.

Ruben, in the end, I don't really care, and I won't comment further on this
ticket.  I personally just tune Ralf out when he goes on like this, but if you
decide that you want to rename this package then be my guest.  Just make another
CVS request and we'll figure out how to get rid of the existing package.

The current client & utils modules are named:

perl-MogileFS-Client-1.08-1.fc8
perl-MogileFS-Utils-2.12-1.fc8

To go with the flow would be to prefix it with perl, but I would personally
prefer that none of these packages be prefixed with perl-. 

When I go looking to install them, I neither care that they're written in perl,
nor do I think to yum install perl-MogileFS-Server. The thing I'm most likely to
try is yum install mogilefs-client mogilefs-utils mogilefs-server.

I want the mogile server, client, and utils, for the most part I don't much care
that they're written in php, perl, or COBOL. Similarly, I want the RPM, and I
don't care that the modules are also available in CPAN.

I only want to say that I don't think it's cool to start arguing with a reviewer after he approved a 
package, in a bugzilla ticket. Especially not if they're in your own commitee.

I really appreciate all the time Tibbs took to review this package (and all it's dependencies).

Having said that, rules are rules, and this is not the place to get them changed, so here we go:

Package Change Request
======================
Package Name: mogilefs-server
New Package Name: perl-MogileFS-Server

I tend to agree that we (the packaging committee) will have to re-examine this
issue in the future and decide on a better method to differentiate between
modules and applications and what to do when an application provides its major
functionality as a module.  Examples of this lack of separation of application
from the module install space exists in python and other packages written in
other languages as well.

For now, cvs/pkgdb rename is done.

Thanks Toshio,

Whats next to do?
I guess I have to bump the release, obsolete the old version of mogilefs-server, provide mogilefs-server 
and retag/rebuild?

(In reply to comment #20)
> I only want to say that I don't think it's cool to start arguing with a
reviewer after he approved a 
> package, in a bugzilla ticket. 
Sorry, but this kind of situations will always be inevitable, when a package
review has failed, which I think it did in this case.

(In reply to comment #20)

Argh, this is wrong either:
> Package Change Request
> ======================
> Package Name: mogilefs-server
> New Package Name: perl-MogileFS-Server

perl-mogilefs-server would have been it.

The perl distribution is called "mogilefs-server", it is not "MogileFS-Server".

(In reply to comment #24)

Argh, your right.

Package Change Request
======================
Package Name: perl-MogileFS-Server
New Package Name: perl-mogilefs-server

cvs done.

Package Change Request
======================
Package Name: perl-mogilefs-server
Branches: f15 f14
New InitialCC: perl-sig

Please add `perl-sig' to CC list for all Fedora branches as this is Perl
package.

This is now done via pkgdb.