Bug 1601186 - Review Request: wpewebkit - A WebKit port optimized for low-end devices
Summary: Review Request: wpewebkit - A WebKit port optimized for low-end devices
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Robert-André Mauchin 🐧
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On: 1601058 1602078
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-07-14 21:00 UTC by Chris King
Modified: 2021-06-11 14:12 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-06-11 14:12:57 UTC
Type: ---
Embargoed:
zebob.m: fedora-review+


Attachments (Terms of Use)

Description Chris King 2018-07-14 21:00:36 UTC
Spec URL: https://copr-be.cloud.fedoraproject.org/results/bunnyapocalypse/WPE-webkit/fedora-rawhide-x86_64/00777347-wpewebkit/wpewebkit.spec

SRPM URL: https://copr-be.cloud.fedoraproject.org/results/bunnyapocalypse/WPE-webkit/fedora-rawhide-x86_64/00777347-wpewebkit/wpewebkit-2.20.1-2.fc29.src.rpm

Description: WPE allows embedders to create simple and performant systems based on Web platform technologies. It is designed with hardware acceleration in mind, leveraging common 3D graphics APIs for best performance. This is also kind-of my first package (my other one being the backend for this package, found here: https://bugzilla.redhat.com/show_bug.cgi?id=1601058), and I still need a sponsor.

Fedora Account System Username: bunnyapocalypse

Additional info: I am unable to show that this builds successfully on koji servers etc as it depends on wpebackend, a package which has not yet been packaged yet. I can, however, verify that it does build properly on my personal computer once I have installed the backend from the copr repository that I've made for it. 

2.) I am unsure if some of the patches included in the webkit2gtk3 package that I based this spec file off of (https://src.fedoraproject.org/rpms/webkit2gtk3/blob/master/f/webkit2gtk3.spec) were needed for this package, nor how to go about adding those should they be necessary as they were all made for webkit2gtk3. I have an inkling that some of them, specifically the fedora-crypto-policy and python2 patch are necessary. 

3.) Finally, to make the package build on my machine I had to add a cmake option on line 102 of the spec that I feel is probably not ideal, so any advice on that would be welcome.

Comment 1 Michael Catanzaro 2018-07-15 00:46:58 UTC
Were you able to successfully run the minibrowser? It cannot work unless you additionally package a WPE backend (e.g. WPEBackend-fdo).

(In reply to Chris King from comment #0)
> 2.) I am unsure if some of the patches included in the webkit2gtk3 package
> that I based this spec file off of
> (https://src.fedoraproject.org/rpms/webkit2gtk3/blob/master/f/webkit2gtk3.
> spec) were needed for this package, nor how to go about adding those should
> they be necessary as they were all made for webkit2gtk3. I have an inkling
> that some of them, specifically the fedora-crypto-policy and python2 patch
> are necessary. 

We'll need to duplicate the fedora-crypto patch in both packages forever, yes.

User agent patch is optional.

The python patch should only be needed to avoid a build failure, so if it builds successfully on rawhide without it, then it should probably be dropped.

The other patches should really go upstream, so we can drop them. I managed to upstream the page size patch recently. The big endian patch is harder.

> 3.) Finally, to make the package build on my machine I had to add a cmake
> option on line 102 of the spec that I feel is probably not ideal, so any
> advice on that would be welcome.

-DCMAKE_EXE_LINKER_FLAGS="${CMAKE_EXE_LINKER_FLAGS} -lrt" should indeed not be needed, would be good to add a FIXME to figure out how to remove that.

