Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: recoll - Desktop full text search tool with a qt gui
https://bugzilla.redhat.com/show_bug.cgi?id=612719
Summary: Review Request: recoll - Desktop full text search tool with a qt gui Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: terjeros@phys.ntnu.no QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, fedora-package-review@redhat.com Estimated Hours: 0.0 Classification: Fedora
spec: http://terjeros.fedorapeople.org/recoll/recoll.spec srpm: http://terjeros.fedorapeople.org/recoll/recoll-1.13.04-3.fc12.src.rpm description: Recoll is a personal full text search package for Linux, FreeBSD and other Unix systems. It is based on a very strong backend (Xapian), for which it provides an easy to use, feature-rich, easy administration interface.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
Terje Røsten terjeros@phys.ntnu.no changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jfd@recoll.org
--- Comment #1 from Terje Røsten terjeros@phys.ntnu.no 2010-07-08 16:51:01 EDT --- *** Bug 590473 has been marked as a duplicate of this bug. ***
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
Ankur Sinha sanjay.ankur@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |sanjay.ankur@gmail.com AssignedTo|nobody@fedoraproject.org |sanjay.ankur@gmail.com Flag| |fedora-review?
--- Comment #2 from Ankur Sinha sanjay.ankur@gmail.com 2010-07-09 03:06:32 EDT --- A preliminary rpmlint check:
[Ankur@localhost SPECS]$ rpmlint recoll.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. [Ankur@localhost SPECS]$ rpmlint ../SRPMS/recoll-1.13.04-3.fc12.src.rpm recoll.src: W: spelling-error Summary(en_US) gui -> GUI, goo, gi recoll.src: W: spelling-error %description -l en_US backend -> backed, backbend, back end recoll.src: W: invalid-url URL: http://www.recoll.org/ <urlopen error timed out> 1 packages and 0 specfiles checked; 0 errors, 3 warnings.
Small issues, worth correcting though. I'll do a detailed review in the next 24 hours.
Ankur
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #3 from Terje Røsten terjeros@phys.ntnu.no 2010-07-09 12:06:09 EDT --- Thanks Ankur, updated package:
spec: http://terjeros.fedorapeople.org/recoll/recoll.spec srpm: http://terjeros.fedorapeople.org/recoll/recoll-1.13.04-4.fc12.src.rpm koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=2309343
koji is F13 as new gcc 4.5 in rawhide seems to have broken qt: http://koji.fedoraproject.org/koji/getfile?taskID=2308941&name=root.log
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #4 from Ankur Sinha sanjay.ankur@gmail.com 2010-07-09 14:42:28 EDT --- I was on my way to a formal review when I noticed some things that are worth clarifying..
wrt https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries
1. I see a unac directory with a "stripped down version of unac". You need to package unac separately and add it as a build requires IMO.
2. I also see xdg-utils bundled with the source. What is the purpose of this? You need to use the system wide package, not a local copy.
These are only two that I've found yet.
Will you please contact upstream and work these issues out? (and also check for other packages that have been used locally and provided with the tar)
regards, Ankur
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #5 from Terje Røsten terjeros@phys.ntnu.no 2010-07-09 19:28:56 EDT --- Ok, Ankur.
I will have a look, thanks for the help so far.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #6 from Jean-Francois Dockes jfd@recoll.org 2010-07-10 02:53:28 EDT ---
- I see a unac directory with a "stripped down version of unac". You need
to
package unac separately and add it as a build requires IMO.
Parts of unac are significantly modified for Recoll use. The comment is misleading, this is not just a "stripped down version of unac". I fixed the README (for the next version). Recoll could not use the standard package.
You can find the new README.recoll file at the following link. I hope this clarifies things: http://bitbucket.org/medoc/recoll/src/tip/unac/
- I also see xdg-utils bundled with the source. What is the purpose of
this?
You need to use the system wide package, not a local copy.
xdg-utils is bundled because it was found useful on some platform (don't remember which), probably because of a buggy system version. Recoll installs a copy of xdg-open to $prefix/share/recoll/filters/ If this is a problem, can it be removed by the rpm after the install step ? Recoll will use the system version then.
These are only two that I've found yet.
Will you please contact upstream and work these issues out? (and also check for other packages that have been used locally and provided with the tar)
There is also a partial and modified copy of binc imap. As far as I know this is not packaged on Fedora and not available as a library, so including the code is the only way to reuse it.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #7 from Terje Røsten terjeros@phys.ntnu.no 2010-07-10 08:14:51 EDT --- Updated package:
- use system xdg-open - trim changelog
spec: http://terjeros.fedorapeople.org/recoll/recoll.spec srpm: http://terjeros.fedorapeople.org/recoll/recoll-1.13.04-5.fc12.src.rpm
I don't see the unac issue as a blocker it' simple application with low risk, upstream seems dead too.
what is recoll using binc for? why is imap _server_ code needed in recoll?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #8 from Jean-Francois Dockes jfd@recoll.org 2010-07-10 10:07:34 EDT --- Recoll just includes the rfc 822 + mime analysis part. The code is both compact and proven robust (imap _server_). The daemon, network, and protocol parts are not in there. A few years ago no email lib that I looked at would fit the bill (either big and full of deps or unreliable/unproven). There has been a few modifications/fixes to the original code to fit Recoll needs, but less than in the unac part.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #9 from Ankur Sinha sanjay.ankur@gmail.com 2010-07-11 01:54:07 EDT --- (In reply to comment #7)
Updated package:
- use system xdg-open
- trim changelog
spec: http://terjeros.fedorapeople.org/recoll/recoll.spec srpm: http://terjeros.fedorapeople.org/recoll/recoll-1.13.04-5.fc12.src.rpm
I don't see the unac issue as a blocker it' simple application with low risk, upstream seems dead too.
what is recoll using binc for? why is imap _server_ code needed in recoll?
hi,
I don't think the unac issue is a blocker, but I will confirm with the devel list first. I'll ping back when I get something.
regards, Ankur
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #10 from Ankur Sinha sanjay.ankur@gmail.com 2010-07-13 07:48:48 EDT --- hi,
I got this from -devel
I think it would be best to talk to upstream of this project and see if there is any chance of them upstreaming any of their patches for these projects. If not, then getting to admit that they are forking them and only using them internally would at least be good to know.
It looks like it's okay. Formal review coming up!
+ OK ? ISSUE - NA
=================================================================================
+ Package meets naming and packaging guidelines + Spec file matches base package name. + Spec has consistant macro usage. + Meets Packaging Guidelines. + License + License field in spec matches + License file included in package + Spec in American English + Spec is legible. + Sources match upstream md5sum: [Ankur@localhost rpmbuild]$ md5sum recoll-1.13.04.tar.gz SOURCES/recoll-1.13.04.tar.gz eba9d639dbac149996a92b9b2dffc5ee recoll-1.13.04.tar.gz eba9d639dbac149996a92b9b2dffc5ee SOURCES/recoll-1.13.04.tar.gz
- Package needs ExcludeArch + BuildRequires correct - Spec handles locales/find_lang - Package is relocatable and has a reason to be. + Package has %defattr and permissions on files is good. + Package has a correct %clean section. + Package has correct buildroot %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) + Package is code or permissible content. - Doc subpackage needed/used. + Packages %doc files don't affect runtime.
- Headers/static libs in -devel subpackage. - Spec has needed ldconfig in post and postun - .pc files in -devel subpackage/requires pkgconfig - .so files in -devel subpackage. - -devel package Requires: %{name} = %{version}-%{release} - .la files are removed.
+ Package is a GUI app and has a .desktop file
? Package compiles and builds on at least one arch. + Package has no duplicate files in %files. + Package doesn't own any directories other packages own. + Package owns all the directories it creates. - No rpmlint output. - final provides and requires are sane: (include output of for i in *rpm; do echo $i; rpm -qp --provides $i; echo =; rpm -qp --requires $i; echo; done manually indented after checking each line. I also remove the rpmlib junk and anything provided by glibc.)
SHOULD Items:
? Should build in mock. ? Should build on all supported archs ? Should function as described. + Should have sane scriptlets. - Should have subpackages require base package with fully versioned depend. + Should have dist tag + Should package latest version - check for outstanding bugs on package. (For core merge reviews)
Issues:
1. I wasn't able to build it for rawhide. You'll have to correct this, since the first branch you'll build in will be *rawhide*.
2. I haven't been able to build it correctly, and so haven't tested it's working.
No rpmlint output on the spec and the source.
A few issues, but should be small enough to quickly be solved.
Ankur
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #11 from Jean-Francois Dockes jfd@recoll.org 2010-07-14 16:51:58 EDT --- To correct the build failure on rawhide: recoll needs a small patch (which fixes some include files) to build with the new xapian 1.2: http://www.lesbonscomptes.com/recoll/files/xapian12.patch
It then works for me (at least build, indexing and command line query, couldn't test the GUI as my rawhide's X11 seems to be broken).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #12 from Terje Røsten terjeros@phys.ntnu.no 2010-07-14 16:57:56 EDT --- Thanks Jegan-Francois,
is the patch rawhide only or does it work for F-12 and F-13 too?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #13 from Jean-Francois Dockes jfd@recoll.org 2010-07-14 17:13:50 EDT --- It's necessary to compile with the latest Xapian, but doesn't break compatibility with older Xapian versions, so you can use it everywhere.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #14 from Terje Røsten terjeros@phys.ntnu.no 2010-07-15 04:38:45 EDT --- Updated package: - Add patch to build with xapian 1.2
spec: http://terjeros.fedorapeople.org/recoll/recoll.spec srpm: http://terjeros.fedorapeople.org/recoll/recoll-1.13.04-6.fc12.src.rpm koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=2321442
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #15 from Ankur Sinha sanjay.ankur@gmail.com 2010-07-31 08:50:58 EDT --- Okay,
Builds with mock.
I do recall some discussion about the code reuse, Jean, can you please update us on it? Do we need a FESCO exception etc. here?
If not, I'll approve the package.
Ankur
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #16 from Jean-Francois Dockes jfd@recoll.org 2010-08-05 17:33:57 EDT --- The exception for recoll library bundling (which I'll persist in calling code reuse :) ) was granted by Fesco on july 20:
http://lists.fedoraproject.org/pipermail/devel/2010-July/139191.html
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
Ankur Sinha sanjay.ankur@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #17 from Ankur Sinha sanjay.ankur@gmail.com 2010-08-07 10:04:10 EDT --- Ah,
I've been a little off then :P
XXX APPROVED XXX
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
Terje Røsten terjeros@phys.ntnu.no changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #18 from Terje Røsten terjeros@phys.ntnu.no 2010-08-09 07:59:17 EDT --- Thanks guys!
New Package SCM Request ======================= Package Name: recoll Short Description: Desktop full text search tool with Qt GUI Owners: terjeros Branches: F13 F14 InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #19 from Kevin Fenzi kevin@tummy.com 2010-08-09 13:24:44 EDT --- Git done (by process-git-requests).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
Terje Røsten terjeros@phys.ntnu.no changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs+ |fedora-cvs?
--- Comment #20 from Terje Røsten terjeros@phys.ntnu.no 2010-08-09 14:46:00 EDT ---
Sorry, forgot F12 branch.
New Package SCM Request ======================= Package Name: recoll Short Description: Desktop full text search tool with Qt GUI Owners: terjeros Branches: F12 InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #21 from Jason Tibbitts tibbs@math.uh.edu 2010-08-09 14:56:54 EDT --- You should file a change request if the package already exists.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #22 from Terje Røsten terjeros@phys.ntnu.no 2010-08-10 02:59:11 EDT --- Thanks for the tip, Jason.
Package Change Request ====================== Package Name: recoll New Branches: F12 Owners: terjeros InitialCC:
Forgot F12 branch in the initial request.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #23 from Kevin Fenzi kevin@tummy.com 2010-08-10 13:56:33 EDT --- Git done (by process-git-requests).
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
Terje Røsten terjeros@phys.ntnu.no changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |RAWHIDE
--- Comment #24 from Terje Røsten terjeros@phys.ntnu.no 2010-08-10 16:30:47 EDT --- Thanks again, Kevin.
recoll imported, built and pushed for all active branches.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
Paolo Bonzini pbonzini@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |pbonzini@redhat.com Flag|fedora-cvs+ |fedora-cvs?
--- Comment #25 from Paolo Bonzini pbonzini@redhat.com 2011-12-19 05:36:54 EST ---
Package Change Request ====================== Package Name: recoll New Branches: el6 Owners: bonzini InitialCC:
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=612719
--- Comment #26 from Jon Ciesla limburgher@gmail.com 2011-12-19 08:01:33 EST --- Git done (by process-git-requests).
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla