Bug 565616 - Review Request: libeina - Data Type Library
Summary: Review Request: libeina - Data Type Library
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Christoph Wickert
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-02-15 18:42 UTC by Thomas Janssen
Modified: 2010-03-12 04:30 UTC (History)
2 users (show)

Fixed In Version: libeina-0.9.9.063-6.fc11
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-02-28 14:44:19 UTC
cwickert: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)

Description Thomas Janssen 2010-02-15 18:42:44 UTC
Spec URL: http://thomasj.fedorapeople.org/reviews/eina-0.spec
SRPM URL: http://thomasj.fedorapeople.org/reviews/eina-0-0.9.9.063-1.fc12.src.rpm
Description: Eina is a data type library.

Comment 1 Thomas Janssen 2010-02-15 18:43:52 UTC
Forgot:

Builds in mock.

[thomas@tusdell SPECS]$ rpmlint eina-0.spec ../SRPMS/eina-0-0.9.9.063-1.fc12.src.rpm ../RPMS/x86_64/eina-0-*
eina-0-devel.x86_64: W: no-documentation
4 packages and 1 specfiles checked; 0 errors, 1 warnings.

Comment 2 Thomas Janssen 2010-02-15 19:26:24 UTC
Forgot another thing..

Two reasons why i named it eina-0. First there's already a package eina in fedora (a music player). So i had to rename it and eet (the current version) searches for a package eina-0.

Comment 3 Thomas Janssen 2010-02-15 21:50:41 UTC
This is not my day.. Added the forgotten man pages.

http://thomasj.fedorapeople.org/reviews/eina-0-0.9.9.063-2.fc12.src.rpm
http://thomasj.fedorapeople.org/reviews/eina-0.spec

Comment 4 Thomas Janssen 2010-02-19 14:20:49 UTC
Spec fixes and name change as discussed in IRC.

http://thomasj.fedorapeople.org/reviews/libeina.spec
http://thomasj.fedorapeople.org/reviews/libeina-0.9.9.063-3.fc12.src.rpm

Comment 5 Christoph Wickert 2010-02-21 11:14:11 UTC
Review for 5d9be61bb47fa6abecdb518e50855f76  libeina-0.9.9.063-3.fc12.src.rpm


TBD - MUST: rpmlint must be run on every package. The output should be posted in the review.
OK - MUST: named according to the Package Naming Guidelines (note: the package was renamed to libeina to avoid the naming conflict with the media player eina)
OK - MUST: spec file name matches the base package %{name}
OK - MUST: package meets the Packaging Guidelines
TBD - MUST: Fedora approved license and meets the Licensing Guidelines
OK - MUST: License field in spec file matches the actual license
OK - MUST: license file included in %doc
OK - MUST: spec is in American English
OK - MUST: spec is legible
OK - MUST: sources match the upstream source by MD5 1289ac1d6b1a7a578158f9ff24301f3f
FIX - MUST: does not successfully compile:

