Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: unixODBC-gui-qt - Qt tools for unixODBC
https://bugzilla.redhat.com/show_bug.cgi?id=767622
Summary: Review Request: unixODBC-gui-qt - Qt tools for unixODBC Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: medium Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: hhorak@redhat.com QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: --- Regression: --- Mount Type: --- Documentation: ---
Spec URL: http://hhorak.fedorapeople.org/unixodbc-gui-qt/unixODBC-gui-qt.spec SRPM URL: http://hhorak.fedorapeople.org/unixodbc-gui-qt/unixODBC-gui-qt-0-0.1.2011120... Description:
unixODBC-gui-qt provides several GUI (Qt) programs and plugins. * administrator (program) * create data source wizard (program) * test (program) * installer (plugin) * auto test (plugin)
These GUI programs and plugins were part of unixODBC project until 2.2.14 and has been shipped in Fedora as unixODBC-kde. Since unixODBC-2.3.0 (which is going to be shipped into Fedora soon) GUI programs and plugins has moved to a separate project unixODBC-gui-qt, which hasn't had a formal release yet, unfortunately. Users would miss unixODBC GUI tools after unixODBC rebase.
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=767622
--- Comment #1 from Tom Lane tgl@redhat.com 2011-12-14 09:52:08 EST --- One note is that when unixODBC is bumped to 2.3.x, it will no longer contain a unixODBC-kde subpackage, and that gap is essentially what this package needs to fill. It strikes me that we probably ought to have a virtual "Provides: unixODBC-kde = 2.3.0-1" in here, as well as the Obsoletes spec.
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=767622
--- Comment #2 from Honza Horak hhorak@redhat.com 2011-12-14 11:00:05 EST --- (In reply to comment #1)
One note is that when unixODBC is bumped to 2.3.x, it will no longer contain a unixODBC-kde subpackage, and that gap is essentially what this package needs to fill. It strikes me that we probably ought to have a virtual "Provides: unixODBC-kde = 2.3.0-1" in here, as well as the Obsoletes spec.
I agree with that and will include this change together with other (I suppose there will be more of them).
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=767622
Dan Horák dan@danny.cz changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED AssignedTo|nobody@fedoraproject.org |dan@danny.cz Flag| |fedora-review?
--- Comment #3 from Dan Horák dan@danny.cz 2011-12-14 13:49:43 EST --- taking for review
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=767622
--- Comment #4 from Honza Horak hhorak@redhat.com 2011-12-15 07:12:25 EST --- I've accidentally attached a link to a binary package, so these are the right links (already with Provides suggested in comment #1): Spec URL: http://hhorak.fedorapeople.org/unixodbc-gui-qt/unixODBC-gui-qt.spec SRPM URL: http://hhorak.fedorapeople.org/unixodbc-gui-qt/unixODBC-gui-qt-0-0.2.2011120...
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=767622
--- Comment #5 from Dan Horák dan@danny.cz 2011-12-15 08:49:37 EST --- formal review is here, see the notes explaining OK* and BAD statuses below:
OK* source files match upstream: compared with my own checkout OK package meets naming and versioning guidelines. BAD specfile is properly named, is cleanly written and uses macros consistently. OK dist tag is present. BAD license field matches the actual license. OK license is open source-compatible (GPL/LGPL). License text included in package. OK latest version is being packaged. OK BuildRequires are proper. OK compiler flags are appropriate. OK package builds in mock (Rawhide/x86_64). OK debuginfo package looks complete. BAD rpmlint is silent. OK* final provides and requires look sane. N/A %check is present and all tests pass. OK shared libraries are added to the regular linker search paths, scriptlet present OK owns the directories it creates. OK doesn't own any directories it shouldn't. OK no duplicates in %files. OK file permissions are appropriate. OK correct scriptlets present. OK code, not content. OK documentation is small, so no -docs subpackage is necessary. OK %docs are not necessary for the proper functioning of the package. OK no headers. OK no pkgconfig files. OK no libtool .la droppings. OK GUI apps with desktop files
- the source archive is not compressed although it has .gz suffix, you can use fedora-getsvn tool for grabbing the sources - instead of using "cp" for installing files you should use either "cp -p" or (better) "install -p -m 644", so the timestamp is kept for the files - licenses as written in README and source files would be GPL+ and LGPL+ because no version is specified, included license texts are irrelevant here (see https://fedoraproject.org/wiki/Licensing/FAQ), clarification with upstream is required, ODBCTestQ4 seems to be GPLv2+, unixODBC library itself is LGPLv2+ - passing --disable-static to configure should let only the shared libs build, removing the need to "rm" them - shouldn't a devel subpackage exist? if not then the *.so links shouldn't be packaged at all - rpmlint complains a bit: unixODBC-gui-qt.x86_64: W: spelling-error Summary(en_US) plugins -> plug ins, plug-ins, plugging unixODBC-gui-qt.x86_64: W: spelling-error %description -l en_US plugins -> plug ins, plug-ins, plugging unixODBC-gui-qt.x86_64: W: spelling-error %description -l en_US plugin -> plug in, plug-in, plugging unixODBC-gui-qt.src: W: spelling-error Summary(en_US) plugins -> plug ins, plug-ins, plugging unixODBC-gui-qt.src: W: spelling-error %description -l en_US plugins -> plug ins, plug-ins, plugging unixODBC-gui-qt.src: W: spelling-error %description -l en_US plugin -> plug in, plug-in, plugging => please fix
unixODBC-gui-qt.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libgtrtstQ4.so unixODBC-gui-qt.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libodbcinstQ4.so => see point above
unixODBC-gui-qt.x86_64: W: no-manual-page-for-binary ODBCTestQ4 unixODBC-gui-qt.x86_64: W: no-manual-page-for-binary ODBCCreateDataSourceQ4 unixODBC-gui-qt.x86_64: W: no-manual-page-for-binary ODBCManageDataSourcesQ4 => would be nice, but not a blocker
unixODBC-gui-qt.src: W: invalid-url Source0: unixODBC-gui-qt-20111208svn95.tar.gz => OK, snapshot is used
unixODBC-gui-qt-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/unixODBC-gui-qt/ODBCTestQ4/* => should be reported/fixed upstream
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=767622
--- Comment #6 from Tom Lane tgl@redhat.com 2011-12-15 10:24:16 EST --- (In reply to comment #5)
Review comments seem mostly sound, except this:
- shouldn't a devel subpackage exist? if not then the *.so links shouldn't be
packaged at all
The reason for the *.so symlinks is that these libraries are meant to be dlopen'ed at runtime by unixODBC, and people are not in the habit of including soname version numbers in the config files that specify what to use. So it is correct that there are *.so links in the base package. They are not meant for devel purposes.
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=767622
--- Comment #7 from Honza Horak hhorak@redhat.com 2011-12-15 12:51:36 EST --- I've asked upstream to specify the license version and correct FSF address. The other issues seem to be clear, I'll attach a fixed srpm and spec as soon as I have an answer from upstream.
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=767622
--- Comment #8 from Honza Horak hhorak@redhat.com 2012-01-06 03:21:38 EST --- (In reply to comment #5)
- the source archive is not compressed although it has .gz suffix, you can use
fedora-getsvn tool for grabbing the sources
Sources are grabbed with fedora-getsvn now.
- instead of using "cp" for installing files you should use either "cp -p" or
(better) "install -p -m 644", so the timestamp is kept for the files
"install -p -m 644" is used now.
- licenses as written in README and source files would be GPL+ and LGPL+
because no version is specified, included license texts are irrelevant here (see https://fedoraproject.org/wiki/Licensing/FAQ), clarification with upstream is required, ODBCTestQ4 seems to be GPLv2+, unixODBC library itself is LGPLv2+
Upstream points explicitly to GPL.txt/LGPL.txt files in source now, so GPLv3/LGPLv3 is used.
- passing --disable-static to configure should let only the shared libs build,
removing the need to "rm" them
Unneeded "rm" removed.
- rpmlint complains a bit:
unixODBC-gui-qt.x86_64: W: spelling-error Summary(en_US) plugins -> plug ins, plug-ins, plugging ... unixODBC-gui-qt.src: W: spelling-error %description -l en_US plugin -> plug in, plug-in, plugging => please fix
Fixed.
- shouldn't a devel subpackage exist? if not then the *.so links shouldn't be
packaged at all unixODBC-gui-qt.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libgtrtstQ4.so unixODBC-gui-qt.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libodbcinstQ4.so => see point above
See comment #6.
unixODBC-gui-qt-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/unixODBC-gui-qt/ODBCTestQ4/* => should be reported/fixed upstream
Reported upstream.
Spec URL: http://hhorak.fedorapeople.org/unixodbc-gui-qt/unixODBC-gui-qt.spec SRPM URL: http://hhorak.fedorapeople.org/unixodbc-gui-qt/unixODBC-gui-qt-0-0.3.2012010...
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=767622
Dan Horák dan@danny.cz changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review? |fedora-review+
--- Comment #9 from Dan Horák dan@danny.cz 2012-01-06 05:31:17 EST --- All issues are fixed or explained, the package is APPROVED.
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=767622
Honza Horak hhorak@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-cvs?
--- Comment #10 from Honza Horak hhorak@redhat.com 2012-01-06 07:12:57 EST --- New Package SCM Request ======================= Package Name: unixODBC-gui-qt Short Description: Several GUI (Qt) programs and plug-ins for unixODBC Owners: hhorak tgl 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=767622
--- Comment #11 from Jon Ciesla limburgher@gmail.com 2012-01-06 09:23:57 EST --- 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=767622
Honza Horak hhorak@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |NEXTRELEASE Last Closed| |2012-01-11 12:07:10
package-review@lists.fedoraproject.org