[WPE] Checkout a specific version of Cog instead of master #7508

mcatanzaro · 2022-12-12T22:20:26Z

`02eb578`

[WPE] Checkout a specific version of Cog instead of master
https://bugs.webkit.org/show_bug.cgi?id=249034

Reviewed by NOBODY (OOPS!).

Igalia wants to start running performance tests, but performance
test results will be inconsistent if the version of cog may be different
from one day to the next. A specific commit should be used instead.

The downside of this change is we now have to remember to update the
Cog version to use in a rather unobvious place, Tools/PlatformWPE.cmake,
which is not something that we will remember to do on a regular basis.
We need a better way to handle this, such as adding it to the Cog
release process.

* Tools/PlatformWPE.cmake:

02eb578

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 gtk-wk2
	❌ 🧪 api-ios	✅ 🧪 mac-wk1	❌ 🧪 api-gtk
	✅ 🛠 tv	❌ 🧪 mac-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch	✅ 🧪 mac-wk2-stress
	✅ 🛠 watch-sim

webkit-early-warning-system · 2022-12-12T22:22:16Z

EWS run on previous version of this PR (hash b103e74)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
⏳ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
⏳ 🧪 bindings	⏳ 🛠 ios-sim	⏳ 🛠 mac-AS-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
⏳ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🧪 api-mac	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 mac-wk1	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk2
	⏳ 🛠 tv-sim	⏳ 🧪 mac-AS-debug-wk2
	⏳ 🛠 watch	⏳ 🧪 mac-wk2-stress
	⏳ 🛠 watch-sim

webkit-early-warning-system · 2022-12-12T22:22:58Z

EWS run on previous version of this PR (hash 87d52fd)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 mac-wk1	⏳ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch	✅ 🧪 mac-wk2-stress
	✅ 🛠 watch-sim

mcatanzaro · 2022-12-15T21:26:40Z

I finally noticed that the EWS is broken:

FAILED: core/libcogcore.so.9.1.1 
cc  -o core/libcogcore.so.9.1.1 core/libcogcore.so.9.1.1.p/cog-directory-files-handler.c.o core/libcogcore.so.9.1.1.p/cog-host-routes-handler.c.o core/libcogcore.so.9.1.1.p/cog-modules.c.o core/libcogcore.so.9.1.1.p/cog-platform.c.o core/libcogcore.so.9.1.1.p/cog-prefix-routes-handler.c.o core/libcogcore.so.9.1.1.p/cog-request-handler.c.o core/libcogcore.so.9.1.1.p/cog-shell.c.o core/libcogcore.so.9.1.1.p/cog-utils.c.o core/libcogcore.so.9.1.1.p/cog-webkit-utils.c.o core/libcogcore.so.9.1.1.p/cog-gamepad-manette.c.o -Wl,--as-needed -Wl,--no-undefined -Wl,-O1 -shared -fPIC -Wl,--start-group -Wl,-soname,libcogcore.so.9 -Wl,-rpath,/usr/lib/x86_64-linux-gnu -Wl,-rpath-link,/usr/lib/x86_64-linux-gnu /usr/lib/x86_64-linux-gnu/libsoup-3.0.so -Wl,--export-dynamic /usr/lib/x86_64-linux-gnu/libgmodule-2.0.so -pthread /usr/lib/x86_64-linux-gnu/libglib-2.0.so /usr/lib/x86_64-linux-gnu/libgio-2.0.so /usr/lib/x86_64-linux-gnu/libgobject-2.0.so /usr/lib/x86_64-linux-gnu/libwpe-1.0.so -L/app/webkit/WebKitBuild/Release/lib -lWPEWebKit-1.1 /usr/lib/x86_64-linux-gnu/libmanette-0.2.so -Wl,--end-group
/usr/lib/gcc/x86_64-unknown-linux-gnu/12.1.0/../../../../x86_64-unknown-linux-gnu/bin/ld: cannot find -lWPEWebKit-1.1: No such file or directory
collect2: error: ld returned 1 exit status

So yeah, that plan is not going to work.

clopez · 2022-12-20T04:21:27Z

