Bug 427171

Summary: Review Request: sqliteman - Manager for sqlite - Sqlite Databases Made Easy
Product: [Fedora] Fedora Reporter: Terje Røsten <terje.rosten>
Component: Package ReviewAssignee: manuel wolfshant <manuel.wolfshant>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, notting
Target Milestone: ---Flags: manuel.wolfshant: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2008-01-14 20:33:29 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Terje Røsten 2008-01-01 22:22:15 UTC
Spec URL: http://terjeros.fedorapeople.org/sqliteman/sqliteman.spec
SRPM URL: http://terjeros.fedorapeople.org/sqliteman/sqliteman-1.0.1-1.fc8.src.rpm
Description:

If you are looking for a tool for tuning SQL statements, manage
tables, views, or triggers, administrate the database space and index
statistics then Sqliteman is the perfect choice.

If you are looking for a graphical queries creation wizards, user
interface designers for your database, or an universal report tool try
the applications designed for tasks such this (Kexi, knoda).

Comment 1 manuel wolfshant 2008-01-03 04:55:08 UTC
Please note that despite what the web page says, the included COPYING file (to
which all source files point) as well as doc/en/license.html include the
following paragraph:

  This program is free software; you can redistribute it and/or modify
   it under the  terms of the GNU General Public License as published
   by the Free Software Foundation; either version 2 of the License,
   or (at your option) any later version.

Therefore I'd say that the license tag in your spec should be GPLv2+

Ref. the desktop file: according to the packaging guidelines, the Icon tag
should either use the full path to the icon or the icon name without extension
(http://fedoraproject.org/wiki/Packaging/Guidelines#head-e205651a2c97a6857ab748c20d8ea60c25e3a520)


Comment 2 Jason Tibbitts 2008-01-03 05:13:49 UTC
The version of the GPL in the COPYING file isn't useful in determining the
actual version of the GPL which applies to the source code.  And if you read
before the section you quoted, you'll see that's in the section "How to Apply
These Terms to Your New Programs" and is just a sample notice which is supposed
to be included in the source code.  It doesn't actually apply to the source
code; in fact, it isn't actually part of the license at all (because it appears
after the "END OF TERMS AND CONDITIONS" text).

If the source code really does not include any information about the version of
the GPL in use, the GPL is explicit: "If the Program does not specify a version
number of this License, you may choose any version ever published by the Free
Software Foundation."  Which means GPL+.  But please note that I haven't
actually looked at the source in this case.

