Bug 484323 - Review Request: perl-KinoSearch - Search engine library
Review Request: perl-KinoSearch - Search engine library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Lubomir Rintel
Fedora Extras Quality Assurance
:
Depends On: 484320 484321
Blocks:
  Show dependency treegraph
 
Reported: 2009-02-05 20:31 EST by Ian Burrell
Modified: 2009-03-30 16:17 EDT (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-03-30 16:17:02 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
lkundrak: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Ian Burrell 2009-02-05 20:31:35 EST
Spec URL: http://znark.com/fedora/perl-KinoSearch.spec
SRPM URL: http://znark.com/fedora/perl-KinoSearch-0.163-1.fc10.src.rpm
Description: KinoSearch is a loose port of the Java search engine library Apache Lucene, written in Perl and C. The archetypal application is website search, but it can be put to many different uses.
Comment 1 Lubomir Rintel 2009-02-09 09:58:30 EST
Hah, I just need this todaym Ian, I owe you a beer.
Taking it for a review.
Comment 2 Lubomir Rintel 2009-02-09 10:23:38 EST
0.) This is redundant:
Requires:       perl(Compress::Zlib)
It gets added by the auroreq generator.

1.) rpmlint:

perl-KinoSearch.i386: E: zero-length /usr/lib/perl5/vendor_perl/5.8.8/i386-linux-thread-multi/auto/KinoSearch/KinoSearch.bs

2.) License:

I'm worried about this, and am blocking the review until it is solved. The perldoc reads:

       Terms of usage for Apache Lucene, from which portions of KinoSearch are derived, are spelled out in the Apache License: see the file "ApacheLicense2.0.txt".

While the original code is licensed "GPL+ or Artistic". If my understanding of things is correct, we can't choose "GPL" since it does not permit redistribution when linked with ASL code and we can't choose Artistic either, since it's incompatible with Fedora. I think a clarification and eventually and addition of exception to allow linking with ASL from upstream would be good here.

IANAL, I might be completely wrong. Adding spot to CC, he may provide valuable advice.

In other respects, the package is perfect:
- SPEC file clean and legible
- Builds in mock, obeys compiler flags
- rpmlint is mostly quiet (see 1.)
- requires/provides (mostly, see 0.) sane
- ASL license 2.0 text included (heh...)
Comment 3 Chris Weyl 2009-02-09 16:14:35 EST
w.r.t. license, my take is: 

Original bits are under "GPL+ or Artistic" (perl), derived bits under the ASL2.0;  the Fedora list of good license indicates that ASL2.0 is compatible with the terms of the GPLv3, so we should redistribute the entire thing under GPLv3+ (since GPL+ -> GPLv3+ w/o problem).
Comment 4 Ian Burrell 2009-02-09 17:25:44 EST
I am pretty sure that none of the code is under ASL 2.0.  The Perl code is derived from Lucene (which is in Java) but it is a rewrite (and not even a straight port like Plucene).  The ASL 2.0 in section 4 allows derivative works to have separate copyright as long as the conditions are met.  Including the ASL license and the notice in the docs is one of the conditions.
Comment 5 Lubomir Rintel 2009-02-10 02:02:35 EST
(In reply to comment #3)
> w.r.t. license, my take is: 
> 
> Original bits are under "GPL+ or Artistic" (perl), derived bits under the
> ASL2.0;  the Fedora list of good license indicates that ASL2.0 is compatible
> with the terms of the GPLv3, so we should redistribute the entire thing under
> GPLv3+ (since GPL+ -> GPLv3+ w/o problem).

Mostly right. Yes. I don't know why I thought ASL2.0 is incompatible with all GPL versions. I was even looking at the compatibility matrix and did not notice that. One small correction would be that the resulting license is "GPLv3" not "GPLv3+", since we can't tell whether ASL is compatible with license that doesn't even exist.

Much thanks for pointing this out Chris, I've obviously not have figured that myself and it clarifies things a lot.

(In reply to comment #4)
> I am pretty sure that none of the code is under ASL 2.0.  The Perl code is
> derived from Lucene (which is in Java) but it is a rewrite (and not even a
> straight port like Plucene).  The ASL 2.0 in section 4 allows derivative works
> to have separate copyright as long as the conditions are met.  Including the
> ASL license and the notice in the docs is one of the conditions.

I've really looked at the code yesterday and haven't found anything copied from lucene, so my feelings are that the ASL license is here "just in case".

So my recommendations here are either of
1.) Set the License to "GPLv3" and comment appropriately
2.) Ensure (check with upstream) there's no ASL code, remove the ASL license file and leave License set to "GPL+ or Artistic" (recommended, but don't do that w/o contacting upstream)