Comment 2 Chris King 2018-07-15 03:02:32 UTC
(In reply to Chris King from comment #0)

> I am unable to show that this builds successfully on koji
> servers etc as it depends on wpebackend

I did not realize you could upload multiple packages to a copr repository.

Here's the build: https://copr.fedorainfracloud.org/coprs/bunnyapocalypse/WPE-webkit/build/777394/

A slightly updated spec: https://copr-be.cloud.fedoraproject.org/results/bunnyapocalypse/WPE-webkit/fedora-rawhide-x86_64/00777394-wpewebkit/wpewebkit.spec

And a matching srpm: https://copr-be.cloud.fedoraproject.org/results/bunnyapocalypse/WPE-webkit/fedora-rawhide-x86_64/00777394-wpewebkit/wpewebkit-2.20.1-2.fc29.src.rpm

Seems to be failing because it thinks that the installed ruby version isn't new enough? hmmmm...

Comment 3 Chris King 2018-07-17 14:12:47 UTC
(In reply to Michael Catanzaro from comment #1)
> Were you able to successfully run the minibrowser? It cannot work unless you
> additionally package a WPE backend (e.g. WPEBackend-fdo).

That is something I've been wondering about, the minibrowser isn't even in the install files, despite my building with the cmake flage of ENABLE_MINIBROWSER=ON. I don't know as much about the inner workings of webkit/WPE as you probably do, so help on this would be great. 

I have packaged wpebackend-fdo 0.1 in my copr repository, I can also make a package review request for that if you think that'll be necessary.

I also wouldn't be against possibly packaging the cog frontend, but it doesn't seem to be mature enough yet to have any versioned releases(?)

> We'll need to duplicate the fedora-crypto patch in both packages forever,
> yes.

Okay, I've included that in the new spec.

> User agent patch is optional.

In that case I won't include it.
 
> The python patch should only be needed to avoid a build failure, so if it
> builds successfully on rawhide without it, then it should probably be
> dropped.

My build is succeeding on f28 but not on rawhide, I think that this might be the issue. currently running a copr build with the python2 patch and I'll get back to you on if that fixes it.

> -DCMAKE_EXE_LINKER_FLAGS="${CMAKE_EXE_LINKER_FLAGS} -lrt" should indeed not
> be needed, would be good to add a FIXME to figure out how to remove that.

Trying to build without this fails in copr, so I guess it's not just my machine, what is the process for adding a FIXME?
 
New spec: https://copr-be.cloud.fedoraproject.org/results/bunnyapocalypse/WPE-webkit/fedora-rawhide-x86_64/00777901-wpewebkit/wpewebkit.spec

New srpm: https://copr-be.cloud.fedoraproject.org/results/bunnyapocalypse/WPE-webkit/fedora-rawhide-x86_64/00777901-wpewebkit/wpewebkit-2.20.1-2.fc29.src.rpm

Thanks for all of the help.

Comment 4 Michael Catanzaro 2018-07-17 15:44:39 UTC
(In reply to Chris King from comment #3)
> That is something I've been wondering about, the minibrowser isn't even in
> the install files, despite my building with the cmake flage of
> ENABLE_MINIBROWSER=ON. I don't know as much about the inner workings of
> webkit/WPE as you probably do, so help on this would be great. 

Hmm, the MiniBrowser was added after 2.20, so you'll have to package a newer tarball. 2.20 is the latest stable version (appropriate for F28), but you probably want the latest unstable for rawhide, which will have MiniBrowser.

> I have packaged wpebackend-fdo 0.1 in my copr repository, I can also make a
> package review request for that if you think that'll be necessary.

It's definitely necessary, or it won't be able to run at all.

> > -DCMAKE_EXE_LINKER_FLAGS="${CMAKE_EXE_LINKER_FLAGS} -lrt" should indeed not
> > be needed, would be good to add a FIXME to figure out how to remove that.
> 
> Trying to build without this fails in copr, so I guess it's not just my
> machine, what is the process for adding a FIXME?

Just add a comment in the spec file.

Comment 5 Chris King 2018-07-17 20:22:16 UTC
(In reply to Michael Catanzaro from comment #4)
> (In reply to Chris King from comment #3)
> > That is something I've been wondering about, the minibrowser isn't even in
> > the install files, despite my building with the cmake flage of
> > ENABLE_MINIBROWSER=ON. I don't know as much about the inner workings of
> > webkit/WPE as you probably do, so help on this would be great. 
> 
> Hmm, the MiniBrowser was added after 2.20, so you'll have to package a newer
> tarball. 2.20 is the latest stable version (appropriate for F28), but you
> probably want the latest unstable for rawhide, which will have MiniBrowser.

Okay, I have a build running for that now in a different copr repo. Hopefully the same spec works for that as well: https://copr.fedorainfracloud.org/coprs/bunnyapocalypse/wpewebkit-rawhide/build/778315/

> > I have packaged wpebackend-fdo 0.1 in my copr repository, I can also make a
> > package review request for that if you think that'll be necessary.
> 
> It's definitely necessary, or it won't be able to run at all.

Okay, I have made a package review request for that: https://bugzilla.redhat.com/show_bug.cgi?id=1602078

> > Trying to build without this fails in copr, so I guess it's not just my
> > machine, what is the process for adding a FIXME?
> 
> Just add a comment in the spec file.

Done.

The python patch was indeed necessary, both 28 and rawhide are building properly for 2.20.1 at least. Will update on 2.21.2 when that's done.

New spec: https://copr-dist-git.fedorainfracloud.org/cgit/bunnyapocalypse/wpewebkit-rawhide/wpewebkit.git/tree/wpewebkit-rawhide.spec?id=f42b42524c6894b95bc5607c5c3da7bf0ced08e0

Comment 6 Michael Catanzaro 2018-07-18 00:47:58 UTC
These:

%{_libdir}/wpe-webkit-0.1/injected-bundle/libWPEInjectedBundle.so
%{_libdir}/wpe-webkit-0.1/libWPEWebInspectorResources.so

need to be in the main package, not the devel package. These aren't symlinks, they are private dlopened libraries, so there's none of the usual versioning. I see why you were confused, though.

By the way, I'm not an experienced Fedora reviewer, but I can review and approve packages. But I'm not a sponsor, so this will do no good unless you find a sponsor. I think you've already read https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group since you've blocked FE-NEEDSPONSOR, but check that page if you haven't already.

Comment 7 Michael Catanzaro 2018-07-18 17:08:44 UTC
Please post an updated SPRM URL as well.

Comment 9 Chris King 2018-07-19 13:07:43 UTC
(In reply to Michael Catanzaro from comment #8)
> Spec:
> https://copr-dist-git.fedorainfracloud.org/cgit/bunnyapocalypse/wpewebkit-
> rawhide/wpewebkit.git/plain/wpewebkit-rawhide.
> spec?id=f42b42524c6894b95bc5607c5c3da7bf0ced08e0

Sorry it took so long to get back to you, wpe took > 9 hours to build on the copr machine, and after all that it still says that it's failing although I don't really see why? Seems like something copr related.

spec: https://copr-be.cloud.fedoraproject.org/results/bunnyapocalypse/wpewebkit-rawhide/fedora-rawhide-x86_64/00778712-wpewebkit/wpewebkit-rawhide.spec

srpm: https://copr-be.cloud.fedoraproject.org/results/bunnyapocalypse/wpewebkit-rawhide/fedora-rawhide-x86_64/00778712-wpewebkit/wpewebkit-2.21.2-1.fc29.src.rpm

copr root, if you want to look at the build: https://copr-be.cloud.fedoraproject.org/results/bunnyapocalypse/wpewebkit-rawhide/fedora-rawhide-x86_64/00778712-wpewebkit/

Comment 10 Chris King 2018-07-24 20:50:03 UTC
I have made an unofficial review, for any potential sponsors lurking in these reviews:

https://bugzilla.redhat.com/show_bug.cgi?id=1379651

Comment 11 Chris King 2018-07-24 21:31:51 UTC
I have made another unofficial review: https://bugzilla.redhat.com/show_bug.cgi?id=1607554

Comment 12 Robert-André Mauchin 🐧 2018-07-25 17:27:36 UTC
Why %{_libexecdir}/wpe-webkit-0.1/MiniBrowser is in devel?

You rename licenses with the macro add_to_license_files but you don't include these renamed licenses in %files.

Comment 13 Chris King 2018-07-25 18:00:47 UTC
(In reply to Robert-André Mauchin from comment #12)
> Why %{_libexecdir}/wpe-webkit-0.1/MiniBrowser is in devel?
The minibrowser, will ideally not be used by the end user as the frontends for the WPE package become more mature. Looking at specs for other webkit packages, it seems standard practice to only include the minibrowser in devel, however I could see an argument for not doing that here due to the lack of mature frontends.

> You rename licenses with the macro add_to_license_files but you don't
> include these renamed licenses in %files.
Fixed that.

Spec: https://copr-dist-git.fedorainfracloud.org/cgit/bunnyapocalypse/wpewebkit-rawhide/wpewebkit.git/tree/wpewebkit-rawhide.spec?id=7d52f8a3911f1209c4f4d2b838e7944481cb864f

SRPM will come when copr is done with the new build.

Comment 14 Chris King 2018-07-25 18:27:18 UTC
I have made another unofficial review: https://bugzilla.redhat.com/show_bug.cgi?id=1469767

Comment 16 Robert-André Mauchin 🐧 2018-08-03 20:43:08 UTC
Build fails with:

Processing files: wpewebkit-devel-2.21.2-1.fc29.x86_64
RPM build errors:
error: File not found: /builddir/build/BUILDROOT/wpewebkit-2.21.2-1.fc29.x86_64/usr/libexec/wpe-webkit-0.1/MiniBrowser
    File not found: /builddir/build/BUILDROOT/wpewebkit-2.21.2-1.fc29.x86_64/usr/libexec/wpe-webkit-0.1/MiniBrowser


See https://koji.fedoraproject.org/koji/taskinfo?taskID=28820766

Comment 17 Chris King 2018-08-03 21:21:01 UTC
(In reply to Robert-André Mauchin from comment #16)
> Build fails with:
> 
> Processing files: wpewebkit-devel-2.21.2-1.fc29.x86_64
> RPM build errors:
> error: File not found:
> /builddir/build/BUILDROOT/wpewebkit-2.21.2-1.fc29.x86_64/usr/libexec/wpe-
> webkit-0.1/MiniBrowser
>     File not found:
> /builddir/build/BUILDROOT/wpewebkit-2.21.2-1.fc29.x86_64/usr/libexec/wpe-
> webkit-0.1/MiniBrowser

Hmmm, this may be due to it needing something like wpebackend-fdo to build the browser? I know Michael said it would definitely be needed to run it, so maybe it's also needed to build it... I'll need to look into this more.

Comment 18 Robert-André Mauchin 🐧 2018-08-04 17:09:12 UTC
Code from https://wpewebkit.org/releases/ doesn't contain Tools/MiniBrowser/ like on Github

Comment 19 Michael Catanzaro 2018-08-04 17:23:57 UTC
That will require the 2.21.3 release, then.

It is overdue, IMO. I will prod upstream.

Comment 20 Chris King 2018-08-06 15:00:36 UTC
(In reply to Michael Catanzaro from comment #19)
> That will require the 2.21.3 release, then.
> 
> It is overdue, IMO. I will prod upstream.

Until then, I guess I'll remove the expectation of the minibrowser being built just so we can finish the packaging process.

New SPEC: https://copr-dist-git.fedorainfracloud.org/cgit/bunnyapocalypse/wpewebkit-rawhide/wpewebkit.git/tree/wpewebkit-rawhide.spec?id=bd489f873558630947b76fd02f2de57a5e48c32c

new SRPM: https://bunnyapocalypse.space/wpewebkit-2.21.2-1.fc28.src.rpm

Comment 21 Robert-André Mauchin 🐧 2018-08-06 17:44:54 UTC
Still an issue with files installed but not packaged:


error: Installed (but unpackaged) file(s) found:
   /usr/include/wpe-webkit-0.1/jsc/JSCAutocleanups.h
   /usr/include/wpe-webkit-0.1/jsc/JSCClass.h
   /usr/include/wpe-webkit-0.1/jsc/JSCContext.h
   /usr/include/wpe-webkit-0.1/jsc/JSCDefines.h
   /usr/include/wpe-webkit-0.1/jsc/JSCException.h
   /usr/include/wpe-webkit-0.1/jsc/JSCValue.h
   /usr/include/wpe-webkit-0.1/jsc/JSCVersion.h
   /usr/include/wpe-webkit-0.1/jsc/JSCVirtualMachine.h
   /usr/include/wpe-webkit-0.1/jsc/JSCWeakValue.h
   /usr/include/wpe-webkit-0.1/jsc/jsc.h


https://koji.fedoraproject.org/koji/getfile?taskID=28885357&volume=DEFAULT&name=build.log&offset=-4000

Comment 22 Chris King 2018-08-06 19:55:06 UTC
(In reply to Robert-André Mauchin from comment #21)
> Still an issue with files installed but not packaged:
> 
> 
> error: Installed (but unpackaged) file(s) found:
>    /usr/include/wpe-webkit-0.1/jsc/JSCAutocleanups.h
>    /usr/include/wpe-webkit-0.1/jsc/JSCClass.h
>    /usr/include/wpe-webkit-0.1/jsc/JSCContext.h
>    /usr/include/wpe-webkit-0.1/jsc/JSCDefines.h
>    /usr/include/wpe-webkit-0.1/jsc/JSCException.h
>    /usr/include/wpe-webkit-0.1/jsc/JSCValue.h
>    /usr/include/wpe-webkit-0.1/jsc/JSCVersion.h
>    /usr/include/wpe-webkit-0.1/jsc/JSCVirtualMachine.h
>    /usr/include/wpe-webkit-0.1/jsc/JSCWeakValue.h
>    /usr/include/wpe-webkit-0.1/jsc/jsc.h
> 
> 
> https://koji.fedoraproject.org/koji/
> getfile?taskID=28885357&volume=DEFAULT&name=build.log&offset=-4000

Oops,

New spec: https://copr-dist-git.fedorainfracloud.org/cgit/bunnyapocalypse/wpewebkit-rawhide/wpewebkit.git/tree/wpewebkit-rawhide.spec?id=0040895813f53b25ee93bad42b1d84510c129114

SRPM updated at the same link

Comment 23 Michael Catanzaro 2018-08-07 00:46:46 UTC
%{_libdir}/libWPEWebKit-0.1.so.* <-- again, don't glob the first version component after the .so or we'll wind up with unexpected soname bumps

Comment 24 Chris King 2018-08-07 01:39:32 UTC
(In reply to Michael Catanzaro from comment #23)
> %{_libdir}/libWPEWebKit-0.1.so.* <-- again, don't glob the first version
> component after the .so or we'll wind up with unexpected soname bumps

Oops, sorry, probably should've done that when I did that for the other packages.

New spec: https://copr-dist-git.fedorainfracloud.org/cgit/bunnyapocalypse/wpewebkit-rawhide/wpewebkit.git/tree/wpewebkit-rawhide.spec?id=76dc59a7a84d22458ba889e7106fc5a2672551cf

Comment 25 Robert-André Mauchin 🐧 2018-08-07 15:22:49 UTC
 - Split the description to stay below 80 characters per line

 - Own these directories:

     Note: No known owner of /usr/libexec/wpe-webkit-0.1, /usr/lib64/wpe-
     webkit-0.1/injected-bundle, /usr/include/wpe-webkit-0.1, /usr/lib64
     /wpe-webkit-0.1




Package Review
==============

Legend:
[x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated
[ ] = Manual review needed



===== MUST items =====

C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Development (unversioned) .so files in -devel subpackage, if present.
     Note: Unversioned so-files in private %_libdir subdirectory (see
     attachment). Verify they are not in ld path.
[x]: If your application is a C or C++ application you must list a
     BuildRequires against gcc, gcc-c++ or clang.
[x]: Header files in -devel subpackage, if present.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.

Generic:
[x]: Package successfully compiles and builds into binary rpms on at least
     one supported primary architecture.
     Note: Using prebuilt packages
[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.
     Note: Checking patched sources after %prep for licenses. Licenses
     found: "MPL (v1.1) GPL (v2 or later) or LGPL (v2.1 or later)", "Public
     domain BSD (3 clause)", "MPL (v1.1) LGPL (v2 or later)", "BSD
     (unspecified)", "MPL (v1.1) LGPL (v2.1 or later)", "LGPL (v2.1)",
     "ISC", "*No copyright* Apache (v2.0)", "LGPL (v2 or later)", "GPL (v3
     or later)", "AFL (v2.0) LGPL (v2 or later)", "BSL", "BSD (2 clause)",
     "Apache (v2.0)", "GPL (v2 or later)", "MIT/X11 (BSD like)", "MPL
     (v1.1)", "*No copyright* LGPL", "*No copyright* Public domain", "*No
     copyright* BSD (unspecified)", "BSD (3 clause)", "MPL (v2.0)", "ICU",
     "LGPL (v2)", "Unknown or generated", "*No copyright* SGI Free Software
     License B", "LGPL (v2.1 or later)". 2544 files have unknown license.
     Detailed output of licensecheck in
     /home/bob/packaging/review/wpewebkit/review-wpewebkit/licensecheck.txt
[x]: License file installed when any subpackage combination is installed.
[!]: Package requires other packages for directories it uses.
     Note: No known owner of /usr/libexec/wpe-webkit-0.1, /usr/lib64/wpe-
     webkit-0.1/injected-bundle, /usr/include/wpe-webkit-0.1, /usr/lib64
     /wpe-webkit-0.1
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[x]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory
     names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[x]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[-]: Large documentation must go in a -doc subpackage. Large could be size
     (~1MB) or number of files.
     Note: Documentation size is 10240 bytes in 1 files.
[x]: Package complies to the Packaging Guidelines
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
     Note: There are rpmlint messages (see attachment).
[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 %license.
[x]: Package does not own files or directories owned by other packages.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Dist tag is present.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install DESTDIR=... doesn't
     work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package does not use a name that already exists.
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[x]: File names are valid UTF-8.
[x]: Packages must not store files under /srv, /opt or /usr/local

===== SHOULD items =====

Generic:
[x]: Reviewer should test that the package builds in mock.
[x]: If the source package does not include license text(s) as a separate
     file from upstream, the packager SHOULD query upstream to include it.
[x]: Final provides and requires are sane (see attachments).
[x]: Fully versioned dependency in subpackages if applicable.
     Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in
     wpewebkit-devel , wpewebkit-debuginfo , wpewebkit-debugsource
[?]: Package functions as described.
[x]: Latest version is packaged.
[x]: Package does not include license text files separate from upstream.
[x]: Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[-]: Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: Package should compile and build into binary rpms on all supported
     architectures.
[-]: %check is present and all tests pass.
[x]: Packages should try to preserve timestamps of original installed
     files.
[x]: Buildroot is not present
[x]: Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
[x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin.
[x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file
[x]: The placement of pkgconfig(.pc) files are correct.
[x]: Sources can be downloaded from URI in Source: tag
[x]: SourceX is a working URL.
[x]: Spec use %global instead of %define unless justified.

===== EXTRA items =====

Generic:
[x]: Rpmlint is run on debuginfo package(s).
     Note: No rpmlint messages.
[x]: Rpmlint is run on all installed packages.
     Note: There are rpmlint messages (see attachment).
[x]: Large data in /usr/share should live in a noarch subpackage if package
     is arched.
[x]: Spec file according to URL is the same as in SRPM.


Rpmlint
-------
Checking: wpewebkit-2.21.2-1.fc29.x86_64.rpm
          wpewebkit-devel-2.21.2-1.fc29.x86_64.rpm
          wpewebkit-debuginfo-2.21.2-1.fc29.x86_64.rpm
          wpewebkit-debugsource-2.21.2-1.fc29.x86_64.rpm
          wpewebkit-2.21.2-1.fc29.src.rpm
wpewebkit.x86_64: W: spelling-error %description -l en_US embedders -> embedded, Decembers, embers
wpewebkit.x86_64: W: spelling-error %description -l en_US performant -> perform ant, perform-ant, performance
wpewebkit.x86_64: E: description-line-too-long C WPE allows embedders to create simple and performant systems based on Web platform technologies.
wpewebkit.x86_64: E: description-line-too-long C It is designed with hardware acceleration in mind, leveraging common 3D graphics APIs for best performance.
wpewebkit.x86_64: W: shared-lib-calls-exit /usr/lib64/libWPEWebKit-0.1.so.2.1.0 _exit.5
wpewebkit.x86_64: W: shared-lib-calls-exit /usr/lib64/libWPEWebKit-0.1.so.2.1.0 exit.5
wpewebkit.x86_64: W: no-manual-page-for-binary WPEWebDriver
wpewebkit-devel.x86_64: W: no-documentation
wpewebkit.src: W: spelling-error %description -l en_US embedders -> embedded, Decembers, embers
wpewebkit.src: W: spelling-error %description -l en_US performant -> perform ant, perform-ant, performance
wpewebkit.src: E: description-line-too-long C WPE allows embedders to create simple and performant systems based on Web platform technologies.
wpewebkit.src: E: description-line-too-long C It is designed with hardware acceleration in mind, leveraging common 3D graphics APIs for best performance.
wpewebkit.src:124: W: macro-in-comment %license
5 packages and 0 specfiles checked; 4 errors, 9 warnings.

Comment 26 Robert-André Mauchin 🐧 2018-08-07 15:23:59 UTC
Also license should be:

LGPLv2+ and BSD

Comment 27 Chris King 2018-08-07 15:32:24 UTC
Fixed all of that, thanks again for reviewing.

New spec: https://copr-dist-git.fedorainfracloud.org/cgit/bunnyapocalypse/wpewebkit-rawhide/wpewebkit.git/tree/wpewebkit-rawhide.spec?id=2cba6e9992cecf9bc2d072b6a3ebcc9f20ed82e2

Updated srpm at same URL.

Comment 28 Robert-André Mauchin 🐧 2018-08-07 15:40:49 UTC
Looks good to me, package approved.

Comment 29 Gwyn Ciesla 2018-08-07 16:16:34 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/wpewebkit

Comment 30 Michael Catanzaro 2018-08-07 17:23:32 UTC
The spec is named:

wpewebkit-rawhide.spec

Should be renamed to wpewebkit.spec.

Comment 31 Michael Catanzaro 2018-08-07 17:31:30 UTC
Also, in the README and description: "inmind" -> "in mind"

Comment 32 Chris King 2018-08-07 17:46:00 UTC
Fixed.

Comment 33 Chris King 2018-08-07 21:17:37 UTC
My build seems to be failing on both ARM arches.

See here: https://koji.fedoraproject.org/koji/taskinfo?taskID=28904171

Looking into it, the -lrt flag I passed in my cmake section of the spec does not seem to be respected at the point in the build where it fails. 

Looking at the same part of the build where it fails on the ARM arches on other arches (such as x86), the -lrt option is being passed on other arches that built successfully, but not on the ARM arches. I don't believe any of the arch specific areas of the spec should effect this.

Any ideas why this may be?

Comment 34 Michael Catanzaro 2018-08-07 22:17:41 UTC
No clue.

Maybe it's time to figure out why you needed to specify -lrt in CMAKE_EXE_LINKER_FLAGS, since of course that should not be necessary.

I see the error you're getting for ARM is:

/usr/bin/ld: CMakeFiles/WebKit.dir/__/__/DerivedSources/WebKit/unified-sources/UnifiedSource10.cpp.o: in function `WebKit::SharedMemory::create(void*, unsigned long, WebKit::SharedMemory::Protection)':
/builddir/build/BUILD/wpewebkit-2.21.2/Source/WebKit/Platform/unix/SharedMemoryUnix.cpp:119: undefined reference to `shm_open'
/usr/bin/ld: /builddir/build/BUILD/wpewebkit-2.21.2/Source/WebKit/Platform/unix/SharedMemoryUnix.cpp:142: undefined reference to `shm_unlink'
/usr/bin/ld: /builddir/build/BUILD/wpewebkit-2.21.2/Source/WebKit/Platform/unix/SharedMemoryUnix.cpp:138: undefined reference to `shm_unlink'
collect2: error: ld returned 1 exit status

And shm_open(3) does say clearly: "Link with -lrt."

I see in Source/WebKit/CMakeLists.txt:

if (UNIX)
    check_function_exists(shm_open SHM_OPEN_EXISTS)
    if (NOT SHM_OPEN_EXISTS)
        set(CMAKE_REQUIRED_LIBRARIES rt)
        check_function_exists(shm_open SHM_OPEN_REQUIRES_LIBRT)
        if (SHM_OPEN_REQUIRES_LIBRT)
            list(APPEND WebKit_LIBRARIES PRIVATE rt)
        endif ()
        unset(CMAKE_REQUIRED_LIBRARIES)
    endif ()
endif ()

And in the aarch64 build log:

-- Looking for shm_open
-- Looking for shm_open - found

If -lrt were required for shm_open, I would have expected it to have printed:

-- Looking for shm_open
-- Looking for shm_open - not found
-- Looking for shm_open
-- Looking for shm_open - found

That output is the same on x86_64, as well.

So something seems to be going wrong there, which I don't understand. That problem creates the need for you to manually pass -lrt. And then something additional is going wrong on ARM for the -lrt to get lost....

Comment 35 Michael Catanzaro 2018-08-07 22:19:32 UTC
I notice https://trac.webkit.org/changeset/230385/webkit touched the librt check recently. May or may not be related.

Comment 36 Robert-André Mauchin 🐧 2018-08-08 05:39:39 UTC
Following this cmake script:

# Use ld.gold if it is available and isn't disabled explicitly
CMAKE_DEPENDENT_OPTION(USE_LD_GOLD "Use GNU gold linker" ON
                       "NOT CXX_ACCEPTS_MFIX_CORTEX_A53_835769;NOT ARM_TRADITIONAL_DETECTED;NOT WIN32;NOT APPLE" OFF)
if (USE_LD_GOLD)
    execute_process(COMMAND ${CMAKE_C_COMPILER} -fuse-ld=gold -Wl,--version ERROR_QUIET OUTPUT_VARIABLE LD_VERSION)
    if ("${LD_VERSION}" MATCHES "GNU gold")
        set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fuse-ld=gold -Wl,--disable-new-dtags")
        set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fuse-ld=gold -Wl,--disable-new-dtags")
    else ()
        message(WARNING "GNU gold linker isn't available, using the default system linker.")
        set(USE_LD_GOLD OFF)
    endif ()
endif ()


we can see that CMAKE_SHARED_LINKER_FLAGS is not set to ${CMAKE_EXE_LINKER_FLAGS} om ARM arches.

Thus you should add it in your cmake call:

%cmake \
  -DPORT=WPE \
  -DCMAKE_BUILD_TYPE=Release \
  -DENABLE_MINIBROWSER=ON \
  -DCMAKE_EXE_LINKER_FLAGS="${CMAKE_EXE_LINKER_FLAGS} -lrt" \
  -DCMAKE_SHARED_LINKER_FLAGS="${CMAKE_SHARED_LINKER_FLAGS} -lrt" \
%ifarch s390 aarch64
  -DUSE_LD_GOLD=OFF \
%endif
%ifarch s390 s390x ppc %{power64}
  -DENABLE_JIT=OFF \
  -DUSE_SYSTEM_MALLOC=ON \
%endif

See successful build: https://koji.fedoraproject.org/koji/taskinfo?taskID=28909553

Comment 37 Michael Catanzaro 2018-08-08 14:27:04 UTC
Thanks for finding a workaround.

Still, it's a shame we don't understand why the existing test for whether -lrt is needed is insufficient. Oh well.

Comment 38 Chris King 2018-08-08 14:28:10 UTC
(In reply to Robert-André Mauchin from comment #36)
> we can see that CMAKE_SHARED_LINKER_FLAGS is not set to
> ${CMAKE_EXE_LINKER_FLAGS} om ARM arches.

Oh sweet, thank you so much for looking into this, that probably would've taken me forever to find (if I ever did).

That said, I think Michael is right in that this solution is a band-aid and we should probably ask upstream about this.

For now though, I have made that change and am attempting a build again.

Comment 39 Mattia Verga 2021-06-11 14:12:57 UTC
Package was imported and then retired, closing ticket.


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