Bug 438126

Summary: Review Request: konq-plugins - Additional plugins that interact with konqueror
Product: [Fedora] Fedora Reporter: Sebastian Vahl <fedora>
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, kevin, notting, rdieter
Target Milestone: ---Flags: manuel.wolfshant: fedora-review+
tcallawa: 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-04-09 20:49:15 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 Sebastian Vahl 2008-03-19 10:09:44 UTC
Spec URL: http://svahl.fedorapeople.org/konq-plugins/konq-plugins.spec
SRPM URL: http://svahl.fedorapeople.org/konq-plugins/konq-plugins-4.0.2-0.1.20080303svn.fc8.src.rpm
Description: 
Some additional plugins that interact with konqueror

* akregator: Add feeds directly to akregator
* autorefresh: Refresh websites after a specifig period
* babelfish: Translate a website with babelfish
* dirfilter: Filter the current directory in many ways
* domtreeviewer: Displays the document object model in a box
* fsview: Graphical Disk Usage for inode/directory
* khtmlsettingsplugin: Enable/disable some HTML settings
* htmlvalidator: Validate webpages with w3c HTML validator
* minitools: Implement bookmarklets into konqueror
* uachanger: Change the user agent for websites

rpmlint:
konq-plugins.i386: W: dangling-symlink /usr/share/doc/HTML/en/konq-plugins/uachanger/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative /usr/share/doc/HTML/en/konq-plugins/uachanger/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink /usr/share/doc/HTML/en/konq-plugins/domtreeviewer/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative /usr/share/doc/HTML/en/konq-plugins/domtreeviewer/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink /usr/share/doc/HTML/en/konq-plugins/fsview/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative /usr/share/doc/HTML/en/konq-plugins/fsview/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink /usr/share/doc/HTML/en/konq-plugins/validators/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative /usr/share/doc/HTML/en/konq-plugins/validators/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink /usr/share/doc/HTML/en/konq-plugins/dirfilter/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative /usr/share/doc/HTML/en/konq-plugins/dirfilter/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink /usr/share/doc/HTML/en/konq-plugins/babel/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative /usr/share/doc/HTML/en/konq-plugins/babel/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink /usr/share/doc/HTML/en/konq-plugins/imgallery/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative /usr/share/doc/HTML/en/konq-plugins/imgallery/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink /usr/share/doc/HTML/en/konq-plugins/webarchiver/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative /usr/share/doc/HTML/en/konq-plugins/webarchiver/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink /usr/share/doc/HTML/en/konq-plugins/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative /usr/share/doc/HTML/en/konq-plugins/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink /usr/share/doc/HTML/en/konq-plugins/crashes/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative /usr/share/doc/HTML/en/konq-plugins/crashes/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink /usr/share/doc/HTML/en/konq-plugins/khtmlsettings/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative /usr/share/doc/HTML/en/konq-plugins/khtmlsettings/common /usr/share/doc/HTML/en/commo

(As usual for KDE:) Safe to ignore.

Some open questions:
- License: I use GPLv2+, but some content is licensed under LGPLv2.1+ and LGPLv2+
- Group: Applications/Internet? IMHO this should be fine because konqueror is now mainly used as webbrowser
- akregator plugin: This plugin is useless without kdepim. But IMHO a "Require: kdepim" for the whole package is too much. So the plugin should maybe splitted into a sub package.
- All plugins should be more tested for functionality.


Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=522020

Comment 1 Kevin Kofler 2008-04-01 16:17:47 UTC
> - License: I use GPLv2+, but some content is licensed under LGPLv2.1+ and 
LGPLv2+

These are all GPLv2+ compatible, but if you want to be precise, you can write: 
GPLv2+ and LGPLv2+ (we don't distinguish between 2.0 and 2.1 in the License 
tags, the only difference is the name anyway).

> - Group: Applications/Internet?

Yes, this is fine.

> - akregator plugin: This plugin is useless without kdepim. But IMHO
> a "Require: kdepim" for the whole package is too much. So the plugin should
> maybe splitted into a sub package.

Not sure about this one, let's discuss this in the meeting. :-)

