Bug 226429 - (sqlite) Merge Review: sqlite
Merge Review: sqlite
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Robin Norwood
Fedora Package Reviews List
:
Depends On:
Blocks: F9MergeReviewTarget
  Show dependency treegraph
 
Reported: 2007-01-31 16:01 EST by Nobody's working on this, feel free to take it
Modified: 2009-06-15 05:28 EDT (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-06-13 18:40:07 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
robin.norwood: fedora‑review+


Attachments (Terms of Use)
patch to specfile to fix some package review issues (3.39 KB, patch)
2008-01-11 22:34 EST, Robin Norwood
no flags Details | Diff
use .so instead of .la files when running tests (938 bytes, patch)
2008-01-11 22:35 EST, Robin Norwood
no flags Details | Diff
fix the bind test - NaN doesn't seem to be valid for a double column (334 bytes, patch)
2008-01-11 22:36 EST, Robin Norwood
no flags Details | Diff
fix io test - but I don't know why it was broken to begin with, so investigation is warrented. (358 bytes, patch)
2008-01-11 22:37 EST, Robin Norwood
no flags Details | Diff
printf formatting is broken for some reason (3.43 KB, patch)
2008-01-11 22:42 EST, Robin Norwood
no flags Details | Diff
the format of this tcl error message changed. the test should be fixed, but I don't know tcl syntax enough to fix it. (682 bytes, patch)
2008-01-11 22:45 EST, Robin Norwood
no flags Details | Diff

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 16:01:11 EST
Fedora Merge Review: sqlite

http://cvs.fedora.redhat.com/viewcvs/devel/sqlite/
Initial Owner: pnasrat@redhat.com
Comment 1 Warren Togami 2007-02-01 14:13:21 EST
https://www.redhat.com/archives/fedora-extras-commits/2005-April/msg01144.html

Well, this was a pre-Bugzilla review.
Comment 2 Robin Norwood 2008-01-11 22:30:54 EST
Package review:

- rpmlint checks return:

    sqlite-3.5.4-2.fc9.src.rpm -

sqlite.src:14: W: unversioned-explicit-obsoletes sqlite3
sqlite.src:14: W: unversioned-explicit-obsoletes sqlite3-devel

sqlite3 last appeared (AFAICT) in FC-3's extras.  Since it's more than 3
releases old, we can drop this obsoletes.  Also, the matching 'provides' are
missing.

sqlite.src: W: mixed-use-of-spaces-and-tabs (spaces: line 57, tab: line 11)

Easy to fix.

  sqlite-devel-3.5.4-2.fc9.i386.rpm -

sqlite-devel.i386: W: summary-ended-with-dot Development tools for the sqlite3
embeddable SQL database engine.

* both of the above are fixed in the attached patch to the spec file. *

  sqlite-tcl-3.5.4-2.fc9.i386.rpm -

sqlite-tcl.i386: W: no-documentation

* Can probably be ignored

sqlite-tcl.i386: W: summary-ended-with-dot Tcl module for the sqlite3 embeddable
SQL database engine.

* fixed in attached patch

sqlite-tcl.i386: E: arch-dependent-file-in-usr-share
/usr/share/tcl8.5/sqlite3/libtclsqlite3.so
sqlite-tcl.i386: W: unstripped-binary-or-object
/usr/share/tcl8.5/sqlite3/libtclsqlite3.so

* This is obviously wrong, they should be in /usr/lib - I'm not sure what to do
to fix this properly, however.

- package meets naming guidelines - Ok
- license ( ) OK, text in %doc, matches source - Ok - Public Domain
- spec file legible, in am. english - Ok
- source matches upstream - Ok
- package compiles on devel (x86) - Ok
- no missing BR - Ok
- no unnecessary BR - Ok
- no locales - Ok
- not relocatable - Ok
- owns all directories that it creates - Ok
- no duplicate files - Ok
- permissions ok - Ok
- %clean - Ok
- macro use consistent - Ok
- code, not content - Ok
- no need for -docs - Ok
- nothing in %doc affects runtime - Ok
- no need for .desktop file - Ok
- devel package ok - Ok
- no .la files - removed in %install section
- post/postun ldconfig ok - Ok
- devel requires base package n-v-r - Ok


- Tests are not run by default.

* This is probably because several tests fail.  I've enabled tests in the
attached specfile patch, and included patches which fix or comment out the tests
that fail.  Someone needs to check to see why the failing tests are failing, and
either fix the tests upstream, fix the bug upstream, or figure out why the tests
fail for us.
Comment 3 Robin Norwood 2008-01-11 22:34:18 EST
Created attachment 291445 [details]
patch to specfile to fix some package review issues

this patch to the specfile fixes some of the package review issues, and enables
make check
Comment 4 Robin Norwood 2008-01-11 22:35:20 EST
Created attachment 291446 [details]
use .so instead of .la files when running tests
Comment 5 Robin Norwood 2008-01-11 22:36:17 EST
Created attachment 291447 [details]
fix the bind test - NaN doesn't seem to be valid for a double column
Comment 6 Robin Norwood 2008-01-11 22:37:22 EST
Created attachment 291448 [details]
fix io test - but I don't know why it was broken to begin with, so investigation is warrented.
Comment 7 Robin Norwood 2008-01-11 22:42:21 EST
Created attachment 291449 [details]
printf formatting is broken for some reason
Comment 8 Robin Norwood 2008-01-11 22:45:00 EST
Created attachment 291450 [details]
the format of this tcl error message changed.  the test should be fixed, but I don't know tcl syntax enough to fix it.
Comment 9 manuel wolfshant 2008-01-12 17:40:25 EST
Robin, the Fedora-review flag should be set to "-" only if a package has
fundamental flaws and the review should deny it.
As long as a package is under review, the flag should be "?"
Comment 10 Robin Norwood 2008-01-13 11:35:12 EST
Yes, you're right, of course - I fail at following the process. :-)

Thanks.
Comment 11 Michal Nowak 2009-04-09 08:56:38 EDT
Panu, this seems to be stuck for some time due to not reply from package maintainers, can you follow to Robin's proposals & review?
Comment 12 Panu Matilainen 2009-04-09 09:02:46 EDT
Lets just say that it might help somewhat that this bug has been brought to my attention :)

