| Message ID | 20180323185942.60252-4-aperez@igalia.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Update WebKitGTK+ to 2.20.0 | expand |
>>>>> "Adrian" == Adrian Perez de Castro <aperez@igalia.com> writes: > Release notes: > https://webkitgtk.org/2018/03/12/webkitgtk2.20.0-released.html > No corresponding WebKit Security Advisory (WSA) has been published > for now. > This also adds a dependency on the woff2 package. In previous versions > WebKitGTK+ bundled a known-to-work snapshot of its Git repository due to > upstream not providing releases. The situation has changed and releases > are now available, and the bundled copy of woff2 has been removed. > Last but not least, this patch selects BR2_PACKAGE_WEBP_DEMUX, which is > now required. > Signed-off-by: Adrian Perez de Castro <aperez@igalia.com> > --- > Changes v1 -> v2: > - Marked as supported on AArch64, which is officially supported upstream. > - Add upstream patch for fixing build failure with multimedia/XSLT > support disabled. > --- > ...ld-failure-when-ENABLE_VIDEO-ENABLE_WEB_A.patch | 32 ++++++++++++++++++++++ > package/webkitgtk/Config.in | 3 ++ > package/webkitgtk/webkitgtk.hash | 8 +++--- > package/webkitgtk/webkitgtk.mk | 4 +-- Your introductory mail talked about 3 additional patches needed for aarch64 support, but I don't see them here?
Hello Adrian, On Sat, 24 Mar 2018 11:51:09 +0100, Peter Korsgaard wrote: > > ...ld-failure-when-ENABLE_VIDEO-ENABLE_WEB_A.patch | 32 ++++++++++++++++++++++ > > package/webkitgtk/Config.in | 3 ++ > > package/webkitgtk/webkitgtk.hash | 8 +++--- > > package/webkitgtk/webkitgtk.mk | 4 +-- > > Your introductory mail talked about 3 additional patches needed for > aarch64 support, but I don't see them here? Could you give some feedback on this question, so that we can move forward with applying the webkitgtk bump ? Thanks! Thomas
Hi! On Tue, 10 Apr 2018 09:53:27 +0200, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > Hello Adrian, > > On Sat, 24 Mar 2018 11:51:09 +0100, Peter Korsgaard wrote: > > > > ...ld-failure-when-ENABLE_VIDEO-ENABLE_WEB_A.patch | 32 ++++++++++++++++++++++ > > > package/webkitgtk/Config.in | 3 ++ > > > package/webkitgtk/webkitgtk.hash | 8 +++--- > > > package/webkitgtk/webkitgtk.mk | 4 +-- > > > > Your introductory mail talked about 3 additional patches needed for > > aarch64 support, but I don't see them here? > > Could you give some feedback on this question, so that we can move > forward with applying the webkitgtk bump ? The change for AArch64 is just listing BR2_aarch64 in WEBKITGTK_ARCH_SUPPORTS, the addition is in the first patch. Anyway, I was waiting a bit until the 2.20.1 release, which just happened: https://webkitgtk.org/2018/04/10/webkitgtk2.20.1-released.html Updating to 2.20.1 directly is IMHO better because it includes the fix for building with multimedia disabled, and a fix to improve memory consumption on devices with smaller amounts of memory — which is particularly welcome for embedded :-) I'm making a test build with 2.20.1 as I wrote this mail, if all goes well the new patch set will supersede this one. Cheers, -- Adrián 🎩
Hello, On Tue, 10 Apr 2018 16:54:09 +0100, Adrian Perez de Castro wrote: > The change for AArch64 is just listing BR2_aarch64 in WEBKITGTK_ARCH_SUPPORTS, > the addition is in the first patch. Then the following paragraph that you wrote: This series starts with the same three patches as the previous submission, and adds on top three additional commits to make WebKitGTK+ available on AArch64 (it's a platform we support upstream), and adds a couple of patches which are already merged in the upstream repository, but are not in the release and are worth applying. is a bit unclear. You're saying "adds on top three additional commits to make WebKitGTK+ available on AArch6". What those three additional commits ? For the record, your patch series was: brotli: new package woff2: new package webkitgtk: update to version 2.20.0 webkitgtk: Add upstream patch for better memory monitoring > Anyway, I was waiting a bit until the 2.20.1 release, which just happened: > > https://webkitgtk.org/2018/04/10/webkitgtk2.20.1-released.html > > Updating to 2.20.1 directly is IMHO better because it includes the fix for > building with multimedia disabled, and a fix to improve memory consumption > on devices with smaller amounts of memory — which is particularly welcome > for embedded :-) > > I'm making a test build with 2.20.1 as I wrote this mail, if all goes well > the new patch set will supersede this one. OK, thanks. Some clarification about the above would be nice. Perhaps it was just a mistake in your cover letter ? Best regards, Thomas
Hi Thomas, On Tue, 10 Apr 2018 17:57:38 +0200, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > On Tue, 10 Apr 2018 16:54:09 +0100, Adrian Perez de Castro wrote: > > > The change for AArch64 is just listing BR2_aarch64 in WEBKITGTK_ARCH_SUPPORTS, > > the addition is in the first patch. > > Then the following paragraph that you wrote: > > This series starts with the same three patches as the previous > submission, and adds on top three additional commits to make > WebKitGTK+ available on AArch64 (it's a platform we support > upstream), and adds a couple of patches which are already merged in > the upstream repository, but are not in the release and are worth > applying. > > is a bit unclear. You're saying "adds on top three additional commits > to make WebKitGTK+ available on AArch6". What those three additional > commits ? > > For the record, your patch series was: > > brotli: new package > woff2: new package > webkitgtk: update to version 2.20.0 > webkitgtk: Add upstream patch for better memory monitoring Ah, right: first I thought of making a separate patch for the small change of marking as available on AArch64. That, plus one commit for the upstream build fix, plus one commit for the upstream memory monitor patch. The first I ended up stashing with “webkitgtk: update to version 2.20.0”, and the other two are not going to be needed for 2.20.1 (they were merged upstream). Sorry if I wasn't being very clear here O:-) If you think enabling the package for AArch64 deserves its own separate commit, let me know and I'll split it back. FWIW, I've been trying AArch64 builds for the RaspberryPi 3, and they work fine; we also use them at work for a couple of clients as well without issues. Cheers, -- Adrián 🎩 > > Anyway, I was waiting a bit until the 2.20.1 release, which just happened: > > > > https://webkitgtk.org/2018/04/10/webkitgtk2.20.1-released.html > > > > Updating to 2.20.1 directly is IMHO better because it includes the fix for > > building with multimedia disabled, and a fix to improve memory consumption > > on devices with smaller amounts of memory — which is particularly welcome > > for embedded :-) > > > > I'm making a test build with 2.20.1 as I wrote this mail, if all goes well > > the new patch set will supersede this one. > > OK, thanks. Some clarification about the above would be nice. Perhaps > it was just a mistake in your cover letter ? > > Best regards, > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) > Embedded Linux and Kernel engineering > https://bootlin.com >
Hello, On Tue, 10 Apr 2018 17:18:26 +0100, Adrian Perez de Castro wrote: > > Then the following paragraph that you wrote: > > > > This series starts with the same three patches as the previous > > submission, and adds on top three additional commits to make > > WebKitGTK+ available on AArch64 (it's a platform we support > > upstream), and adds a couple of patches which are already merged in > > the upstream repository, but are not in the release and are worth > > applying. > > > > is a bit unclear. You're saying "adds on top three additional commits > > to make WebKitGTK+ available on AArch6". What those three additional > > commits ? > > > > For the record, your patch series was: > > > > brotli: new package > > woff2: new package > > webkitgtk: update to version 2.20.0 > > webkitgtk: Add upstream patch for better memory monitoring > > Ah, right: first I thought of making a separate patch for the small change > of marking as available on AArch64. That, plus one commit for the upstream > build fix, plus one commit for the upstream memory monitor patch. The first > I ended up stashing with “webkitgtk: update to version 2.20.0”, and the other > two are not going to be needed for 2.20.1 (they were merged upstream). OK, thanks for the clarification. > Sorry if I wasn't being very clear here O:-) > > If you think enabling the package for AArch64 deserves its own separate > commit, let me know and I'll split it back. FWIW, I've been trying AArch64 > builds for the RaspberryPi 3, and they work fine; we also use them at work > for a couple of clients as well without issues. I think it's OK to enable on AArch64 at the same time as you do the bump. It could be done in separate commits (each change stands on its own), but I'm fine with those changes being in the same commit. Best regards, Thomas
Hi again, On Tue, 10 Apr 2018 20:55:16 +0200, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > Hello, > > On Tue, 10 Apr 2018 17:18:26 +0100, Adrian Perez de Castro wrote: > > > > Then the following paragraph that you wrote: > > > > > > This series starts with the same three patches as the previous > > > submission, and adds on top three additional commits to make > > > WebKitGTK+ available on AArch64 (it's a platform we support > > > upstream), and adds a couple of patches which are already merged in > > > the upstream repository, but are not in the release and are worth > > > applying. > > > > > > is a bit unclear. You're saying "adds on top three additional commits > > > to make WebKitGTK+ available on AArch6". What those three additional > > > commits ? > > > > > > For the record, your patch series was: > > > > > > brotli: new package > > > woff2: new package > > > webkitgtk: update to version 2.20.0 > > > webkitgtk: Add upstream patch for better memory monitoring > > > > Ah, right: first I thought of making a separate patch for the small change > > of marking as available on AArch64. That, plus one commit for the upstream > > build fix, plus one commit for the upstream memory monitor patch. The first > > I ended up stashing with “webkitgtk: update to version 2.20.0”, and the other > > two are not going to be needed for 2.20.1 (they were merged upstream). > > OK, thanks for the clarification. Also, I have found out from one of my test builds that in both 2.20.x releases we have an ugly regression of the JSC JIT support on 32-bit MIPS and ARM when using the softfp ABI. A patch for this is at https://bugs.webkit.org/show_bug.cgi?id=183786 but we (the WebKitGTK upstream) haven't decided yet whether it will be in the next 2.20.x release. Some of us think it is worth a try, so I am in the middle of backporting the patch (and a few other patches needed by it) on top of 2.20.1, to include them in Buildroot, which is a good way of trying getting to try them on top of 2.20.1 and decide if they will go in a release as well. If you think it would be better to first disable JIT for 32-bit ARM and MIPS when doing the webkitgtk package version bump, and later on re-enabling it with the backpoted changes, please let me know. > > Sorry if I wasn't being very clear here O:-) > > > > If you think enabling the package for AArch64 deserves its own separate > > commit, let me know and I'll split it back. FWIW, I've been trying AArch64 > > builds for the RaspberryPi 3, and they work fine; we also use them at work > > for a couple of clients as well without issues. > > I think it's OK to enable on AArch64 at the same time as you do the > bump. It could be done in separate commits (each change stands on its > own), but I'm fine with those changes being in the same commit. > > Best regards, Most likely I will keep it along with the version bump, then. Thanks for all the feedback! -- Adrián 🎩
Hello, On Tue, 10 Apr 2018 20:18:30 +0100, Adrian Perez de Castro wrote: > > OK, thanks for the clarification. > > Also, I have found out from one of my test builds that in both 2.20.x releases > we have an ugly regression of the JSC JIT support on 32-bit MIPS and ARM when > using the softfp ABI. > > A patch for this is at https://bugs.webkit.org/show_bug.cgi?id=183786 but we > (the WebKitGTK upstream) haven't decided yet whether it will be in the next > 2.20.x release. Some of us think it is worth a try, so I am in the middle of > backporting the patch (and a few other patches needed by it) on top of 2.20.1, > to include them in Buildroot, which is a good way of trying getting to try > them on top of 2.20.1 and decide if they will go in a release as well. > > If you think it would be better to first disable JIT for 32-bit ARM and MIPS > when doing the webkitgtk package version bump, and later on re-enabling it > with the backpoted changes, please let me know. Since I don't know what is the extent of the breakage, and I have a limited understanding of WebkitGtk anyway, I'll let you decide what is best. By "softfp ABI", are you talking soft-float (i.e the hardware has no FPU at all) or about -mfloat-abi=softfp on ARM, which means that the FPU may be used, but that integer registers are used to pass floating point arguments ? In either case, it would mean that ARM EABIhf is not broken, and therefore JIT should be not be disabled for all ARM configurations, but only the ones using softfp. Best regards, Thomas
Hello, On Tue, 10 Apr 2018 23:22:40 +0200, Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > On Tue, 10 Apr 2018 20:18:30 +0100, Adrian Perez de Castro wrote: > > > > OK, thanks for the clarification. > > > > Also, I have found out from one of my test builds that in both 2.20.x releases > > we have an ugly regression of the JSC JIT support on 32-bit MIPS and ARM when > > using the softfp ABI. > > > > A patch for this is at https://bugs.webkit.org/show_bug.cgi?id=183786 but we > > (the WebKitGTK upstream) haven't decided yet whether it will be in the next > > 2.20.x release. Some of us think it is worth a try, so I am in the middle of > > backporting the patch (and a few other patches needed by it) on top of 2.20.1, > > to include them in Buildroot, which is a good way of trying getting to try > > them on top of 2.20.1 and decide if they will go in a release as well. > > > > If you think it would be better to first disable JIT for 32-bit ARM and MIPS > > when doing the webkitgtk package version bump, and later on re-enabling it > > with the backpoted changes, please let me know. > > Since I don't know what is the extent of the breakage, and I have a > limited understanding of WebkitGtk anyway, I'll let you decide what is > best. On certain situations, the code generated by the JavaScriptCore JIT compiler may contain calls to C++ functions which are part of the runtime. The JIT compiler has to generate those calls using the same ABI conventions as the C/C++ compiler. Any code generating calls into the runtime will either crash or behave in unpredictable ways. The patch is non-trivial, but fortunately it's just a backport of an existing fix so there are decent chances that I may have it ready and tested by next week. > By "softfp ABI", are you talking soft-float (i.e the hardware has no > FPU at all) or about -mfloat-abi=softfp on ARM, which means that the > FPU may be used, but that integer registers are used to pass floating > point arguments ? I mean using “-mfloat-abi=softfp” (FPU may be used, integer regs used). <trivia> The JavaScriptCore JIT does not run on soft-float hardware at all, and can emit NEON code, so the build recipe includes a check on BR2_ARM_CPU_HAS_NEON. There is a mode that does not use NEON, but instead it won't use Thumb2 either, just traditional ARM 32-bit instructions... but the traditional-ARM configuration has been bitrotting for quite some time. Anyway, nowadays most ARM processors with enough power to run WebKit decently all have NEON support, so probably not a big deal. </trivia> > In either case, it would mean that ARM EABIhf is not broken, and > therefore JIT should be not be disabled for all ARM configurations, but > only the ones using softfp. That is correct: ARM EABIhf (including AArch64) works well. If I don't manage to have the patch working on top of 2.20.1 in the next few days I will submit patch set to update to 2.20.1 with JIT support disabled for ARM when using softfp. As for MIPS, currently the webkitgtk recipe never enables the JIT support. With patch backported it will be possible to also enable it for 32-bit MIPS (and possibly for 64-bit as well, but I don't dare to do that because I lack hardware to run tests.) Cheers, -- Adrián 🎩
diff --git a/package/webkitgtk/0002-WPE-GTK-Build-failure-when-ENABLE_VIDEO-ENABLE_WEB_A.patch b/package/webkitgtk/0002-WPE-GTK-Build-failure-when-ENABLE_VIDEO-ENABLE_WEB_A.patch new file mode 100644 index 0000000000..2c69fa89ae --- /dev/null +++ b/package/webkitgtk/0002-WPE-GTK-Build-failure-when-ENABLE_VIDEO-ENABLE_WEB_A.patch @@ -0,0 +1,32 @@ +From ecfba9336863c2dad22c9ad1dfd1f6ea1801ccdb Mon Sep 17 00:00:00 2001 +From: "aperez@igalia.com" + <aperez@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc> +Date: Thu, 22 Mar 2018 13:48:34 +0000 +Subject: [PATCH xserver 2/2] [WPE][GTK] Build failure when ENABLE_VIDEO, + ENABLE_WEB_AUDIO and ENABLE_XSLT are disabled + https://bugs.webkit.org/show_bug.cgi?id=183896 + +Reviewed by Yusuke Suzuki. + +No new tests needed. + +* bindings/js/JSWebAnimationCustom.cpp: Add missing #include of Document.h + +Signed-off-by: Adrian Perez de Castro <aperez@igalia.com> +Upstream-Status: Committed + +diff --git a/Source/WebCore/bindings/js/JSWebAnimationCustom.cpp b/Source/WebCore/bindings/js/JSWebAnimationCustom.cpp +index 8b56f86c151..b8716be0f1c 100644 +--- a/Source/WebCore/bindings/js/JSWebAnimationCustom.cpp ++++ b/Source/WebCore/bindings/js/JSWebAnimationCustom.cpp +@@ -26,6 +26,7 @@ + #include "config.h" + #include "JSWebAnimation.h" + ++#include "Document.h" + #include "JSAnimationEffect.h" + #include "JSAnimationTimeline.h" + #include "JSDOMConstructor.h" +-- +2.16.2 + diff --git a/package/webkitgtk/Config.in b/package/webkitgtk/Config.in index 31794ed2a4..99755d8d69 100644 --- a/package/webkitgtk/Config.in +++ b/package/webkitgtk/Config.in @@ -2,6 +2,7 @@ config BR2_PACKAGE_WEBKITGTK_ARCH_SUPPORTS bool # ARM needs BLX, so v5t+, BE completely untested so disabled default y if BR2_arm && !BR2_ARM_CPU_ARMV4 + default y if BR2_aarch64 default y if BR2_i386 || BR2_x86_64 # Disabled on MIPS big endian due to sigbus default y if BR2_mipsel || BR2_mips64el @@ -41,6 +42,8 @@ config BR2_PACKAGE_WEBKITGTK select BR2_PACKAGE_LIBXSLT select BR2_PACKAGE_SQLITE select BR2_PACKAGE_WEBP + select BR2_PACKAGE_WEBP_DEMUX + select BR2_PACKAGE_WOFF2 select BR2_PACKAGE_XLIB_LIBXCOMPOSITE if BR2_PACKAGE_LIBGTK3_X11 select BR2_PACKAGE_XLIB_LIBXDAMAGE if BR2_PACKAGE_LIBGTK3_X11 select BR2_PACKAGE_XLIB_LIBXRENDER if BR2_PACKAGE_LIBGTK3_X11 diff --git a/package/webkitgtk/webkitgtk.hash b/package/webkitgtk/webkitgtk.hash index dd2b28d372..28468a5297 100644 --- a/package/webkitgtk/webkitgtk.hash +++ b/package/webkitgtk/webkitgtk.hash @@ -1,7 +1,7 @@ -# From https://webkitgtk.org/releases/webkitgtk-2.18.6.tar.xz.sums -md5 c1a548595135ee75ad3bf2e18ac83112 webkitgtk-2.18.6.tar.xz -sha1 fb0daa85142cfe8822de518dfaa7bd5c3cdd6c23 webkitgtk-2.18.6.tar.xz -sha256 93912cc2f40f12e452be1ca4babdbdaac0ec4f828d441257a6b06c2963bbac3c webkitgtk-2.18.6.tar.xz +# From https://webkitgtk.org/releases/webkitgtk-2.20.0.tar.xz.sums +md5 51cc47345c3ec53802718cd6fc9b746f webkitgtk-2.20.0.tar.xz +sha1 f1e021318f06085fff5455667459bf26716d610f webkitgtk-2.20.0.tar.xz +sha256 57f640f720bd9a8a7207f3321cf803a15c2f207b4e7b75ff1be17bc1eeb00a3c webkitgtk-2.20.20.tar.xz # Hashes for license files: sha256 0b5d3a7cc325942567373b0ecd757d07c132e0ebd7c97bfc63f7e1a76094edb4 Source/WebCore/LICENSE-APPLE diff --git a/package/webkitgtk/webkitgtk.mk b/package/webkitgtk/webkitgtk.mk index eb39664b1e..e27f970104 100644 --- a/package/webkitgtk/webkitgtk.mk +++ b/package/webkitgtk/webkitgtk.mk @@ -4,7 +4,7 @@ # ################################################################################ -WEBKITGTK_VERSION = 2.18.6 +WEBKITGTK_VERSION = 2.20.0 WEBKITGTK_SITE = http://www.webkitgtk.org/releases WEBKITGTK_SOURCE = webkitgtk-$(WEBKITGTK_VERSION).tar.xz WEBKITGTK_INSTALL_STAGING = YES @@ -14,7 +14,7 @@ WEBKITGTK_LICENSE_FILES = \ Source/WebCore/LICENSE-LGPL-2.1 WEBKITGTK_DEPENDENCIES = host-ruby host-flex host-bison host-gperf \ enchant harfbuzz icu jpeg libgcrypt libgtk3 libsecret libsoup \ - libtasn1 libxml2 libxslt sqlite webp + libtasn1 libxml2 libxslt sqlite webp woff2 WEBKITGTK_CONF_OPTS = \ -DENABLE_API_TESTS=OFF \ -DENABLE_GEOLOCATION=OFF \
Release notes: https://webkitgtk.org/2018/03/12/webkitgtk2.20.0-released.html No corresponding WebKit Security Advisory (WSA) has been published for now. This also adds a dependency on the woff2 package. In previous versions WebKitGTK+ bundled a known-to-work snapshot of its Git repository due to upstream not providing releases. The situation has changed and releases are now available, and the bundled copy of woff2 has been removed. Last but not least, this patch selects BR2_PACKAGE_WEBP_DEMUX, which is now required. Signed-off-by: Adrian Perez de Castro <aperez@igalia.com> --- Changes v1 -> v2: - Marked as supported on AArch64, which is officially supported upstream. - Add upstream patch for fixing build failure with multimedia/XSLT support disabled. --- ...ld-failure-when-ENABLE_VIDEO-ENABLE_WEB_A.patch | 32 ++++++++++++++++++++++ package/webkitgtk/Config.in | 3 ++ package/webkitgtk/webkitgtk.hash | 8 +++--- package/webkitgtk/webkitgtk.mk | 4 +-- 4 files changed, 41 insertions(+), 6 deletions(-) create mode 100644 package/webkitgtk/0002-WPE-GTK-Build-failure-when-ENABLE_VIDEO-ENABLE_WEB_A.patch