Comment 2 Sebastian Vahl 2008-04-01 17:35:17 UTC
(In reply to comment #1)
> These are all GPLv2+ compatible, but if you want to be precise, you can 
write: 
> GPLv2+ and LGPLv2+ (we don't distinguish between 2.0 and 2.1 in the License 
> tags, the only difference is the name anyway).

Ok. I'll stick to GPLv2+ then.

> Not sure about this one, let's discuss this in the meeting. :-)

Decision was to don't require kdepim or create a subpkg but to add a short 
explanation in %description


Spec Url: http://svahl.fedorapeople.org/konq-plugins/konq-plugins.spec
SRPM Url: 
http://svahl.fedorapeople.org/konq-plugins/konq-plugins-4.0.2-0.2.20080303svn.fc8.src.rpm

Changelog:
- rebuild for NDEBUG and _kde4_libexecdir
- enhance %%description a bit

Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=542372



Comment 3 manuel wolfshant 2008-04-05 14:22:08 UTC
everything seems OK, only odd thing is that patch2 is not applied.

I'll come back a bit later after I test it. Sadly, it does not build for F-7
(i.e. I cannot test on my sacrifice box == my workstation) so I have to try on
another computer

Comment 4 manuel wolfshant 2008-04-06 00:38:49 UTC
Is this package intended only for F-9/rawhide ? It does not build for F-8 either...

Comment 5 Kevin Kofler 2008-04-06 04:37:48 UTC
Yes, this is Rawhide-only, it's for the KDE 4 Konqueror.

Comment 6 Kevin Kofler 2008-04-06 04:43:56 UTC
(The KDE 3 version is part of kdeaddons, by the way.)

Comment 7 Rex Dieter 2008-04-08 16:23:16 UTC
fwiw, I've played with this looking for breakage, for only a few minutes here
and there.  found no show-stoppers, nothing remotely thorough mind you.

Comment 8 Rex Dieter 2008-04-08 16:26:31 UTC
wolfy, if you consider getting yourself working rawhide (to test) to review this
a blocker, please remove yourself as reviewer, else, please continue. :)

Comment 9 manuel wolfshant 2008-04-08 19:25:02 UTC
I've been trying since Sunday to test the rpm, but I've been hit by several
bugs. First konqueror would not start, then (twice) I hit
https://www.redhat.com/archives/fedora-devel-list/2008-April/msg00690.html

Rex, feel free to review the bug yourself if so you wish. All I can do is to
upgrade my F8 for the 4th time and hope for the best.

Comment 10 manuel wolfshant 2008-04-09 00:33:47 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: devel/x86_64
 [x] Rpmlint output:
