Bug 506232 - Review Request: rekonq - KDE browser based on Webkit
Review Request: rekonq - KDE browser based on Webkit
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Orcan Ogetbil
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-06-16 05:01 EDT by Eelko Berkenpies
Modified: 2009-10-06 06:12 EDT (History)
6 users (show)

See Also:
Fixed In Version: 0.2.0-3.fc10
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-10-06 06:00:23 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
oget.fedora: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Eelko Berkenpies 2009-06-16 05:01:49 EDT
Spec URL: http://kaboon.fedorapeople.org/rekonq/0.1.0/rekonq.spec

SRPM URL: http://kaboon.fedorapeople.org/rekonq/0.1.0/F11/rekonq-0.1.0-1.fc11.src.rpm

Description: 

rekonq is a KDE browser based on Webkit. Its code is based on Nokia QtDemoBrowser, just like Arora. It's implementation is going to embrace KDE technologies to have a full-featured KDE web browser.

Rekonq currently provides:

* A good tabbed browsing experience
* Download files through KDE download system
* Share bookmarks with Konqueror
* Navigate in a proxied net
* Browse anonymously
* Inspect web pages
Comment 1 Orcan Ogetbil 2009-07-02 23:29:17 EDT
Here's my review for this:

* Buildroot doesn't obey the guidelines:
   http://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

* The source files claim GPLv2+ but the COPYING file says GPLv3. This software cannot be GPLv3 because it links to kdebase-workspace which is GPLv2. GPLv2 and GPLv3 don't go very well together. See:
   http://fedoraproject.org/wiki/Licensing#GPL_Compatibility_Matrix
The license tag should be GPLv2+ but please ask upstream and verify this. Also, please don't package the false COPYING file.

* Source0 must be given in full URL. See:
   https://fedoraproject.org/wiki/Packaging/SourceURL#Referencing_Source

* build.log says
   NOTE: msgfmt not found. Translations will *not* be installed
You need to add gettext to BR.

! Please make the description span 80 columns (as much as possible).

* Packages containing GUI applications must include a %{name}.desktop file, and that file must be properly installed with desktop-file-install in the %install section. Follow:
   http://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files
Also, rpmlint says:
   rekonq.x86_64: E: invalid-desktopfile /usr/share/applications/kde4/rekonq.desktop

* ScriptletSnippets aren't used properly. Follow:
   http://fedoraproject.org/wiki/Packaging/ScriptletSnippets

! Please use the %{name} macro extensively for consistency

* A package must own all directories that it creates. If it does not create a directory that it uses, then it should require a package which does create that directory.
Currently, package does not own %{_kde4_appsdir}/rekonq/ . You can replace
   %{_kde4_appsdir}/rekonq/rekonqui.rc
   %{_kde4_appsdir}/rekonq/pics/loading.gif
   %{_kde4_appsdir}/rekonq/defaultbookmarks.xbel
   %{_kde4_appsdir}/rekonq/htmls/notfound.html
   %{_kde4_appsdir}/rekonq/pics/webkit-icon.png
with just
   %{_kde4_appsdir}/%{name}/
to deal with this issue.

? You are using versioned BR. Why are you not using the corresponding versioned R? 

! Also the versions you use for BRs do not match what CMakeLists.txt file says.
Comment 2 Eelko Berkenpies 2009-07-03 03:19:22 EDT
Orcan,

Thank you very much for your detailed and helpful package review ( you found a bit more issues than I was hoping for :) ). I think I've covered most of the issues you found by now. I'm now waiting for upstream to comment on the license part.