Comment 3 Terje Røsten 2008-01-03 06:44:12 UTC
(In reply to comment #2)
> The version of the GPL in the COPYING file isn't useful in determining the
> actual version of the GPL which applies to the source code.

From main.cpp:

 For general Sqliteman copyright and licensing information please refer
 to the COPYING file provided with the program. Following this notice may exist
 a copyright and/or license notice that predates the release of Sqliteman
 for which a new license (GPL+exception) is in place.

So I guess that's untrue in this case or?



Comment 4 Terje Røsten 2008-01-03 06:55:47 UTC
(In reply to comment #1)
> Therefore I'd say that the license tag in your spec should be GPLv2+

Agree, fixed

> Ref. the desktop file: according to the packaging guidelines, the Icon tag
> should either use the full path to the icon or the icon name without extension

Forgot that, fixed.

Spec : http://terjeros.fedorapeople.org/sqliteman/sqliteman.spec
SRPM : http://terjeros.fedorapeople.org/sqliteman/sqliteman-1.0.1-2.fc8.src.rpm


Comment 5 manuel wolfshant 2008-01-03 08:43:31 UTC
Package Review
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines.
 [x] Spec file name must match the base package %{name}, in the format %{name}.spec.
 [x] Package meets the Packaging Guidelines.
 [x] Package successfully compiles and builds into binary rpms on at least one
supported architecture.
     Tested on: x86_64
 [x] Rpmlint output:
        sqliteman.src: E: unknown-key GPG#7666df64 (can be ignored)
        sqliteman: no output
 [x] Package is not relocatable.
 [x] Buildroot is correct
(%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n))
 [x] Package is licensed with an open-source compatible license and meets other
legal requirements as defined in the legal section of Packaging Guidelines.
 [x] License field in the package spec file matches the actual license.
     License type:GPLv2+
 [x] If (and only if) the source package includes the text of the license(s) in
its own file, then that file, containing the text of the license(s) for the
package is included in %doc.
 [x] Spec file is legible and written in American English.
 [x] Sources used to build the package matches the upstream source, as provided
in the spec URL.
     SHA1SUM of package: 1c4ac936174f0f1dbddec479657e1da0dd133d01
sqliteman-1.0.1.tar.gz
 [x] Package is not known to require ExcludeArch
 [x] All build dependencies are listed in BuildRequires, except for any that are
listed in the exceptions section of Packaging Guidelines.
 [-] The spec file handles locales properly.
 [-] ldconfig called in %post and %postun if required.
 [x] Package must own all directories that it creates.
 [x] Package requires other packages for directories it uses.
 [x] Package does not contain duplicates in %files.
 [x] Permissions on files are set properly.
 [x] Package has a %clean section, which contains rm -rf %{buildroot} (or
$RPM_BUILD_ROOT).
 [x] Package consistently uses macros.
 [x] Package contains code, or permissable content.
 [-] Large documentation files are in a -doc subpackage, if required.
 [x] Package uses nothing in %doc for runtime.
 [-] Header files in -devel subpackage, if present.
 [-] Static libraries in -devel subpackage, if present.
 [-] Package requires pkgconfig, if .pc files are present.
 [-] Development .so files in -devel subpackage, if present.
 [-] Fully versioned dependency in subpackages, if present.
 [x] Package does not contain any libtool archives (.la).
               [x] Package contains a properly installed %{name}.desktop file if
it is a GUI application.
 [x] Package does not own files or directories owned by other packages.

=== SUGGESTED ITEMS ===
 [x] Latest version is packaged.
 [x] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contains
translations for supported Non-English languages, if available.
 [x] Reviewer should test that the package builds in mock.
     Tested on: devel/x86_64
 [x] Package should compile and build into binary rpms on all supported
architectures.
     Tested on:devel/x86_64
 [?] Package functions as described.
 [x] Scriptlets must be sane, if used.
 [-] The placement of pkgconfig(.pc) files is correct.
 [-] File based requires are sane.

=== Issues ===
1. The spec file tries to pack some icons using
# fix icons
mv %{buildroot}%{_datadir}/icons %{buildroot}%{_datadir}/pixmaps
%files
_datadir}/pixmaps/%{name}.png
However the final package has:
-rw-r--r--    1 root    root              417 Jan  3 10:09
/usr/share/sqliteman/icons/clear_table_contents.png
-rw-r--r--    1 root    root             5684 Jan  3 10:09
/usr/share/sqliteman/icons/database.png
-rw-r--r--    1 root    root             3150 Jan  3 10:09
/usr/share/sqliteman/icons/database_commit.png
-rw-r--r--    1 root    root             5990 Jan  3 10:09
/usr/share/sqliteman/icons/database_rollback.png
-rw-r--r--    1 root    root              572 Jan  3 10:09
/usr/share/sqliteman/icons/delete_table_row.png
-rw-r--r--    1 root    root              692 Jan  3 10:09
/usr/share/sqliteman/icons/document-new.png
-rw-r--r--    1 root    root             1001 Jan  3 10:09
/usr/share/sqliteman/icons/document-open.png
-rw-r--r--    1 root    root             1097 Jan  3 10:09
/usr/share/sqliteman/icons/document-save-as.png
-rw-r--r--    1 root    root             1150 Jan  3 10:09
/usr/share/sqliteman/icons/document-save.png
-rw-r--r--    1 root    root             1700 Jan  3 10:09
/usr/share/sqliteman/icons/index.png
-rw-r--r--    1 root    root              795 Jan  3 10:09
/usr/share/sqliteman/icons/insert_table_row.png
-rw-r--r--    1 root    root              568 Jan  3 10:09
/usr/share/sqliteman/icons/key.png
-rw-r--r--    1 root    root             2533 Jan  3 10:09
/usr/share/sqliteman/icons/runexplain.png
-rw-r--r--    1 root    root              809 Jan  3 10:09
/usr/share/sqliteman/icons/runsql.png
-rw-r--r--    1 root    root            27738 Jan  3 10:09
/usr/share/sqliteman/icons/sqliteman.png
-rw-r--r--    1 root    root             2687 Jan  3 10:09
/usr/share/sqliteman/icons/system.png
-rw-r--r--    1 root    root              966 Jan  3 10:09
/usr/share/sqliteman/icons/table.png
-rw-r--r--    1 root    root             2202 Jan  3 10:09
/usr/share/sqliteman/icons/trigger.png
-rw-r--r--    1 root    root              989 Jan  3 10:09
/usr/share/sqliteman/icons/view.png
In addition to that,
- it would be nice if timestamps of all those icon files would be preserved
- I think you should include sqliteman/icons/AUTHORS.
2. According to sqliteman/icons/AUTHORS, the icons are released under the LGPL
license. Which, if I understand correctly the guidelines from
http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#head-5dcaa7704b32aabaddc2e709f328f48eea6c91de
make me think that you  have to 
- either use as tag "GPLv2+ and LGPLv2+" or
- separate the icons in another standalone package and use different license
tags for the main and the -icons package

If I am wrong here, by all means please do correct me, I am fairly new in the
licensing field.



Comment 6 Terje Røsten 2008-01-03 19:51:04 UTC
> 1. The spec file tries to pack some icons using
> # fix icons
> mv %{buildroot}%{_datadir}/icons %{buildroot}%{_datadir}/pixmaps
> %files
> %{_datadir}/pixmaps/%{name}.png

My bad here, the comment should be:

# fix the desktop icon

the mv command is moving just moving

/usr/share/icons/sqliteman.png to /usr/share/pixmaps/sqliteman.png

so that the menu system can find the icon. 

Rest of the icons are for the app itself and should not be moved.
However I can add a the following:

# The entire source code is GPLv2+ except icons which are LGPLv2+
License: GPLv2+ and LGPLv2+





Comment 7 Terje Røsten 2008-01-03 20:03:06 UTC
Updated pkg:
- fix license again
- improve comment about movement of desktop icon


spec: http://terjeros.fedorapeople.org/sqliteman/sqliteman.spec
srpm: http://terjeros.fedorapeople.org/sqliteman/sqliteman-1.0.1-3.fc8.src.rpm


Comment 8 manuel wolfshant 2008-01-04 04:30:54 UTC
I would have loved to see the timestamps preserved, too

All issues that I have pointed were fixed, the program worked OK on my desktop
so the package is APPROVED

Comment 9 Terje Røsten 2008-01-04 17:37:31 UTC
> All issues that I have pointed were fixed, the program worked OK on my desktop
> so the package is APPROVED

Thanks!


New Package CVS Request
=======================
Package Name: sqliteman
Short Description: Manager for sqlite - Sqlite Databases Made Easy
Owners: terjeros
Branches: F-7 F-8
InitialCC:
Cvsextras Commits: yes


Comment 10 Kevin Fenzi 2008-01-04 18:51:37 UTC
cvs done.

Comment 11 Terje Røsten 2008-01-14 20:33:29 UTC
Pushed for rawhide, F-8 and F-7.