Bug 415211

Summary: Review Request: WebKit - Web content engine library
Product: [Fedora] Fedora Reporter: Peter Gordon <peter>
Component: Package ReviewAssignee: Tim Lauridsen <tim.lauridsen>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, lemenkov, mtasaka, notting, rdieter
Target Milestone: ---Flags: tim.lauridsen: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 1.0.0-0.3.svn28482.fc7 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-12-12 19:53:07 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 Peter Gordon 2007-12-07 04:19:56 UTC
Spec URL: http://pgordon.fedorapeople.org/for-review/WebKit.spec
SRPM URL: http://pgordon.fedorapeople.org/for-review/WebKit-1.0.0-0.2.svn28482.src.rpm

Description: WebKit is an open source web browser engine.

Comment 1 Peter Gordon 2007-12-07 04:27:16 UTC
*** Bug 411471 has been marked as a duplicate of this bug. ***

Comment 2 Tim Lauridsen 2007-12-07 12:30:57 UTC
MUST:

X rpmlint must be silent
  $ rpmlint ~/rpmbuild/SRPMS/WebKit-1.0.0-0.2.svn28482.fc8.src.rpm 
  $ rpmlint ~/rpmbuild/RPMS/i386/WebKit-gtk-1.0.0-0.2.svn28482.fc8.i386.rpm 
  WebKit-gtk.i386: W: no-documentation
  $ rpmlint ~/rpmbuild/RPMS/i386/WebKit-gtk-devel-1.0.0-0.2.svn28482.fc8.i386.rpm 
  WebKit-gtk-devel.i386: W: no-documentation
  WebKit-gtk-devel.i386: E: only-non-binary-in-usr-lib
  $ rpmlint ~/rpmbuild/RPMS/i386/WebKit-qt-
  WebKit-qt-1.0.0-0.2.svn28482.fc8.i386.rpm       
WebKit-qt-devel-1.0.0-0.2.svn28482.fc8.i386.rpm
  $ rpmlint ~/rpmbuild/RPMS/i386/WebKit-qt-1.0.0-0.2.svn28482.fc8.i386.rpm 
  WebKit-qt.i386: W: no-documentation
  $ rpmlint ~/rpmbuild/RPMS/i386/WebKit-qt-devel-1.0.0-0.2.svn28482.fc8.i386.rpm 
  WebKit-qt-devel.i386: W: no-documentation
  $ rpmlint ~/rpmbuild/RPMS/i386/WebKit-doc-1.0.0-0.2.svn28482.fc8.i386.rpm 
  $ rpmlint ~/rpmbuild/RPMS/i386/WebKit-debuginfo-1.0.0-0.2.svn28482.fc8.i386.rpm 

  W: no-documentation should be ok, because docs is in doc package.

  E: only-non-binary-in-usr-lib, i am not sure what this means.

* source match upstream
  $ sha1sum ~/rpmbuild/SOURCES/WebKit-r28482.tar.bz2 
  52b8534bff2727ca6cff39ee87d6c41417ec8e1c 
/home/tim/rpmbuild/SOURCES/WebKit-r28482.tar.bz2
  $ sha1sum WebKit-r28482.tar.bz2 
  52b8534bff2727ca6cff39ee87d6c41417ec8e1c  WebKit-r28482.tar.bz2


