Bug 1062542 - Review Request: libdatrie - Implementation of Double-Array structure for representing trie
Summary: Review Request: libdatrie - Implementation of Double-Array structure for repr...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dominik 'Rathann' Mierzejewski
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 733925 (view as bug list)
Depends On:
Blocks: 1062540
TreeView+ depends on / blocked
 
Reported: 2014-02-07 08:47 UTC by Christopher Meng
Modified: 2014-07-14 00:53 UTC (History)
5 users (show)

Fixed In Version: libdatrie-0.2.8-4.fc20
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-07-14 00:53:27 UTC
Type: ---
Embargoed:
dominik: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Christopher Meng 2014-02-07 08:47:50 UTC
Spec URL: http://cicku.me/libdatrie.spec
SRPM URL: http://cicku.me/libdatrie-0.2.8-1.fc21.src.rpm
Description: datrie is an implementation of double-array structure for representing trie, as proposed by Junichi Aoe.

Trie is a kind of digital search tree, an efficient indexing method with O(1) 
time complexity for searching. Comparably as efficient as hashing, trie also 
provides flexibility on incremental matching and key spelling manipulation. 
This makes it ideal for lexical analyzers, as well as spelling dictionaries.
Fedora Account System Username: cicku

Comment 1 Christopher Meng 2014-02-07 08:51:45 UTC
*** Bug 733925 has been marked as a duplicate of this bug. ***

Comment 2 Michael Schwendt 2014-02-07 15:50:22 UTC
%check
make check

is missing.


> %files devel
> %doc AUTHORS ChangeLog NEWS README*
> %doc %{_docdir}/%{name}-devel

This causes "File listed twice" warnings on F20 or newer due to the unversioned docdirs feature. The line that includes the local %doc files also includes anything in its docdir (here %{_docdir}/%{name}-devel).
https://fedorahosted.org/fpc/ticket/338

Installing into a temporary directory --with-html-docdir=$(pwd)/something would be one work-around, because then you could use %doc magic to include the local tree.


> %doc %{_docdir}/%{name}-devel

Anything below %_docdir is marked %doc implicitly (rpm -E %__docdir_path).


> %files utils
> %{_bindir}/trietool-0.2
> %{_mandir}/man1/trietool-0.2.1*

What's the reason this developer tool is put into a separate package?

Comment 3 Christopher Meng 2014-02-08 01:02:23 UTC
Fixed:

Spec URL: http://cicku.me/libdatrie.spec
SRPM URL: http://cicku.me/libdatrie-0.2.8-2.fc21.src.rpm

Comment 4 Dominik 'Rathann' Mierzejewski 2014-02-23 17:01:09 UTC
rpmlint found rpath in trietool:

libdatrie-devel.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/trietool-0.2 ['/usr/lib64']

Out of curiosity, do you know why it's suffixed with the version number?

Otherwise it looks fine. Please fix the rpath and I'll approve.

