Bug 177862 – Remove WOFF2 from Source/ThirdParty

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Frédéric Wang (:fredw)]

We can not do that until WOFF2 can be properly packaged, but let's try and prepare a patch for this.

[Tomas Popela]

And also we should remove brotli as it is a dependency of WOFF2

[Frédéric Wang (:fredw)]

(In reply to Tomas Popela from comment #1)
> And also we should remove brotli as it is a dependency of WOFF2

Yes, it's bug 177804 ;-)

[Tomas Popela]

Created attachment 322650 [details]
Patch

[Frédéric Wang (:fredw)]

Comment on attachment 322650 [details]
Patch

Sorry, I was not clear enough. We just want to remove the source from ThirdParty and use a jhbuild / system library instead.

[Frédéric Wang (:fredw)]

Created attachment 322653 [details]
Patch (WIP)

This is what I meant. But I just realized that I also need to make woff2_out.h public, so I'll need to update my WOFF2 branch on github.

[Frédéric Wang (:fredw)]

Created attachment 322660 [details]
Patch

[Frédéric Wang (:fredw)]

Created attachment 322661 [details]
Patch (177768+177804+177862)

[Michael Catanzaro]

(In reply to Frédéric Wang (:fredw) from comment #4)
> Comment on attachment 322650 [details]
> Patch
> 
> Sorry, I was not clear enough. We just want to remove the source from
> ThirdParty and use a jhbuild / system library instead.

I think this patch is what Tom will use in Fedora and RHEL until the license issue is resolved. This is also what we'll wind up using if we're unable to resolve the license situation with Google. Hopefully it won't come to that.

[Frédéric Wang (:fredw)]

Created attachment 322811 [details]
Patch

[Michael Catanzaro]

Comment on attachment 322811 [details]
Patch

Thanks for working on this!

I think this one is not ready for review yet. We'll need to decide whether to tell Linux distros to package your fork, or whether we can convince Google to release a usable library so we don't need your fork, or whether it's best to just keep it under ThirdParty (and update it very regularly!) if we fail at convincing upstream to put this under a reasonable license. Let's wait a few days and see if upstream takes some action on this.

[Build Bot]

Comment on attachment 322811 [details]
Patch

Attachment 322811 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4765928

New failing tests:
workers/wasm-hashset.html

[Build Bot]

Created attachment 322821 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

[Frédéric Wang (:fredw)]

Created attachment 322994 [details]
Patch

[Frédéric Wang (:fredw)]

Created attachment 323017 [details]
Patch

Now there is a release we can use as a reference: https://github.com/google/woff2/releases/tag/v1.0.1

[Frédéric Wang (:fredw)]

Created attachment 323019 [details]
Patch

[Michael Catanzaro]

Comment on attachment 323019 [details]
Patch

r=me, but we need to convince Carlos Garcia and Carlos Lopez before landing, since it was just released today. I think it makes sense to match what we are doing for libbrotli, though. And we are very early in the release cycle, so this is the right time to do it.

[Carlos Garcia Campos]

I'm ok

[Tomas Popela]

I tried this patch with freshly packaged woff2 for Fedora and it works (after applying one small, but fundamental patch). Thank you Fred!

--- a/Source/cmake/OptionsGTK.cmake
+++ b/Source/cmake/OptionsGTK.cmake
@@ -387,7 +387,7 @@ if (USE_WOFF2)
     if (NOT BROTLIDEC_FOUND)
        message(FATAL_ERROR "librotlidec is needed for USE_WOFF2.")
     endif ()
-    find_package(WOFF2Dec, 1.0.1)
+    find_package(WOFF2Dec 1.0.1)
     if (NOT WOFF2DEC_FOUND)
        message(FATAL_ERROR "liwoff2dec is needed for USE_WOFF2.")
     endif ()

[Frédéric Wang (:fredw)]

(In reply to Tomas Popela from comment #18)
> I tried this patch with freshly packaged woff2 for Fedora and it works
> (after applying one small, but fundamental patch). Thank you Fred! 

Great thanks! Indeed I quickly edited the patch by hand before going to week-end and made that mistake!

(In reply to Michael Catanzaro from comment #16)
> Comment on attachment 323019 [details]
> Patch
> 
> r=me, but we need to convince Carlos Garcia and Carlos Lopez before landing,
> since it was just released today. I think it makes sense to match what we
> are doing for libbrotli, though. And we are very early in the release cycle,
> so this is the right time to do it.

It seems that the two Carlo's are fine with that change, so I'll land it.

[Frédéric Wang (:fredw)]

Committed r223042: <http://trac.webkit.org/changeset/223042>