+ find doc/man/man3 -size -100c -delete
find: `doc/man/man3': No such file or directory

OK - MUST: If the package does not successfully compile, build or work on an architecture, then those architectures should be listed in the spec in ExcludeArch.
TBD - MUST: all build dependencies are listed in BuildRequires.
N/A - MUST: handles locales properly with %find_lang
OK - MUST: Every binary RPM package (or subpackage) which stores shared library files (not just symlinks) in any of the dynamic linker's default paths, must call ldconfig in %post and %postun.
OK - MUST: Package does not bundle copies of system libraries.
N/A - MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review.
OK - MUST: owns all directories that it creates
OK - MUST: no duplicate files in the %files listing
OK - MUST: Permissions on files are set properly, includes %defattr(...)
OK - MUST: package has a %clean section, which contains rm -rf %{buildroot}.
OK - MUST: consistently uses macros
OK - MUST: package contains code, or permissable content
N/A - MUST: Large documentation files should go in a -doc subpackage
OK - MUST: Files included as %doc do not affect the runtime of the application
OK - MUST: Header files must be in a -devel package
N/A - MUST: Static libraries must be in a -static package
FIX - MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'.
OK - MUST: If a package contains library files with a suffix, then library files that end in .so must go in a -devel package.
OK - MUST: devel packages must require the base package using a fully versioned dependency
OK - MUST: The package does not contain any .la libtool archives.
N/A - MUST: Packages containing GUI applications must include a %{name}.desktop file
OK - MUST: package does not own files or directories already owned by other packages.
OK - MUST: at the beginning of %install, the package runs rm -rf %{buildroot}.
OK - MUST: all filenames valid UTF-8


SHOULD Items:
OK - SHOULD: Source package includes license text(s) as a separate file.
N/A - SHOULD: The description and summary sections in the package spec file should contain translations for supported Non-English languages, if available.
TBD - SHOULD: builds in mock.
TBD - SHOULD: compiles and builds into binary rpms on all supported architectures.
TBD - SHOULD: functions as described.
OK - SHOULD: Scriptlets are used, those scriptlets must be sane.
N/A - SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
OK - SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg
OK - SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.


Other items:
OK - latest stable version
OK - SourceURL valid
OK - Compiler flags ok
TBD - Debuginfo complete


Issues:
- Two FIX items, see above: build error, missing "Requires: pkgconfig" of the -devel package

- the licensing is unclear:
eina_value.c is BSD, the rest of the sources is LGPLv2+, but COPYING is GPLv2. I guess COPYING is wrong and the package is LGPLv2+. Could you clarify this with upstream?

- you should add a %check section and run the included tests

- ChangeLog is missing from %doc, on the other hand README is so useless you can drop it.

Comment 6 Thomas Janssen 2010-02-21 15:58:16 UTC
- added Requires for -devel
- corrected license
- enabled tests
- added a note about zero byte changelog and removed readme

http://thomasj.fedorapeople.org/reviews/libeina-0.9.9.063-4.fc12.src.rpm
http://thomasj.fedorapeople.org/reviews/libeina.spec

Comment 7 Christoph Wickert 2010-02-21 23:29:55 UTC
Still doesn't build in mock:

+ chmod -x 'doc/html/*'
chmod: cannot access `doc/html/*': No such file or directory
Fehler beim Bauen des RPM:
Fehler: Fehler-Status beim Beenden von /var/tmp/rpm-tmp.LwFKWv (%install)
    Fehler-Status beim Beenden von /var/tmp/rpm-tmp.LwFKWv (%install)

Comment 8 Thomas Janssen 2010-02-22 10:49:33 UTC
Ok, stress is over, my brain is back working as expected.

- added missing BRs doxygen check

http://thomasj.fedorapeople.org/reviews/libeina.spec
http://thomasj.fedorapeople.org/reviews/libeina-0.9.9.063-5.fc12.src.rpm

Though, it fails to build at the test in mock and koji (even for F-12, that works locally). And i have right now no idea why. Is this test badly needed?

http://koji.fedoraproject.org/koji/taskinfo?taskID=2004500

Following the test from my local build:

make  check-local
make[2]: Entering directory `/home/thomas/rpmbuild/BUILD/eina-0.9.9.063'
Running suite(s): Eina
 eina_test_log.c:37 eina_log_macro() Critical message

 eina_test_log.c:38 eina_log_macro() An error

CRI:MyDomain eina_test_log.c:54 eina_log_domains_macros() A critical message

ERR:MyDomain eina_test_log.c:55 eina_log_domains_macros() An error

-01:Levels eina_test_log.c:127 eina_log_level_indexes() Negative index message

you should have a safety check failure below:
ERR: eina_magic.c:249 eina_magic_string_set() safety check failed: magic_name == NULL
you should have a safety check failure below:
ERR: eina_magic.c:249 eina_magic_string_set() safety check failed: magic_name == NULL
you should see 'Input handle pointer is NULL' below
CRI: eina_test_magic.c:69 eina_magic_simple() *** Eina Magic Check Failed !!!
    Input handle pointer is NULL !
*** NAUGHTY PROGRAMMER!!!
*** SPANK SPANK SPANK!!!
*** Now go fix your code. Tut tut tut!


you should see 'Input handle has already been freed' below
CRI: eina_test_magic.c:79 eina_magic_simple() *** Eina Magic Check Failed !!!
    Input handle has already been freed!
*** NAUGHTY PROGRAMMER!!!
*** SPANK SPANK SPANK!!!
*** Now go fix your code. Tut tut tut!


you should see 'Input handle is wrong type' below
CRI: eina_test_magic.c:83 eina_magic_simple() *** Eina Magic Check Failed !!!
    Input handle is wrong type
    Expected: 7781fee7 - Eina Magic Test
    Supplied: 028757b2 - (null)
*** NAUGHTY PROGRAMMER!!!
*** SPANK SPANK SPANK!!!
*** Now go fix your code. Tut tut tut!


you should have a safety check failure below:
ERR: eina_inlist.c:414 eina_inlist_remove() safety check failed: list == NULL
you should have a safety check failure below:
ERR: eina_inlist.c:415 eina_inlist_remove() safety check failed: item == NULL
# specimen      experiment time starting time   ending time
100000  3777086 151459  3928545
200000  8644658 3929258 12573916
you should have a safety check failure below:
ERR: eina_counter.c:436 eina_counter_dump() safety check failed: counter == NULL
you should have a safety check failure below:
ERR: eina_file.c:247 eina_file_split() safety check failed: path == NULL
Run specimens_check: 1000
   0    1    2    3    4    5    6    7    8    9 
  10   11   12   13   14   15   16   17   18   19 
  20   21   22   23   24   25   26   27   28   29 
  30   31   32   33   34   35   36   37   38   39 
  40   41   42   43   44   45   46   47   48   49 
  50   51   52   53   54   55   56   57   58   59 
  60   61   62   63   64   65   66   67   68   69 
  70   71   72   73   74   75   76   77   78   79 
  80   81   82   83   84   85   86   87   88   89 
  90   91   92   93   94   95   96   97   98   99 
100%: Checks: 54, Failures: 0, Errors: 0
make[2]: Leaving directory `/home/thomas/rpmbuild/BUILD/eina-0.9.9.063'
make[1]: Leaving directory `/home/thomas/rpmbuild/BUILD/eina-0.9.9.063'
+ exit 0

It builds fine in mock and koji without the tests enabled.

Comment 9 Thomas Janssen 2010-02-23 09:59:17 UTC
Ok, i had a discussion with upstream in IRC and they think the tests are wrong and i should disable them. They might fix it in later snapshots/releases.

So i followed upstream and disabled the test again.

http://thomasj.fedorapeople.org/reviews/libeina-0.9.9.063-5.fc12.src.rpm
http://thomasj.fedorapeople.org/reviews/libeina.spec

Comment 10 Thomas Janssen 2010-02-24 09:10:48 UTC
To have that complete:

The test fails locally as well after i de-installed eina-0 (former libeina).
As discussed in IRC, adding:
export LD_LIBRARY_PATH=$( pwd )/src/lib/.libs/libeina.so
doesn't help and the tests fail with:
ERR:eina_list eina_list.c:493 eina_list_init() ERROR: Mempool for list cannot be allocated in list init.
ERR:eina eina_main.c:203 eina_init() Could not initialize eina module 'list'.
CRI: eina_array.c:413 eina_array_new() eina_log_print() unknown domain -1, original message format 'array=%p'
CRI: eina_module.c:290 eina_module_new() eina_log_print() unknown domain -1, original message format 'm=%p, file=%s'
CRI: eina_module.c:290 eina_module_new() eina_log_print() unknown domain -1, original message format 'm=%p, file=%s'
CRI: eina_module.c:290 eina_module_new() eina_log_print() unknown domain -1, original message format 'm=%p, file=%s'
CRI: eina_module.c:591 eina_module_list_load() eina_log_print() unknown domain -1, original message format 'array %p, count %u'
CRI: eina_module.c:348 eina_module_load() eina_log_print() unknown domain -1, original message format 'm=%p, handle=%p, file=%s, refs=%d'
CRI: eina_mempool.c:128 eina_mempool_register() eina_log_print() unknown domain -1, original message format 'be=%p, name=%p'
make[2]: *** [check-local] Segmentation fault
make[2]: Leaving directory `/home/thomas/rpmbuild/BUILD/eina-0.9.9.063'
make[1]: *** [check-am] Error 2
make[1]: Leaving directory `/home/thomas/rpmbuild/BUILD/eina-0.9.9.063'
make: *** [check-recursive] Error 1
error: Bad exit status from /var/tmp/rpm-tmp.FNwSl2 (%check)

I still think following upstreams advice is fine.

Comment 11 Christoph Wickert 2010-02-25 00:12:04 UTC
I tried with:

%check
LD_LIBRARY_PATH=$( pwd )/src/lib/.libs/
LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$( pwd )/src/modules/mp/pass_through/.libs/
LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$( pwd )/src/modules/mp/chained_pool/.libs/
LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$( pwd )/src/modules/mp/fixed_bitmap/.libs/
LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$( pwd )/src/tests/.libs/
export LD_LIBRARY_PATH
make check

but still the tests fail. We should leave them out for now, but please work with upstream on getting this fixed.
And please disable the tests in configure for now because they end up in the debuginfo package


OK, lest check the remaining items:
OK - MUST: $ rpmlint /var/lib/mock/fedora-rawhide-x86_64/result/libeina-*
libeina.src: W: spelling-error %description -l en_US Eina -> Erna, Etna, Edna
libeina.src: W: spelling-error %description -l en_US multi -> mulch, mufti
libeina.x86_64: W: spelling-error %description -l en_US Eina -> Erna, Etna, Edna
libeina.x86_64: W: spelling-error %description -l en_US multi -> mulch, mufti
libeina-devel.x86_64: W: spelling-error Summary(en_US) Eina -> Erna, Etna, Edna
4 packages and 0 specfiles checked; 0 errors, 5 warnings.

All warnings are bogus, ignore them.

OK - MUST: Fedora approved license and meets the Licensing Guidelines: LGPLv2
OK - MUST: License field in spec file matches the actual license
OK - MUST: successfully compiles and builds into binary rpms on x86_64
OK - MUST: all build dependencies are listed in BuildRequires.
OK - MUST: Packages containing pkgconfig(.pc) files must 'Requires:
pkgconfig'.

SHOULD Items:
OK - SHOULD: builds in mock
OK - SHOULD: compiles and builds into binary rpms on all supported
architectures.
OK - SHOULD: functions as described.

Other items:
TBD - Debuginfo complete

All issues are fixed, the package is APPROVED.

Comment 12 Thomas Janssen 2010-02-25 07:20:57 UTC
Thanks for the review Christoph.

New Package CVS Request
=======================
Package Name: libeina
Short Description: A multi-platform library that provides optimized data types
Owners: thomasj
Branches: F-11 F-12 F-13
InitialCC: cassmodiah

Comment 13 Jason Tibbitts 2010-02-25 17:47:46 UTC
CVS done (by process-cvs-requests.py).

Comment 14 Fedora Update System 2010-02-26 10:04:54 UTC
libeina-0.9.9.063-6.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/libeina-0.9.9.063-6.fc12

Comment 15 Fedora Update System 2010-02-26 10:05:06 UTC
libeina-0.9.9.063-6.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/libeina-0.9.9.063-6.fc11

Comment 16 Fedora Update System 2010-02-26 10:05:11 UTC
libeina-0.9.9.063-6.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/libeina-0.9.9.063-6.fc13

Comment 17 Fedora Update System 2010-02-27 03:28:20 UTC
libeina-0.9.9.063-6.fc13 has been pushed to the Fedora 13 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update libeina'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F13/FEDORA-2010-2998

Comment 18 Fedora Update System 2010-02-27 03:36:00 UTC
libeina-0.9.9.063-6.fc12 has been pushed to the Fedora 12 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update libeina'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F12/FEDORA-2010-3035

Comment 19 Fedora Update System 2010-02-27 03:47:26 UTC
libeina-0.9.9.063-6.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update libeina'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2010-3089

Comment 20 Fedora Update System 2010-02-28 14:44:12 UTC
libeina-0.9.9.063-6.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 21 Fedora Update System 2010-03-12 04:25:18 UTC
libeina-0.9.9.063-6.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 22 Fedora Update System 2010-03-12 04:30:29 UTC
libeina-0.9.9.063-6.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.


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