I'll post the updated files as soon as I've fully (I hope) sorted things out (can't access my Fedora account from here either). 

- Eelko
Comment 3 Ben Boeckel 2009-07-27 17:03:35 EDT
Ping. Status update on this?
Comment 4 Eelko Berkenpies 2009-08-04 07:48:20 EDT
Sorry for the wait, guys. I've been on vacation. 

As far as I have figured out for now ( I've got tons of e-mail to go through :/ ), this is still an upstream issue and under discussion for the next release.
Comment 5 Eelko Berkenpies 2009-08-06 02:52:46 EDT
Here's an upstream reference: http://mail.kde.org/pipermail/rekonq/2009-July/000230.html

Still ongoing as far as I know.
Comment 6 Kevin Kofler 2009-08-25 16:52:25 EDT
Shipping the GPLv3 with a software which is GPLv2+ is legal. You can specify any minimum version, it doesn't have to be the one you ship. You're only required to ship a copy of any (one) version of the GPL which is valid for your software, not all of them. So, I don't see any licensing issue there, it's safe to mark this as GPLv2+ if the license header says so.

Plus, are you sure the parts of kdebase-workspace which are being used are not actually LGPL or GPLv2+? GPL v2 only is deprecated in KDE, most stuff is not actually v2 only. But this isn't really relevant as the code is GPLv2+ anyway.
Comment 7 Eelko Berkenpies 2009-08-25 17:01:06 EDT
Finally an update;

Spec URL: http://kaboon.fedorapeople.org/rekonq/0.2.0/rekonq.spec

SRPM URL:
http://kaboon.fedorapeople.org/rekonq/0.2.0/F11/rekonq-0.2.0-1.fc11.src.rpm

I fixed the spec file and updated to the latest upstream version. I think I
fixed most, if not all, issues reported earlier.
Comment 8 Orcan Ogetbil 2009-08-26 16:25:20 EDT
Hi Eelko, thanks for the update.

I had a look at the new package:

* The BR's are messed up. You want to BR qt-devel and kde-workspace-devel. Not just qt and kde-workspace! Otherwise it won't build on koji.

I suggest you running mock or the scratch build feature of koji before posting updates.

Other than this, the following issues are still there:

(from comment #1)

> * ScriptletSnippets aren't used properly. Follow:
>    http://fedoraproject.org/wiki/Packaging/ScriptletSnippets
> 

> * Currently, package does not own %{_kde4_appsdir}/rekonq/ .
Comment 9 Eelko Berkenpies 2009-08-26 16:48:52 EDT
Alright, thanks again Orcan.

I intentionally mixed/messed up the BR's. Somehow I thought it the -devel packes  weren't needed to build it as it build perfectly fine without them on my system (afaik). I'll change that back.

While I'm at it, I'll also try your other suggestions and fixes and will get back on it as soon as possible.

Yet again, thanks so far!

- Eelko
Comment 10 Eelko Berkenpies 2009-08-29 06:18:03 EDT
Alright, here it goes again. The scratch build was succesfull and I think I have resolved the remaining issues also. I sorted out the BR's and the ScriptletSnippets as far as I can tell. I'm not sure if I dealt correctly the ownership issue.

Spec URL: http://kaboon.fedorapeople.org/rekonq/0.2.0/rekonq.spec

Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1642555
Comment 11 Orcan Ogetbil 2009-08-30 16:34:59 EDT
Hi Eelko,
The directory is still unowned. I gave the solution to this problem above in comment #1. Let me rephrase:

   %{_kde4_appsdir}/%{name}/* owns everything inside the directory but does not own the directory itself.

   %dir %{_kde4_appsdir}/%{name} owns just the directory but does not own the contents of the directory.

   %{_kde4_appsdir}/%{name}/ owns both the directory and its contents. This is what you want.

Alternatively you can do
   %dir %{_kde4_appsdir}/%{name}
   %{_kde4_appsdir}/%{name}/*
which is longer but has the same effect.
Comment 12 Eelko Berkenpies 2009-08-31 02:13:45 EDT
Hi Orcan,

Thank you very much for that last comment with example! I now fully understand the issue which is being referred to.

I will get that fixed this evening.
Comment 13 Eelko Berkenpies 2009-08-31 14:43:30 EDT
'lo Orcan,

I chose the last way to add the ownership. I think it's easier to read and better to understand (for me at least - so I will not forget it next time when I submit a package for review). :)

Hopefully the last and final update;

Spec URL: http://kaboon.fedorapeople.org/rekonq/0.2.0/rekonq.spec

Koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1646581

- Eelko
Comment 14 Orcan Ogetbil 2009-09-15 03:00:37 EDT
Hi Eelko,
Sorry, I've been busy with life. I will finish the review later this week.
Comment 15 Orcan Ogetbil 2009-09-17 04:29:36 EDT
Everything seems good now. Sorry for the delay!

-----------------------------------------
This package (rekonq) is APPROVED by oget
-----------------------------------------
Comment 16 Eelko Berkenpies 2009-09-17 06:40:01 EDT
Thank you very much Orcan. Getting this package reviewed has been quite a good learning process for me. I don't think I'll ever miss out on those pesky little details again. :)

New Package CVS Request
=======================
Package Name: rekonq
Short Description: KDE browser based on QtWebkit
Owners: kaboon
Branches: F-11 F-12
InitialCC: kaboon
Comment 17 Kevin Fenzi 2009-09-17 15:11:11 EDT
cvs done.
Comment 18 Fedora Update System 2009-09-19 08:04:18 EDT
rekonq-0.2.0-3.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/rekonq-0.2.0-3.fc11
Comment 19 Eelko Berkenpies 2009-09-20 15:21:51 EDT
I am sorry and I've should have done this straight away;

Package Change Request
======================
Package Name: rekonq
New Branches: F-10
Owners: kaboon
Comment 20 Jason Tibbitts 2009-09-21 21:34:30 EDT
CVS done.
Comment 21 Fedora Update System 2009-09-22 13:05:00 EDT
rekonq-0.2.0-3.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/rekonq-0.2.0-3.fc10
Comment 22 Fedora Update System 2009-09-24 01:10:15 EDT
rekonq-0.2.0-3.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update rekonq'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-9826
Comment 23 Fedora Update System 2009-09-24 01:16:36 EDT
rekonq-0.2.0-3.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update rekonq'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-9865
Comment 24 Ben Boeckel 2009-10-02 20:01:05 EDT
Should this be closed now that it is in the repos?
Comment 25 Kevin Kofler 2009-10-02 20:56:44 EDT
Bodhi is going to close it once the updates go stable. (By the way, they should be queued for stable now, one week of testing is enough!) But review requests can in principle be closed as soon as the package is imported.
Comment 26 Eelko Berkenpies 2009-10-03 03:44:31 EDT
Now marked as stable. I'll leave it up to Bodhi to close this one for me and next time I'll do it myself. :)

Thanks
Comment 27 Fedora Update System 2009-10-06 06:00:16 EDT
rekonq-0.2.0-3.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.
Comment 28 Fedora Update System 2009-10-06 06:12:28 EDT
rekonq-0.2.0-3.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

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