source RPM:
W: patch-not-applied Patch2: konq-plugins-4.0.1-icons.patch
-> Sebastian ? Should this patch be applied ?
binary RPM:
konq-plugins.i386: W: dangling-symlink
/usr/share/doc/HTML/en/konq-plugins/uachanger/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative
/usr/share/doc/HTML/en/konq-plugins/uachanger/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink
/usr/share/doc/HTML/en/konq-plugins/domtreeviewer/common
/usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative
/usr/share/doc/HTML/en/konq-plugins/domtreeviewer/common
/usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink
/usr/share/doc/HTML/en/konq-plugins/fsview/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative
/usr/share/doc/HTML/en/konq-plugins/fsview/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink
/usr/share/doc/HTML/en/konq-plugins/validators/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative
/usr/share/doc/HTML/en/konq-plugins/validators/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink
/usr/share/doc/HTML/en/konq-plugins/dirfilter/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative
/usr/share/doc/HTML/en/konq-plugins/dirfilter/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink
/usr/share/doc/HTML/en/konq-plugins/babel/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative
/usr/share/doc/HTML/en/konq-plugins/babel/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink
/usr/share/doc/HTML/en/konq-plugins/imgallery/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative
/usr/share/doc/HTML/en/konq-plugins/imgallery/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink
/usr/share/doc/HTML/en/konq-plugins/webarchiver/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative
/usr/share/doc/HTML/en/konq-plugins/webarchiver/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink
/usr/share/doc/HTML/en/konq-plugins/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative
/usr/share/doc/HTML/en/konq-plugins/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink
/usr/share/doc/HTML/en/konq-plugins/crashes/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative
/usr/share/doc/HTML/en/konq-plugins/crashes/common /usr/share/doc/HTML/en/common
konq-plugins.i386: W: dangling-symlink
/usr/share/doc/HTML/en/konq-plugins/khtmlsettings/common
/usr/share/doc/HTML/en/common
konq-plugins.i386: W: symlink-should-be-relative
/usr/share/doc/HTML/en/konq-plugins/khtmlsettings/common
/usr/share/doc/HTML/en/common
-> acceptable for a KDE package
 [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.
 [!] License field in the package spec file matches the actual license.
     License type:GPLv2+ and BSD and LGPLv2+
     License tag: 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:not available, source is a svn checkout.
  Running diff on the individual files mostly match. There are however 24
different source and graphic files and 864 different translation files. All
differences seem to be due to svn updates.
  I will rely on Sebastian's judgment over the version to be shipped in Fedora.
 [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.
 [x] 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 ===
 [!] Latest version is packaged.
not a problem, the package uses svn version 4.0.2, meanwhile 4.0.3 has been
released. As stated above, I rely on Sebastian to pick the best release to be
shipped in Fedora.
 [x] Package does not include license text files separate from upstream.
 [-] Description and summary sections in the package spec file contain
translations for supported Non-English languages, if available.
 [x] Reviewer should test that the package builds in mock.
     Tested on: koji scratch build
 [x] Package should compile and build into binary rpms on all supported
architectures.
     Tested on:koji scratch build
 [x] 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. Not all the plugins are using the same license and they are delivered as
separate libraries. Therefore I think that the correct license tag is "GPLv2+
and LGPLv2+" (http://fedoraproject.org/wiki/Licensing/FAQ , "Multiple licensing
situations", answer A3)
2. The autorefresh plugin has no license specified. IANAL, but I am almost
certain this has to be clarified before including it in Fedora
3. One patch is included in the src.rpm but not applied. Maybe you could either
drop it, use it or add a note mentioning why is it included but not used ?


================
*** APPROVED *** but please fix the license tag before commit
================


Comment 11 Sebastian Vahl 2008-04-09 08:28:06 UTC
(In reply to comment #10)
>  [x] Rpmlint output:
> source RPM:
> W: patch-not-applied Patch2: konq-plugins-4.0.1-icons.patch
> -> Sebastian ? Should this patch be applied ?

No, it's not needed anymore. I'll remove it.

>  [!] Latest version is packaged.
> not a problem, the package uses svn version 4.0.2, meanwhile 4.0.3 has been
> released. As stated above, I rely on Sebastian to pick the best release to 
be
> shipped in Fedora.

konq-plugins did not get changes for a long time. The script create_tarball.rb 
use the actual verstion of KDE also for the created tarball. At time of 
creation this was 4.0.2.
However, there were some changes in the past days. So I could 
a) update the package and submit the new version for review again
b) import the package and update it after the import


> 1. Not all the plugins are using the same license and they are delivered as
> separate libraries. Therefore I think that the correct license tag 
is "GPLv2+
> and LGPLv2+" (http://fedoraproject.org/wiki/Licensing/FAQ , "Multiple 
licensing
> situations", answer A3)

Ok. I'll use "GPLv2+ and LGPLv2+" then.

> 2. The autorefresh plugin has no license specified. IANAL, but I am almost
> certain this has to be clarified before including it in Fedora

I'll try to ping upstream to include a license in autorefresh.* files then. 
But what to to until then? Import it and wait for upstream or wait with the 
import until upstream has fixed this?

> 3. One patch is included in the src.rpm but not applied. Maybe you could 
either
> drop it, use it or add a note mentioning why is it included but not used ?

See above. I'll drop it.


Comment 12 Kevin Kofler 2008-04-09 08:37:25 UTC
Just assume it's GPL, KDE has a global licensing policy for a reason.

Comment 13 manuel wolfshant 2008-04-09 09:01:14 UTC
> At time of creation this was 4.0.2.
I've compared the included tar with svn version 4.0.2, as pulled by the
create_tarball.rb script. The differences I have mentioned are between these two
file version.

>However, there were some changes in the past days.
Yup, they switched to 4.0.3

> So I could 
>a) update the package and submit the new version for review again
no need to re-review, the differences are in the source code, not in the package
itself


>b) import the package and update it after the import
feel free to update whenever you seem fit. I for one would import directly the
new version, but it's your choice


