Bug 248185 – libavif and dav1d should live under ThirdParty/

• Component: Platform
• Status: RESOLVED FIXED

[Michael Catanzaro]

Since 255797@main libavif is now bundled to provide AVIF decoding. Unfortunately, it was imported under Source/WebCore/PAL/libavif rather than Source/ThirdParty. It's important to keep all bundled code under Source/ThirdParty so that we can easily keep track of it, see when the code has been last updated, decide whether to include or exclude it from release tarballs, and ensure it's properly tracked in downstream metadata. Having bundled sources in multiple locations is going to make this very difficult, especially for people unfamiliar with WebKit. Example problem that has already occurred: it's accidentally included in the WebKitGTK 2.39.1 release tarballs. Had I not noticed it due to luck, it would have resulted in violation of security policy for downstream distributions that requires metadata for tracking bundled libraries. There's no way that product security organizations can properly react to a libavif vulnerability without metadata to know that it's there, but we didn't announce that we put it there, because how to know about it when it's in a random place? Whereas if it were under Source/ThirdParty, then it would have been automatically excluded from releases unless a human decided to allowlist it. Also, we can periodically check to see that everything there is kept is reasonably up to date.

So we should move libavif to Source/ThirdParty. Unfortunately, there's not much I can do to help with fixing this. Since this requires editing XCode build files, this move really has to be handled by Apple developers or someone familiar with XCode development.

[Michael Catanzaro]

Um, same problem with dav1d, except that one is even tricker since it's layered underneath libavif with no indication that it's yet another upstream source. I would have missed that entirely if not for the title of bug #246598.

[Michael Catanzaro]

A ideal layout for this would be: Source/ThirdParty/libavif, Source/ThirdParty/dav1d.

Alternatively, if keeping dav1d underneath the libavif directory is really desired, then it should be named differently to make clear that it is another third-party source, e.g. Source/ThirdParty/libavif/ThirdParty/dav1d.

[Radar WebKit Bug Importer]

<rdar://problem/102727980>

[Myles C. Maxfield]

We intentionally chose to put it inside WebCore because putting it outside WebCore would make it very difficult to integrate with Apple's build system.

The idea is that we would stop bundling it (and delete the checked-in copy!) in the future when all of Apple's OSes that ToT WebKit runs on would support avif natively. So we thought that all the pain of integrating with Apple's build system wouldn't be worth it for just a few years.

How do you feel about putting a few years' deadline on the amount of time that third party sources are in multiple directories to keep track of?

[Michael Catanzaro]

(In reply to Myles C. Maxfield from comment #4)
> We intentionally chose to put it inside WebCore because putting it outside
> WebCore would make it very difficult to integrate with Apple's build system.

This is strange because WebCore already has an unavoidable hard dependency on Source/ThirdParty due to both ANGLE and libwebrtc. But anyway, if it really needs to be under WebCore, how about Source/WebCore/ThirdParty?

Either:

 Source/WebCore/ThirdParty/libavif
 Source/WebCore/ThirdParty/dav1d

Or:

 Source/WebCore/ThirdParty/libavif
 Source/WebCore/ThirdParty/libavif/ThirdParty/dav1d

(Notably, I suggest avoiding Source/WebCore/ThirdParty/libavif/dav1d, to make it clear that's a second third-party source.)

If one of those suggestions works for you, then we can dodge the problem and not bother with discussing how long it's OK for things to be messy. :)

[Myles C. Maxfield]

Can we keep it in PAL?

[Tim Nguyen (:ntim)]

> We intentionally chose to put it inside WebCore because putting it outside WebCore would make it very difficult to integrate with Apple's build system.

PDF.js is prior art of integrating a ThirdParty directory into WebCore for XCode build purposes (see bug 241117)

[Michael Catanzaro]

(In reply to Myles C. Maxfield from comment #6)
> Can we keep it in PAL?

Seems like an odd place, but sure:

 Source/WebCore/PAL/ThirdParty/libavif
 Source/WebCore/PAL/ThirdParty/dav1d

Or:

 Source/WebCore/PAL/ThirdParty/libavif
 Source/WebCore/PAL/ThirdParty/libavif/ThirdParty/dav1d

Just stuffing ThirdParty into the path will help a lot.

[Myles C. Maxfield]

I'm working on this tonight.

[Myles C. Maxfield]

Okay, I have a patch that works with local builds. Let's make sure it builds with Apple's build system before uploading.

[Myles C. Maxfield]

https://github.com/WebKit/WebKit/pull/8364

[Myles C. Maxfield]

https://github.com/WebKit/WebKit/pull/8368

[EWS]

Committed 258646@main (8a6b8098b05c): <https://commits.webkit.org/258646@main>

Reviewed commits have been landed. Closing PR #8368 and removing active labels.