Comment 5 Christopher Meng 2014-02-23 17:06:12 UTC
(In reply to Dominik 'Rathann' Mierzejewski from comment #4)
> rpmlint found rpath in trietool:
> 
> libdatrie-devel.x86_64: E: binary-or-shlib-defines-rpath
> /usr/bin/trietool-0.2 ['/usr/lib64']
> 
> Out of curiosity, do you know why it's suffixed with the version number?
> 
> Otherwise it looks fine. Please fix the rpath and I'll approve.

Haha I know. I've had same question in my mind when I packaged this package, and I tell upstream and he said that it was intended for API bump, but now it seems it's useless and confusing, he will remove it later maybe.

On Fri, Feb 7, 2014 at 3:46 PM, Christopher Meng <cickumqt@xxxxxxx> wrote:

> Can you rename trietool-0.2 to trietool?
>
> Why 0.2 suffixed?

Because when releasing 0.2.0, it broke ABI with 0.1.x,
and both versions were expected to be installed together.

Now that libdatrie 0.1.x has been obsolete for a long time,
I'm considering dropping the suffix, with backward compatibility
in mind.

Comment 6 Christopher Meng 2014-02-23 17:42:43 UTC
Spec URL: http://cicku.me/libdatrie.spec
SRPM URL: http://cicku.me/libdatrie-0.2.8-3.fc21.src.rpm

Comment 7 Dominik 'Rathann' Mierzejewski 2014-02-24 13:37:21 UTC
Unfortunately, the rpath is still there:

[...]
INFO: Done(build/RPMS/libdatrie-0.2.8-3.fc20.src.rpm) Config(fedora-rawhide-x86_64) 0 minutes 36 seconds
INFO: Results and/or logs in: /var/lib/mock/fedora-rawhide-x86_64/result
Finish: run
$ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result
[...]
libdatrie-devel.x86_64: E: binary-or-shlib-defines-rpath /usr/bin/trietool-0.2 ['/usr/lib64']

Comment 8 Daiki Ueno 2014-03-26 06:32:29 UTC
Any update?

Comment 9 Michael Schwendt 2014-04-01 12:10:46 UTC
> http://cicku.me/libdatrie.spec

| Not Found
| 
| The requested URL /libdatrie.spec was not found on this server.

Comment 10 Christopher Meng 2014-05-22 09:40:22 UTC
Hi,

I hacked and finished:

SPEC URL: http://misc.cicku.me/fedora/libdatrie.spec
SRPM URL: http://misc.cicku.me/fedora/libdatrie-0.2.8-3.fc21.src.rpm

Comment 11 Ralf Corsepius 2014-05-27 06:30:21 UTC
I hate packages, which install docs into a different directory but /usr/share/doc/<package>. These days, there isn't any reason for not doing so.

All you need to do is to install these docs just like normal files and to reference them as normal files. 
This basically boils donw to not use %doc <relative path> in %files but to use
%{_pkgdocdir}/....

Comment 12 Christopher Meng 2014-06-18 10:51:48 UTC
(In reply to Ralf Corsepius from comment #11)
> I hate packages, which install docs into a different directory but
> /usr/share/doc/<package>. These days, there isn't any reason for not doing
> so.
> 
> All you need to do is to install these docs just like normal files and to
> reference them as normal files. 
> This basically boils donw to not use %doc <relative path> in %files but to
> use
> %{_pkgdocdir}/....

It's not a big problem, though. I've addressed the issue.

Due to bad Internet connection in China now, I've uploaded the SRPM to cloud disk(not publicly accessible):

NEW SRPM URL: https://mega.co.nz/#!yIZxlIoJ!b7n-scDAJfEa3tsBZbJ9Gi7zdNJRpyQJCnbljY_ZviM

Comment 13 Dominik 'Rathann' Mierzejewski 2014-06-18 22:42:38 UTC
rpath issue is gone, good.

APPROVED.

Comment 14 Dominik 'Rathann' Mierzejewski 2014-06-18 22:46:06 UTC
Scratch that.
With the 

%files devel
%{_pkgdocdir}/

change, both libdatrie and libdatrie-devel hold all doc files now, which I don't think was intended. Please fix the doc file duplication.

Comment 15 Christopher Meng 2014-06-20 08:33:53 UTC
(In reply to Dominik 'Rathann' Mierzejewski from comment #14)
> Scratch that.
> With the 
> 
> %files devel
> %{_pkgdocdir}/
> 
> change, both libdatrie and libdatrie-devel hold all doc files now, which I
> don't think was intended. Please fix the doc file duplication.

I've rearranged the %doc:

https://mega.co.nz/#!CIxggKRK!r7F7_3cZhOPya9OXOsFqKLTah1tv0H53k-sfHYMgNC0

Comment 16 Dominik 'Rathann' Mierzejewski 2014-06-24 08:18:47 UTC
So you did, but now all the API docs are in the main package instead of -devel. I don't think that was intended, either. You can use the _pkgdocdir macro like Ralf suggested, but you must %exclude the files that should be in the -devel package in %files section for the main package.

For example, like this (I'd keep AUTHORS COPYING ChangeLog NEWS README in the main package):

%files
%doc AUTHORS COPYING ChangeLog NEWS README
%exclude %{_pkgdocdir}/*.html
%exclude %{_pkgdocdir}/*.png
%exclude %{_pkgdocdir}/*.js
%exclude %{_pkgdocdir}/*.css
%exclude %{_pkgdocdir}/README.migration
%{_libdir}/libdatrie.so.*

%files devel
%{_pkgdocdir}/*.html
%{_pkgdocdir}/*.png
%{_pkgdocdir}/*.js
%{_pkgdocdir}/*.css
%{_pkgdocdir}/README.migration
%{_includedir}/datrie/
%{_libdir}/libdatrie.so
%{_libdir}/pkgconfig/datrie-0.2.pc
%{_bindir}/trietool*
%{_mandir}/man1/trietool*.1*

Comment 17 Christopher Meng 2014-06-24 11:43:32 UTC
No sweat, I understand your logic now:

Here is my new %files section:

%doc COPYING
%{_libdir}/libdatrie.so.*
%exclude %{_pkgdocdir}/*.css
%exclude %{_pkgdocdir}/*.html
%exclude %{_pkgdocdir}/*.js
%exclude %{_pkgdocdir}/*.png

%files devel
%doc AUTHORS ChangeLog NEWS README*
%{_pkgdocdir}/*.css
%{_pkgdocdir}/*.html
%{_pkgdocdir}/*.js
%{_pkgdocdir}/*.png
%{_includedir}/datrie/
%{_libdir}/libdatrie.so
%{_libdir}/pkgconfig/datrie-0.2.pc
%{_bindir}/trietool*

Note that I don't want to put useless files for end users in main-pkg, I hope you can understand. Thus only the license file packaged in main-pkg.

Is it OK? If so please set + on this review, thanks in advance.

Comment 18 Dominik 'Rathann' Mierzejewski 2014-06-24 11:51:48 UTC
(In reply to Christopher Meng from comment #17)
> No sweat, I understand your logic now:
> 
> Here is my new %files section:
> 
> %doc COPYING
> %{_libdir}/libdatrie.so.*
> %exclude %{_pkgdocdir}/*.css
> %exclude %{_pkgdocdir}/*.html
> %exclude %{_pkgdocdir}/*.js
> %exclude %{_pkgdocdir}/*.png
> 
> %files devel
> %doc AUTHORS ChangeLog NEWS README*
> %{_pkgdocdir}/*.css
> %{_pkgdocdir}/*.html
> %{_pkgdocdir}/*.js
> %{_pkgdocdir}/*.png
> %{_includedir}/datrie/
> %{_libdir}/libdatrie.so
> %{_libdir}/pkgconfig/datrie-0.2.pc
> %{_bindir}/trietool*
> 
> Note that I don't want to put useless files for end users in main-pkg, I
> hope you can understand. Thus only the license file packaged in main-pkg.

That is OK. I personally like to read ChangeLogs and READMEs from time to time, but that's not a review blocker.

> Is it OK? If so please set + on this review, thanks in advance.

Not quite. The %doc macro in -devel subpackage will put the listed files
in /usr/share/doc/libdatrie-devel instead of libdatrie. If you're putting the HTML docs from -devel in /usr/share/doc/libdatrie, you should be consistent and put all the docs there.

Comment 19 Christopher Meng 2014-07-02 10:27:10 UTC
NEW SPEC URL: http://us-la.cicku.me/libdatrie.spec
NEW SRPM URL: http://us-la.cicku.me/libdatrie-0.2.8-4.fc21.src.rpm

Comment 20 Dominik 'Rathann' Mierzejewski 2014-07-03 04:52:19 UTC
Nice and smart fix. Package APPROVED.

Comment 21 Christopher Meng 2014-07-03 05:03:16 UTC
Thanks......

New Package SCM Request
=======================
Package Name: libdatrie
Short Description: Implementation of Double-Array structure for representing trie
Upstream URL: http://linux.thai.net/projects/datrie
Owners: cicku ueno
Branches: f20

Comment 22 Gwyn Ciesla 2014-07-03 12:00:11 UTC
Git done (by process-git-requests).

Comment 23 Fedora Update System 2014-07-03 12:27:30 UTC
libdatrie-0.2.8-4.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/libdatrie-0.2.8-4.fc20

Comment 24 Fedora Update System 2014-07-04 00:32:17 UTC
libdatrie-0.2.8-4.fc20 has been pushed to the Fedora 20 testing repository.

Comment 25 Fedora Update System 2014-07-14 00:53:27 UTC
libdatrie-0.2.8-4.fc20 has been pushed to the Fedora 20 stable repository.


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