Re #12: Kevin, could you please point me to the docs which simplify our life and
justify overriding http://fedoraproject.org/wiki/Licensing/FAQ in the case of KDE ?


Comment 14 Sebastian Vahl 2008-04-09 09:26:09 UTC
(In reply to comment #12)
> Just assume it's GPL, KDE has a global licensing policy for a reason.

Ok. Thx.

(In reply to comment #13)
> no need to re-review, the differences are in the source code, not in the 
package itself

Ok. I'll do the changes after the import then.

> >b) import the package and update it after the import
> feel free to update whenever you seem fit. I for one would import directly 
the
> new version, but it's your choice

In my reading of the packaging guidelines I'm only allowed to import the 
_approved_ SRPM (here 4.0.2-0.2-20080303svn). But when I do the changes is not 
important, so I'll do it after the import. :)

Comment 15 Sebastian Vahl 2008-04-09 09:28:02 UTC
(In reply to comment #10)
> ================
> *** APPROVED *** but please fix the license tag before commit
> ================

Thanks!


New Package CVS Request
=======================
Package Name: konq-plugins
Short Description: Additional plugins that interact with konqueror
Owners: svahl,than,rdieter,kkofler,ltinkl
Branches: 
InitialCC: 
Cvsextras Commits: no



Comment 16 Kevin Kofler 2008-04-09 11:03:58 UTC
Here's the evidence that the plugin is GPL:
http://websvn.kde.org/trunk/extragear/base/konq-plugins/autorefresh/autorefresh.desktop?revision=748952&view=markup
It says:
X-KDE-PluginInfo-License=GPL
The version isn't specified, but this isn't a blocker.

And more generally speaking: The FAQ you pointed to says:

4. If neither the source, nor the upstream composed documentation says 
anything about the license version, then it could be under _ANY_ version of 
the GPL. The version listed in COPYING is irrelevant from this perspective. 
Technically it could be under any license, but if all we have to go by is 
COPYING, we'll guess COPYING is accurate. 

Now, keep in mind that most upstreams are probably leaving the versioning out 
by accident. If you get to case 4, you definitely want to let upstream know 
that you are unable to determine the applicable version(s) of the license from 
the source and documentation. They'll almost certainly let you know what their 
intended license version is, and (hopefully) correct it in the upstream 
source.

The problem of missing license headers in some files is unfortunately fairly 
common, not only in KDE. I don't think we have any reason to consider this 
anything other than an accident, considering KDE's licensing policy.

http://techbase.kde.org/Policies/Licensing_Policy
(Note: There are older revisions which didn't require GPLv3 compatibility, but 
they all required licenses which are acceptable for Fedora.)

Comment 17 manuel wolfshant 2008-04-09 11:45:26 UTC
Excellent, this clarifies the license of autorefresh. Thanks, Kevin !

PS: I had the impression that in comment #12 you were speaking about the license
tag of the RPM, not about this particular plugin. It's all clear now.

Comment 18 Kevin Kofler 2008-04-09 14:40:30 UTC
> In my reading of the packaging guidelines I'm only allowed to import the 
> _approved_ SRPM (here 4.0.2-0.2-20080303svn).

If you're that pedantic... ;-) I have imported approved SRPM + minor updates on 
more than one occasion. It doesn't really matter either way, of course.


As for the License tag, as the LGPL can be converted to the GPL, writing just 
GPLv2+ instead of LGPLv2+ or GPLv2+ is not that big an issue, but of course 
being precise is better.

Comment 19 Kevin Kofler 2008-04-09 14:46:48 UTC
Oops, I meant writing just "GPLv2+" instead of "LGPLv2+ and GPLv2+", not "or". 
Either way, my point stands. :-)

Comment 20 Tom "spot" Callaway 2008-04-09 15:44:33 UTC
Please request some branches. I'm assuming you don't want an empty cvs add. :P

Comment 21 Kevin Kofler 2008-04-09 15:54:24 UTC
We want devel and F-9, we can wait for the mass-branching for the latter 
though, we don't need separate F10 builds yet.

Comment 22 Tom "spot" Callaway 2008-04-09 17:10:38 UTC
cvs done.

Comment 23 Sebastian Vahl 2008-04-09 20:49:15 UTC
Package imported and built:
http://koji.fedoraproject.org/koji/taskinfo?taskID=560246

Thanks all!