It seems the build failure is because you used as version 0.15.1, which is a commit previous to Igalia/cog@be29794 (API changes after #3886)

So, instead of using a tagged version I think we should use a git hash of the last version on master, and update this periodically as needed (the build breaks or someone remembers to update it).

So try using 7eae03e instead of 0.15.1

mcatanzaro · 2022-12-20T19:31:00Z

Secondary goal here is to use a version of cog that can build #5944, so using anything on the cog master branch will not be possible unless we're willing to make cog depend on WebKit pull request branches. So I'll prepare a cog side branch and switch WebKit to use that for now. Not sure how soon I'll get to this, though.

clopez · 2022-12-20T20:09:51Z

Maybe an idea is to patch the Cog Meson build system to allow manually selecting which API to use and pass the desired parameter to Cog/Meson from WebKit/CMake

mcatanzaro · 2022-12-22T19:09:11Z

Maybe an idea is to patch the Cog Meson build system to allow manually selecting which API to use and pass the desired parameter to Cog/Meson from WebKit/CMake

That won't work either. If we use the wpe-webkit-2.0 API then we have a circular dependency between cog and WebKit when making API changes. If we use the wpe-webkit-1.1 API, like I tried to do above, then the build will fail because it doesn't exist anymore in WebKit.

I see two workable options:

WebKit must depend on side branches of cog, where cog is modified to account for WebKit API changes that have not yet landed in WebKit
Simply turn off ENABLE_COG for now, and reenable right before releasing 2.39.90 when the API stabilizes

My preference is option 2 because that would be easier for WebKit development, but it risks that we forget to keep cog in good shape.

mcatanzaro · 2022-12-22T19:28:12Z

So for this pull request, let me switch to using the latest release of cog.

I'll open a separate pull request to temporarily disable cog. I think perf tests can use MiniBrowser instead, at least for the next two months, right?

https://bugs.webkit.org/show_bug.cgi?id=249034 Reviewed by NOBODY (OOPS!). Igalia wants to start running performance tests, but performance test results will be inconsistent if the version of cog may be different from one day to the next. A specific commit should be used instead. The downside of this change is we now have to remember to update the Cog version to use in a rather unobvious place, Tools/PlatformWPE.cmake, which is not something that we will remember to do on a regular basis. We need a better way to handle this, such as adding it to the Cog release process. * Tools/PlatformWPE.cmake:

webkit-early-warning-system · 2022-12-22T19:40:27Z

EWS run on current version of this PR (hash 02eb578)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 gtk-wk2
	❌ 🧪 api-ios	✅ 🧪 mac-wk1	❌ 🧪 api-gtk
	✅ 🛠 tv	❌ 🧪 mac-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch	✅ 🧪 mac-wk2-stress
	✅ 🛠 watch-sim

mcatanzaro · 2022-12-22T19:40:27Z

So for this pull request, let me switch to using the latest release of cog.

Switched to 0.16.1. Hopefully the EWS will like this.

I'll open a separate pull request to temporarily disable cog. I think perf tests can use MiniBrowser instead, at least for the next two months, right?

#8011

clopez · 2022-12-22T23:38:05Z

I'll open a separate pull request to temporarily disable cog. I think perf tests can use MiniBrowser instead, at least for the next two months, right?

Not really. Using cog is a must.
Also many developers rely on the Cog build.
I don't think we should stop building Cog with WPE. Also I'm not really understanding what is really the problem here.
Why can't you add an optional configuration option parameter inside Cog to make it use one API or the other? I'm a bit lost

mcatanzaro · 2022-12-23T01:15:05Z

Why can't you add an optional configuration option parameter inside Cog to make it use one API or the other? I'm a bit lost

See #7508 (comment)

clopez · 2022-12-26T17:56:16Z

I see.
The issue is mainly that we should be able to modify Cog to use the new modified API that we are modifying in the same WebKit patch, but we can't have this changes in Cog main branch until the new WebKit API goes into the stable releases.

I see three possible solutions to this:

Use cog side branches as you propose: first we commit the needed changes to use the new API on a branch of the repo Cog and then we implement the WebKit patch changing the API and also modifying the commit to fetch from Cog to be the one of this side branch of the repo Cog.
Modify the cmake code that checkouts the Cog repository to be able to apply a patch to it after fetching it. So we can have a patch that applies over Cog with the modified API. That way when you upload the patch for WebKit with the new changes to the API you also update this new-api-cog.patch patch file that will be applied to Cog before building it. We can add on the release notes of WPE a note saying that this patch should be synced with Cog upstream on the moment of the release.
Import the Cog source code into Source/ThirdParty to be able to modify it directly and then sync it with upstream periodically as needed (e.g.: when we do a new release of WPE).

I can see pros&cons for each solution. But given that modifying the API is not something usual maybe the less bad option for now is to use cog side branches. Perhaps we can name the branch in Cog something like: api-next.

WDYT?

mcatanzaro · 2022-12-26T19:08:06Z

I'm fine with any of the three options, but I suspect option 1 will be easiest.

Option 3 (import into Source/ThirdParty) would require resurrecting cog's CMake build system. Surely that's doable, but this is a temporary problem and it makes sense to go with the simplest solution.

mcatanzaro · 2023-01-06T22:41:36Z

Closing this. I will do this directly in #5944 instead.

mcatanzaro requested a review from JonWBedard as a code owner December 12, 2022 22:20

mcatanzaro self-assigned this Dec 12, 2022

mcatanzaro added the WPE WebKit WebKit WPE component label Dec 12, 2022

mcatanzaro requested review from clopez and a team December 12, 2022 22:20

mcatanzaro force-pushed the eng/WPE-Checkout-a-specific-version-of-Cog-instead-of-master branch from b103e74 to 87d52fd Compare December 12, 2022 22:22

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 12, 2022

mcatanzaro mentioned this pull request Dec 22, 2022

[WPE][GTK] Remove webkit_web_context_set_sandbox_enabled() from modern API #5944

Merged

mcatanzaro removed the merging-blocked Applied to prevent a change from being merged label Dec 22, 2022

mcatanzaro force-pushed the eng/WPE-Checkout-a-specific-version-of-Cog-instead-of-master branch from 87d52fd to 02eb578 Compare December 22, 2022 19:39

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 22, 2022

clopez mentioned this pull request Dec 28, 2022

[WPE] Temporarily disable ENBALE_COG by default #8011

Closed

mcatanzaro closed this Jan 6, 2023

mcatanzaro deleted the eng/WPE-Checkout-a-specific-version-of-Cog-instead-of-master branch January 6, 2023 22:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WPE] Checkout a specific version of Cog instead of master #7508