* package is named appropriately
* it is legal for Fedora to distribute this
* license field matches the actual license.
* license is open source-compatible.
* specfile name matches %{name}
* summary and description fine
* correct buildroot
* %{?dist} is used
* license text included in package and marked with %doc
* package meets FHS (http://www.pathname.com/fhs/)
* changelog format fine 
* Packager tag not used
* Vendor tag not used
* Distribution tag not used
* License used and not Copyright 
* Summary tag does not end in a period
* specfile is legible
* package successfully compiles and builds on at least x86
X make sure lines are <= 80 characters
  PROMBLEM : Some lines are longer than 80 chars.
* specfile written in American English
* doc goes in a -doc sub-package
* /sbin/ldconfig used in packages containing libraries.
* no rpath.
* no a gui app
* header files goes into -devel sub-package.
* *.so goes into -devel sub-package.
* devel package require the base package using a fully versioned dependency
* macros used appropriately and consistently
* no %makeinstall
* install section must begin with rm -rf $RPM_BUILD_ROOT or %{buildroot}
* no locales
* split Requires(pre,post) into two separate lines
* package not relocatable
* package contains code
* package owns all directories and files
* no %files duplicates
X %defattrs present ( %defattr(-, root, root, -))
  PROBLEM : %defattr(-,root,root,-) is missing in qt & qt-devel %files section.
* %clean present
* %doc files do not affect runtime



SHOULD:
* package should include license text in the package and mark it with %doc
* package should build on i386
? package should build in mock
 - I haven't tried, but should not be a problem

Comment 3 Mamoru TASAKA 2007-12-07 13:43:30 UTC
I just tried to rebuild on dist-f9, however it failed at least on
ppc.
http://koji.fedoraproject.org/koji/taskinfo?taskID=281420

Comment 4 Rex Dieter 2007-12-07 13:48:53 UTC
Be aware, if you choose to include qt(4) support here, it will (likely?) be 
short-lived.  WebKit is slated to be included in qt-4.4 proper.

Comment 5 Peter Gordon 2007-12-07 21:22:21 UTC
(In reply to comment #2)
> MUST:
> 
> X rpmlint must be silent
>   E: only-non-binary-in-usr-lib, i am not sure what this means.

This is ignorable, as the devel subpackages contain only the DSO symlink, header
files, and some other build data (such as a pkgconfig file). 

> X make sure lines are <= 80 characters
>   PROMBLEM : Some lines are longer than 80 chars.

Everything that is user-visible is within the 80-char limit; and the only things
that are not are long path names and such whose split would make it even more
messy, which I'd hate to do.


> X %defattrs present ( %defattr(-, root, root, -))
>   PROBLEM : %defattr(-,root,root,-) is missing in qt & qt-devel %files section.

Oopsie; I fixed this in the 0.3 release.



(In reply to comment #3)
> I just tried to rebuild on dist-f9, however it failed at least on
> ppc.
> http://koji.fedoraproject.org/koji/taskinfo?taskID=281420
Gaah, stupid assembly in the spinlocking. Sorry about that. I've added a patch
which forces use of the pthread implementation instead of handcoded ASM. That
seems to have fixed it:
http://koji.fedoraproject.org/koji/taskinfo?taskID=282220(In reply to comment #4)



(In reply to comment #4)
> Be aware, if you choose to include qt(4) support here, it will (likely?) be 
> short-lived.  WebKit is slated to be included in qt-4.4 proper.

Ah, I didn't about its inclusion plan for Qt 4.4; but when that is released, it
should be a simple enough effort to Obsoletes/Provides the WebKit-qt and
WebKit-qt-devel packages for a smooth upgrade. :)


Updated spec: http://pgordon.fedorapeople.org/for-review/WebKit.spec
Updated SRPM:
http://pgordon.fedorapeople.org/for-review/WebKit-1.0.0-0.3.svn28482.fc8.src.rpm

* Thu Dec 06 2007 Peter Gordon <peter> 1.0.0-0.3.svn28482
- Add proper %%defattr line to qt, qt-devel, and doc subpackages.
- Add patch to forcibly build the TCSpinLock code using the pthread
  implementation:
  + TCSpinLock-use-pthread-stubs.patch


Thanks for the review!

Comment 6 Tim Lauridsen 2007-12-09 08:05:23 UTC
Looks good.

APPROVED.



Comment 7 Peter Gordon 2007-12-09 08:43:51 UTC
Awesome. Thanks for the swift review!

New Package CVS Request
=======================
Package Name: WebKit
Short Description: Web content engine library
Owners: pgordon
Branches: devel F-8 F-7
InitialCC: (none)
Cvsextras Commits: Yes

Comment 8 Peter Gordon 2007-12-11 18:54:13 UTC
Err; forgot to set the fedora-cvs flag. Stupid me.

Comment 9 Kevin Fenzi 2007-12-11 21:21:55 UTC
cvs done.

Comment 10 Fedora Update System 2007-12-12 19:53:05 UTC
WebKit-1.0.0-0.3.svn28482.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 11 Fedora Update System 2007-12-12 19:57:20 UTC
WebKit-1.0.0-0.3.svn28482.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.