Bug 438126
Summary: | Review Request: konq-plugins - Additional plugins that interact with konqueror | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Sebastian Vahl <fedora> |
Component: | Package Review | Assignee: | manuel wolfshant <manuel.wolfshant> |
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | 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
> - 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. :-) (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 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 Is this package intended only for F-9/rawhide ? It does not build for F-8 either... Yes, this is Rawhide-only, it's for the KDE 4 Konqueror. (The KDE 3 version is part of kdeaddons, by the way.) 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. wolfy, if you consider getting yourself working rawhide (to test) to review this a blocker, please remove yourself as reviewer, else, please continue. :) 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. 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 ================ (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. Just assume it's GPL, KDE has a global licensing policy for a reason. > 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 ? (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. :) (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 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.) 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. > 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.
Oops, I meant writing just "GPLv2+" instead of "LGPLv2+ and GPLv2+", not "or". Either way, my point stands. :-) Please request some branches. I'm assuming you don't want an empty cvs add. :P 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. cvs done. Package imported and built: http://koji.fedoraproject.org/koji/taskinfo?taskID=560246 Thanks all! |