[Bug 915791] New: Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915791

Bug ID: 915791
           Summary: Review Request: perl-MogileFS-Server - Server part of
                    the MogileFS distributed file system
           Product: Fedora
           Version: rawhide
         Component: Package Review
          Severity: medium
          Priority: medium
          Reporter: [e-mail]

Spec URL:
http://ppisar.fedorapeople.org/perl-MogileFS-Server/perl-MogileFS-Server.spe...
SRPM URL:
http://ppisar.fedorapeople.org/perl-MogileFS-Server/perl-MogileFS-Server-2.6...
Description:
Server part of the MogileFS distributed file system.

Fedora Account System Username: ppisar

-----
This package perl-MogileFS-Server replaces perl-mogilefs-server. Binary
packages keep the same name for compatibility. The only difference is new
upstream and systemd support.

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915791

Petr Pisar  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |913251
                 CC|                            |[e-mail]

--- Comment #1 from Petr Pisar  ---
*** Bug 913493 has been marked as a duplicate of this bug. ***

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #2 from Petr Pisar  ---
*** Bug 720916 has been marked as a duplicate of this bug. ***

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915791

Petr Šabata  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
           Assignee|[e-mail]    |[e-mail]
              Flags|                            |fedora-review?

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #3 from Petr Šabata  ---
Hmmm, many of the tests fail in mock.
It also seems your will need SCTP support which is optional in Fedora.  This
will require thorough inspection...

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #4 from Petr Pisar  ---
These are some time-outs or locking problems in SQLite:

# Creating 100 files...
# created 10/100
[Wed Mar 20 16:51:06 2013] crash log: Database error from
MogileFS::Store/lib/MogileFS/Store.pm/1219: database is locked at
lib/MogileFS/Worker/Monitor.pm line 378.
# created 20/100
[Wed Mar 20 16:51:07 2013] Child 2640 (monitor) died: 256 (UNEXPECTED)
[Wed Mar 20 16:51:07 2013] Job monitor has only 0, wants 1, making 1.
# created 30/100
MogileFS::Backend: timed out after 3s against 127.0.0.1:7001 when sending
command: [create_close
domain=testdom&fid=43&devid=2&path=http://127.0.1.1:7500/dev2/0/000/000/0000000043.fid&size=8192&key=ma...
] at /usr/share/perl5/vendor_perl/MogileFS/NewHTTPFile.pm line 335.
# Tests were run but no plan was declared and done_testing() was not seen.
[Wed Mar 20 16:51:16 2013] ending run due to SIGTERM

that occur only in mock environment. There are some massive parallel tests and
SQLite is not capable to handle them probably. Other option is to use MySQL or
Postgres back-end, but AFAIK both are fuzzy in mock.

I'm going to disable the tests.

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #5 from Petr Pisar  ---
Updated package is available:
Spec URL:
http://ppisar.fedorapeople.org/perl-MogileFS-Server/perl-MogileFS-Server.spe...
SRPM URL:
http://ppisar.fedorapeople.org/perl-MogileFS-Server/perl-MogileFS-Server-2.6...

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #6 from Petr Pisar  ---
SRPM URL:
http://ppisar.fedorapeople.org/perl-MogileFS-Server/perl-MogileFS-Server-2.6...

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #7 from Petr Šabata  ---
Ok, better now.

Patches make sense, ok.

Majority of the files don't mention their license, there's no license on the
CPAN page and there's no LICENSE or COPYING file either.

mogilefsd is 'GPLv2 or artistic' +who-knows and mogstored is '(GPLv2 or
Artistic) and (GPL+ or Artistic)' +who-knows.  Backends have no license at all.
 I'm not sure if we may assume Perl-like license or if this is okay or not. 
Asking upstream for clarification would help.  Also, if it's possible to use a
different License tag for each subpackage, do that for mogilefsd.

You're missing a build-time dep, perl(MogileFS::Admin), required by various
tests.

Thanks to Patch0, perl(Fcntl) and perl(IO::AIO) are not required.  You may drop
those.