I'll look into this as soon as I've dealt with some other more pressing matters.
Comment 13 Panu Matilainen 2009-05-15 03:40:42 EDT
Okay, I fixed the rpmlint nitpickery over tab-vs-space etc + obsoletes + the io tests (the difference comes from SQLITE_DISABLE_DIRSYNC build-switch so its ok) in devel. With that, as of sqlite 3.6.14, the test-suite is error-free on ix86 + x86_64, the other testsuite-patches here dont seem to be relevant (at least anymore). Except, hmm, what's with the "use .so instead of .la" patch?

PPC (both 32 and 64bit) is not quite there however, this nan-breakage:
http://www.sqlite.org/cvstrac/tktview?tn=3404 and then a pile of rtree-tests fails (possibly due to the nan-issue, haven't looked that closely).
Comment 14 Michal Nowak 2009-05-15 03:53:17 EDT
Thanks, Panu!

--

Robin: Can you continue with the review, please? The packages are here: http://koji.fedoraproject.org/koji/packageinfo?packageID=485
Comment 15 Robin Norwood 2009-05-15 12:14:53 EDT
Yes, I'll get on it.  I probably won't be able to get to it until tomorrow, though.
Comment 16 Robin Norwood 2009-05-17 00:29:52 EDT
Sorry, one more: rpmlint points out that the -rpath issue is present here.  Ref:

https://fedoraproject.org/wiki/Packaging/Guidelines#Beware_of_Rpath

The suggested method using sed to strip out the hardcoded paths seems to work for me at first glance.
Comment 17 Panu Matilainen 2009-06-12 06:50:43 EDT
rpath is disabled starting with 3.6.14-2
Comment 18 Michal Nowak 2009-06-12 07:39:28 EDT
Thanks! So close to review+ :), right?
Comment 19 Robin Norwood 2009-06-12 11:28:18 EDT
Yup!  Passes review.
Comment 20 Michal Nowak 2009-06-13 18:40:07 EDT
Thanks all.
Comment 21 Panu Matilainen 2009-06-15 05:28:52 EDT
Wohoo :) Thanks.

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