[WPE] Checkout a specific version of Cog instead of master #7508

mcatanzaro commented Dec 12, 2022 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Dec 12, 2022 •

edited

webkit-early-warning-system commented Dec 12, 2022 •

edited

mcatanzaro commented Dec 15, 2022

clopez commented Dec 20, 2022

mcatanzaro commented Dec 20, 2022

clopez commented Dec 20, 2022 •

edited

mcatanzaro commented Dec 22, 2022

mcatanzaro commented Dec 22, 2022

webkit-early-warning-system commented Dec 22, 2022 •

edited

mcatanzaro commented Dec 22, 2022

clopez commented Dec 22, 2022 •

edited

mcatanzaro commented Dec 23, 2022

clopez commented Dec 26, 2022 •

edited

mcatanzaro commented Dec 26, 2022

mcatanzaro commented Jan 6, 2023

[WPE] Checkout a specific version of Cog instead of master #7508

[WPE] Checkout a specific version of Cog instead of master #7508

Conversation

mcatanzaro commented Dec 12, 2022 • edited by webkit-early-warning-system

02eb578

webkit-early-warning-system commented Dec 12, 2022 • edited

webkit-early-warning-system commented Dec 12, 2022 • edited

mcatanzaro commented Dec 15, 2022

clopez commented Dec 20, 2022

mcatanzaro commented Dec 20, 2022

clopez commented Dec 20, 2022 • edited

mcatanzaro commented Dec 22, 2022

mcatanzaro commented Dec 22, 2022

webkit-early-warning-system commented Dec 22, 2022 • edited

mcatanzaro commented Dec 22, 2022

clopez commented Dec 22, 2022 • edited

mcatanzaro commented Dec 23, 2022

clopez commented Dec 26, 2022 • edited

mcatanzaro commented Dec 26, 2022

mcatanzaro commented Jan 6, 2023

mcatanzaro commented Dec 12, 2022 •

edited by webkit-early-warning-system

`02eb578`

webkit-early-warning-system commented Dec 12, 2022 •

edited

webkit-early-warning-system commented Dec 12, 2022 •

edited

clopez commented Dec 20, 2022 •

edited

webkit-early-warning-system commented Dec 22, 2022 •

edited

clopez commented Dec 22, 2022 •

edited

clopez commented Dec 26, 2022 •

edited