And finally, what's the reason for that 'exit 0' in your scriptlets?

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #8 from Petr Pisar  ---
(In reply to comment #7)
> Majority of the files don't mention their license, there's no license on the
> CPAN page and there's no LICENSE or COPYING file either.

There is an old home page http://code.google.com/p/mogilefs/ that refers the
whole project as `Artistic License/GPL'.
> mogilefsd is 'GPLv2 or artistic' +who-knows and mogstored is '(GPLv2 or
> Artistic) and (GPL+ or Artistic)' +who-knows.  Backends have no license at
> all.  I'm not sure if we may assume Perl-like license or if this is okay or
> not. Asking upstream for clarification would help.

I sent e-mail to the authors.
> And finally, what's the reason for that 'exit 0' in your scriptlets?

Scriptlets are not allowed to fail in general
https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Syntax.

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #9 from Petr Pisar  ---
I haven't received any response from the upstream.

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=915791

Jóhann B. Guðmundsson  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|---                         |WONTFIX
        Last Closed|                            |2014-05-26 06:00:29

--- Comment #10 from Jóhann B. Guðmundsson  ---
Given that I have left the project and a new individual may or may not continue
with systemd integration in the project by submitting new feature following
whatever demands FPC and FESCo might have and thus new units in the process I'm
closing this and all remaining bugs I had submitted for this particular feature
as WONTFIX

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=915791

Petr Pisar  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|CLOSED                      |ASSIGNED
         Resolution|WONTFIX                     |---
           Keywords|                            |Reopened

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=915791

Jóhann B. Guðmundsson  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|913251                      |

Referenced Bugs:

https://bugzilla.redhat.com/show_bug.cgi?id=913251
[Bug 913251] TRACKER: Remaining Systemd migration for Fedora

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #11 from Petr Šabata  ---
(In reply to Petr Pisar from comment #9)
> I haven't received any response from the upstream.

Still nothing?

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #12 from Petr Pisar  ---
No reply from upstream.

But there were three releases in the mean time and the latest one adds LICENSE
file http://cpansearch.perl.org/src/DORMANDO/MogileFS-Server-2.72/LICENSE
with this content:

License granted to use/distribute under the same terms as Perl itself.

Should I rebase the package, or are you fine with this statement from a future
release?

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #13 from Petr Šabata  ---
Just saying `fine' would be the easier way, however, it wouldn't feel right.

Please, rebase the package.  I'll re-review it.

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

[Red Hat Bugzilla]

https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #14 from Petr Pisar  ---
Spec URL:
http://ppisar.fedorapeople.org/perl-MogileFS-Server/perl-MogileFS-Server.spe...
SRPM URL:
https://ppisar.fedorapeople.org/perl-MogileFS-Server/perl-MogileFS-Server-2....

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

https://bugzilla.redhat.com/show_bug.cgi?id=915791

Petr Pisar  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |needinfo?([e-mail]
                   |                            |m)

--- Comment #15 from Petr Pisar  ---
Aby progress?

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

https://bugzilla.redhat.com/show_bug.cgi?id=915791

Petr Šabata  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|needinfo?([e-mail] |
                   |m)                          |

--- Comment #16 from Petr Šabata  ---
Sorry for the delay; I'll look into it & hopefully finish it sometime this
week.

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

https://bugzilla.redhat.com/show_bug.cgi?id=915791

Petr Pisar  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |upstream-release-monitoring
                   |                            |@fedoraproject.org

--- Comment #17 from Petr Pisar  ---
*** Bug 1439873 has been marked as a duplicate of this bug. ***

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #18 from Petr Šabata  ---
* Several of the listed build dependencies don't appear to be used at build
  time, including:
  - perl(DBD::SQLite)
  - perl(Fcntl)
  - perl(Perlbal)
  - perl(Perlbal::Socket)
  - perl(Perlbal::TCPListener)
  - net-tools

* You could use the NO_PACKLIST feature.

* mogstored is missing some runtime dependencies, namely:
  - perl(Mogstored::ChildProcess::DiskUsage)
  - perl(Mogstored::ChildProcess::IOStat)
  - perl(Pod::Usage)

* Does it make sense to have the None backend in a separate subpackage?

* To me, "Same terms as Perl itself.  Artistic/GPLv2, at your choosing"
  doesn't read as "(GPL+ or Artistic) and (GPLv2 or Artistic)" but more like
  "GPL+ or GPLv2 or Artistic".

* I think the rest is fine.

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #19 from Petr Pisar  ---
(In reply to Petr Šabata from comment #18)
> Several of the listed build dependencies don't appear to be used at build
> time, including:
> perl(DBD::SQLite)

This is used when running t/store-sqlite.t.
> perl(Fcntl)

Removed.
> perl(Perlbal)

Removed.
> perl(Perlbal::Socket)

Removed.
> perl(Perlbal::TCPListener)

Removed.
> net-tools

Removed.
> You could use the NO_PACKLIST feature.

Done.
> mogstored is missing some runtime dependencies, namely:
> perl(Mogstored::ChildProcess::DiskUsage)

Added.
> perl(Mogstored::ChildProcess::IOStat)

Added.
> perl(Pod::Usage)

Added.
> Does it make sense to have the None backend in a separate subpackage?

For the symmetry.
> To me, "Same terms as Perl itself.  Artistic/GPLv2, at your choosing"
> doesn't read as "(GPL+ or Artistic) and (GPLv2 or Artistic)" but more like
> "GPL+ or GPLv2 or Artistic".

I can see your point. My reading of the second sentence is this is an
explanation (a wrong one) of the first sentence.

Your reading can be simplified to "GPL+ or Artistic".
My reading can be simplified to "GPLv2 or Artistic".

I will use stricter "GPLv2 or Artistic" that conform to both readings and I
will try to ask the author.

Spec URL:
https://ppisar.fedorapeople.org/perl-MogileFS-Server/perl-MogileFS-Server.sp...
SRPM URL:
https://ppisar.fedorapeople.org/perl-MogileFS-Server/perl-MogileFS-Server-2....

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #20 from Petr Pisar  ---
I received a license clarification from the author. He say "GPL+ or Artistic"
it the right interpretation. Updated package is on the same address.

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

https://bugzilla.redhat.com/show_bug.cgi?id=915791

Petr Šabata  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|fedora-review?              |fedora-review+

--- Comment #21 from Petr Šabata  ---
(In reply to Petr Pisar from comment #19)
> (In reply to Petr Šabata from comment #18)
> > Several of the listed build dependencies don't appear to be used at build
> > time, including:
> > perl(DBD::SQLite)
>
> This is used when running t/store-sqlite.t.

Ok, I thought you weren't running the test, probably due to the comment
in %check.
> > perl(Fcntl)
>
> Removed.
> > perl(Perlbal)
>
> Removed.
> > perl(Perlbal::Socket)
>
> Removed.
> > perl(Perlbal::TCPListener)
>
> Removed.
> > net-tools
>
> Removed.

Ack.
> > You could use the NO_PACKLIST feature.
>
> Done.

You should also update the EE::MM dependency to require >= 6.76, which is
the version which introduced this feature.
> > mogstored is missing some runtime dependencies, namely:
> > perl(Mogstored::ChildProcess::DiskUsage)
>
> Added.
> > perl(Mogstored::ChildProcess::IOStat)
>
> Added.
> > perl(Pod::Usage)
>
> Added.

Ack.
> > Does it make sense to have the None backend in a separate subpackage?
>
> For the symmetry.

Ok.
> > To me, "Same terms as Perl itself.  Artistic/GPLv2, at your choosing"
> > doesn't read as "(GPL+ or Artistic) and (GPLv2 or Artistic)" but more like
> > "GPL+ or GPLv2 or Artistic".
>
> I can see your point. My reading of the second sentence is this is an
> explanation (a wrong one) of the first sentence.
>
> Your reading can be simplified to "GPL+ or Artistic".
> My reading can be simplified to "GPLv2 or Artistic".
>
> I will use stricter "GPLv2 or Artistic" that conform to both readings and I
> will try to ask the author.

Clarified with the next update.
> Spec URL:
> https://ppisar.fedorapeople.org/perl-MogileFS-Server/perl-MogileFS-Server.
> spec
> SRPM URL:
> https://ppisar.fedorapeople.org/perl-MogileFS-Server/perl-MogileFS-Server-2.
> 72-1.fc27.src.rpm

No blockers here.  Approving.

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

https://bugzilla.redhat.com/show_bug.cgi?id=915791

--- Comment #22 from Gwyn Ciesla  ---
Package request has been approved:
https://admin.fedoraproject.org/pkgdb/package/rpms/perl-MogileFS-Server

[Bug 915791] Review Request: perl-MogileFS-Server - Server part of the MogileFS distributed file system

https://bugzilla.redhat.com/show_bug.cgi?id=915791

Petr Pisar  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
   Fixed In Version|                            |perl-MogileFS-Server-2.72-1
                   |                            |.fc27
         Resolution|---                         |RAWHIDE
        Last Closed|2014-05-26 06:00:29         |2017-04-28 01:54:45

--- Comment #23 from Petr Pisar  ---
Thank you for the review and the repository. I added the version constraint.