Page MenuHomeFreeBSD

www/webkit2-gtk3: Update to 3.20.0 (WIP)

Authored by tobik on Mar 27 2018, 6:12 PM.


Group Reviewers
rP466598: www/webkit2-gtk3: Update to 2.20.0
  • Add WAYLAND option

Initial update to 3.18.3 by Greg V

PR: 223733
Security: 1ce95bc7-3278-11e8-b527-00012e582166

Test Plan

All dependent ports except www/epiphany build fine on 11.1/amd64 with the following make.conf:


editors_emacs-devel_SET=        XWIDGETS

The build of Epiphany was trivial to fix. Patch included here.

Builds on 10.3/i386 with

editors_emacs-devel_SET=        XWIDGETS

were fine too.

Runtime appears to be ok on 11.1/amd64 and 12.0-CURRENT.

Diff Detail

rP FreeBSD ports repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Maybe update Epiphany as well? Epiphany 3.28 built without the extra patch for me with webkit2 2.19.92 (almost 2.20)

51 ↗(On Diff #40811)

Can we keep MiniBrowser enabled and installed by default? It is very useful for debugging.

52 ↗(On Diff #40811)

Since WebKit is written in C++, do you think it is better to set these macros in CXXFLAGS as well? I know CPPFLAGS is a better place for preprocessor macros, but cmake doesn't seem to support CPPFLAGS.

  • Build MiniBrowser again
  • Don't set CMAKE_BUILD_TYPE as USES=cmake does it already
82 ↗(On Diff #40842)

I am not sure why this DEBUG_*_OFF variables were added. Building WebKit with debug symbols enabled does require ar and ld supporting thin archives, so pulling binutils from ports is correct. But I don't know there are any reasons to force the use of ar and ld from base when debug symbols are disabled. Do you think we can remove these two DEBUG_*_OFF variables?

  • Use cmake:outsource
  • Add comment to explain why binutils are needed for DEBUG
82 ↗(On Diff #40842)

It was added in rP441828. I think this still applies now.

  • Fix build on 12.0-CURRENT/armv7
In file included from DerivedSources/WebCore/unified-sources/UnifiedSource345.cpp:1:
In file included from /wrkdirs/usr/ports/www/webkit2-gtk3/work/webkitgtk-2.20.0/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.cpp:28:
/wrkdirs/usr/ports/www/webkit2-gtk3/work/webkitgtk-2.20.0/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h:147:46: error: no member named 'lightVector' in 'WebCore::LightSource::PaintingData'
        floatArguments.lightX = paintingData.lightVector.x();
                                ~~~~~~~~~~~~ ^
/wrkdirs/usr/ports/www/webkit2-gtk3/work/webkitgtk-2.20.0/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h:148:46: error: no member named 'lightVector' in 'WebCore::LightSource::PaintingData'
        floatArguments.lightY = paintingData.lightVector.y();
                                ~~~~~~~~~~~~ ^
/wrkdirs/usr/ports/www/webkit2-gtk3/work/webkitgtk-2.20.0/Source/WebCore/platform/graphics/cpu/arm/filters/FELightingNEON.h:149:46: error: no member named 'lightVector' in 'WebCore::LightSource::PaintingData'
        floatArguments.lightZ = paintingData.lightVector.z();
                                ~~~~~~~~~~~~ ^
3 errors generated.
82 ↗(On Diff #40842)

Yes, you are correct. It is still needed but I forgot it. Personally I prefer binutils from ports to elftoolchain included in base because having an ar supporting thin archives greatly reduces the disk space required to build WebKit. I know it is probably not worth to add a check here, and I just mentioned it for reference. It doesn't mean that non-debug build requires programs from base. It is just a simple solution to avoid mixing tools from base and ports by (correctly) assuming things in base are always available.

74 ↗(On Diff #40857)

It is nice to see the comment here. Do you want to put this WebKit bug report link here as well?

82 ↗(On Diff #40842)

Sorry, I don't know how to mark these comments as done. Can you tell me how to do it?

  • Add WebKit bug link re DEBUG
tobik marked 5 inline comments as done.Apr 1 2018, 5:48 PM
tobik added inline comments.
82 ↗(On Diff #40842)

There should be a checkbox labeled "Done" in the top-right corner of inline comments.

AFAIK only the review author can do this.

tobik marked 2 inline comments as done.Apr 1 2018, 5:48 PM

Thanks for your work. Although I haven't tested it on my machine, these changes look good to me now.

This revision was not accepted when it landed; it landed in state Needs Review.Apr 5 2018, 8:14 PM
This revision was automatically updated to reflect the committed changes.