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: ppisar@redhat.com
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.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915791
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |913251 CC| |johannbg@gmail.com
--- Comment #1 from Petr Pisar ppisar@redhat.com --- *** Bug 913493 has been marked as a duplicate of this bug. ***
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #2 from Petr Pisar ppisar@redhat.com --- *** Bug 720916 has been marked as a duplicate of this bug. ***
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915791
Petr Šabata psabata@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |psabata@redhat.com Assignee|nobody@fedoraproject.org |psabata@redhat.com Flags| |fedora-review?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #3 from Petr Šabata psabata@redhat.com --- 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...
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #4 from Petr Pisar ppisar@redhat.com --- 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.
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #5 from Petr Pisar ppisar@redhat.com --- 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...
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #6 from Petr Pisar ppisar@redhat.com --- SRPM URL: http://ppisar.fedorapeople.org/perl-MogileFS-Server/perl-MogileFS-Server-2.6...
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #7 from Petr Šabata psabata@redhat.com --- 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?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #8 from Petr Pisar ppisar@redhat.com --- (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.
https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #9 from Petr Pisar ppisar@redhat.com --- I haven't received any response from the upstream.
https://bugzilla.redhat.com/show_bug.cgi?id=915791
Jóhann B. Guðmundsson johannbg@gmail.com 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 johannbg@gmail.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=915791
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|CLOSED |ASSIGNED Resolution|WONTFIX |--- Keywords| |Reopened
https://bugzilla.redhat.com/show_bug.cgi?id=915791
Jóhann B. Guðmundsson johannbg@gmail.com 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
https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #11 from Petr Šabata psabata@redhat.com --- (In reply to Petr Pisar from comment #9)
I haven't received any response from the upstream.
Still nothing?
https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #12 from Petr Pisar ppisar@redhat.com --- 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?
https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #13 from Petr Šabata psabata@redhat.com --- Just saying `fine' would be the easier way, however, it wouldn't feel right.
Please, rebase the package. I'll re-review it.
https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #14 from Petr Pisar ppisar@redhat.com --- 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....
https://bugzilla.redhat.com/show_bug.cgi?id=915791
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(psabata@redhat.co | |m)
--- Comment #15 from Petr Pisar ppisar@redhat.com --- Aby progress?
https://bugzilla.redhat.com/show_bug.cgi?id=915791
Petr Šabata psabata@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(psabata@redhat.co | |m) |
--- Comment #16 from Petr Šabata psabata@redhat.com --- Sorry for the delay; I'll look into it & hopefully finish it sometime this week.
https://bugzilla.redhat.com/show_bug.cgi?id=915791
Petr Pisar ppisar@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |upstream-release-monitoring | |@fedoraproject.org
--- Comment #17 from Petr Pisar ppisar@redhat.com --- *** Bug 1439873 has been marked as a duplicate of this bug. ***
https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #18 from Petr Šabata psabata@redhat.com --- * 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.
https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #19 from Petr Pisar ppisar@redhat.com --- (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....
https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #20 from Petr Pisar ppisar@redhat.com --- I received a license clarification from the author. He say "GPL+ or Artistic" it the right interpretation. Updated package is on the same address.
https://bugzilla.redhat.com/show_bug.cgi?id=915791
Petr Šabata psabata@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #21 from Petr Šabata psabata@redhat.com --- (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.
https://bugzilla.redhat.com/show_bug.cgi?id=915791
--- Comment #22 from Gwyn Ciesla limburgher@gmail.com --- Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/perl-MogileFS-Server
https://bugzilla.redhat.com/show_bug.cgi?id=915791
Petr Pisar ppisar@redhat.com 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 ppisar@redhat.com --- Thank you for the review and the repository. I added the version constraint.
package-review@lists.fedoraproject.org