This can't block a review:

APPROVED

Please address the points 0.) and 1.) of comment #2 upon import.
(I'll check! :)
Comment 6 Ian Burrell 2009-02-11 19:55:19 EST
New Package CVS Request
=======================
Package Name: perl-KinoSearch
Short Description: Search engine library
Owners: iburrell
Branches: F-9 F-10 EL-5
InitialCC: perl-sig
Comment 7 Tom "spot" Callaway 2009-02-12 13:25:30 EST
I'm fine with Lubomir's assessment in Comment #5. Lifting FE-Legal.
Comment 8 Kevin Fenzi 2009-02-13 01:42:44 EST
cvs done.
Comment 9 Lubomir Rintel 2009-02-19 10:48:46 EST
Ian: I see you've attempted the builds, but still haven't clarified the situation with the License.

Your build failed on ppc, please don't trigger any more builds until the problem with license is resolved.
Comment 10 Lubomir Rintel 2009-02-26 21:18:58 EST
Ping?
Comment 11 Kevin Fenzi 2009-02-27 11:18:53 EST
Ian: Can we please make sure the license is right before doing builds?

Did you query upstream about the ASL issue? what was the reply?

Please don't build until this is all figured out.
Comment 12 Lubomir Rintel 2009-03-22 17:04:02 EDT
I've sent a mail upstream myself today.
Comment 13 Robert Scheck 2009-03-29 07:40:06 EDT
What's the issue with licensing? As per comment #7 our Fedora Legal seems to
be already fine. Which other issues are there now?

Currently, the packages are failing on ppc and ppc64 (endian things?), maybe
we can just ExcludeArch (or even fix) them?

Anyway I'm suggesting the upgrade to 0.164 as well as adding two missing build
requirements for %check section.

Index: perl-KinoSearch.spec
===================================================================
RCS file: /cvs/pkgs/rpms/perl-KinoSearch/devel/perl-KinoSearch.spec,v
retrieving revision 1.4
diff -u -r1.4 perl-KinoSearch.spec
--- perl-KinoSearch.spec        26 Feb 2009 20:33:09 -0000      1.4
+++ perl-KinoSearch.spec        29 Mar 2009 11:36:30 -0000
@@ -1,6 +1,6 @@
 Name:           perl-KinoSearch
-Version:        0.163
-Release:        4%{?dist}
+Version:        0.164
+Release:        1%{?dist}
 Summary:        Search engine library
 License:        GPL+ or Artistic
 Group:          Development/Libraries
@@ -13,6 +13,8 @@
 BuildRequires:  perl(Lingua::Stem::Snowball) >= 0.94
 BuildRequires:  perl(Lingua::StopWords) >= 0.02
 BuildRequires:  perl(Module::Build)
+BuildRequires:  perl(Test::Pod::Coverage) >= 1.04
+BuildRequires:  perl(Test::Pod) >= 1.14
 Requires:       perl(Compress::Zlib)
 Requires:       perl(Lingua::Stem::Snowball) >= 0.94
 Requires:       perl(Lingua::StopWords) >= 0.02


+ ./Build test
t/000-load.t .................. ok
t/001-build_invindexes.t ...... ok
t/002-kinosearch.t ............ ok
t/010-verify_args.t ........... ok
t/011-class.t ................. ok
t/012-priority_queue.t ........ ok
t/013-bit_vector.t ............ ok
t/015-sort_external.t ......... ok
#   Failed test 'signed byte'
#   at t/101-simple_template_io.t line 21.
#     Structures begin differing at:
#          $got->[0] = '128'
#     $expected->[0] = '-128'
# Looks like you failed 1 test of 13.
t/101-simple_template_io.t .... 
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/13 subtests 
t/102-strings_template_io.t ... ok
#   Failed test 'b2'
#   at t/103-repeats_template_io.t line 20.
#     Structures begin differing at:
#          $got->[0] = '128'
#     $expected->[0] = '-128'
#   Failed test 'b131'
#   at t/103-repeats_template_io.t line 20.
#     Structures begin differing at:
#          $got->[0] = '128'
#     $expected->[0] = '-128'
#   Failed test 'b149'
#   at t/103-repeats_template_io.t line 20.
#     Structures begin differing at:
#          $got->[0] = '128'
#     $expected->[0] = '-128'
#   Failed test 'b256'
#   at t/103-repeats_template_io.t line 20.
#     Structures begin differing at:
#          $got->[0] = '128'
#     $expected->[0] = '-128'
# Looks like you failed 4 tests of 18.
t/103-repeats_template_io.t ... 
Dubious, test returned 4 (wstat 1024, 0x400)
Failed 4/18 subtests 
t/104-parse_template_io.t ..... ok
t/105-invindex.t .............. ok
Lockfile looks dead - removing at /builddir/build/BUILD/KinoSearch-0.164/blib/lib/KinoSearch/Store/FSLock.pm line 40.
t/106-locking.t ............... ok
t/150-polyanalyzer.t .......... ok
t/152-token_batch.t ........... ok
t/153-lc_normalizer.t ......... ok
t/154-tokenizer.t ............. ok
t/155-stopalizer.t ............ ok
t/201-field_infos.t ........... ok
t/202-term.t .................. ok
t/203-compound_file_reader.t .. ok
t/204-fields_reader.t ......... ok
t/205-seg_reader.t ............ ok
t/206-seg_infos.t ............. ok
t/207-seg_term_enum.t ......... ok
t/208-terminfo.t .............. ok
t/209-term_infos_reader.t ..... ok
t/210-deldocs.t ............... ok
t/211-seg_term_docs.t ......... ok
t/212-multi_term_docs.t ....... ok
t/213-segment_merging.t ....... ok
t/214-spec_field.t ............ ok
t/302-many_fields.t ........... ok
t/303-highlighter.t ........... ok
t/501-termquery.t ............. ok
t/502-phrasequery.t ........... ok
t/503-booleanquery.t .......... ok
t/504-similarity.t ............ ok
t/505-hit_queue.t ............. ok
t/506-hit_collector.t ......... ok
t/507-query_filter.t .......... ok
t/508-hits.t .................. ok
t/509-multi_searcher.t ........ ok
t/510-remote_search.t ......... ok
t/601-queryparser.t ........... ok
t/602-boosts.t ................ ok
t/603-query_boosts.t .......... ok
t/604-simple_search.t ......... ok
t/701-uscon.t ................. ok
t/999-remove_invindexes.t ..... ok
t/pod-coverage.t .............. skipped: Only run during development
t/pod.t ....................... ok
Test Summary Report
-------------------
t/101-simple_template_io.t  (Wstat: 256 Tests: 13 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
t/103-repeats_template_io.t (Wstat: 1024 Tests: 18 Failed: 4)
  Failed tests:  2, 4, 6, 8
  Non-zero exit status: 4
Files=53, Tests=752, 20 wallclock secs ( 0.40 usr  0.13 sys + 18.67 cusr  1.28 csys = 20.48 CPU)
Result: FAIL
Failed 2/53 test programs. 5/752 subtests failed.
Comment 14 Kevin Fenzi 2009-03-29 13:16:17 EDT
>What's the issue with licensing? As per comment #7 our Fedora Legal seems to
>be already fine. Which other issues are there now?

See comment #5 item 2. 

The License in the spec is currently "GPL+ or Artistic", this MUST be confirmed with upstream before we know ASL doesn't matter here.
Comment 15 Lubomir Rintel 2009-03-30 16:17:02 EDT
Confirmed with upstream, clarified in the package.
Fixed on PowerPC.

I'm a bit worried about the package, since Ian doesn't respond (hope he's doing well), but let's assume perl-sig in watchbugzilla is enough to ensure the package is being cared about.

Note You need to log in before you can comment on or make changes to this bug.