Haiku WebKit port patchset

Where Haiku specific WebKit (legacy and new) patches are available? How to apply it on upstream WebKit?

1 Like

Its history (default branch) is mixed with Haiku and non/Haiku commits and have strange big commits like Merge commit 'bd66a76939e3d63c8f471cd02ff2fe39bca01eca' into HEAD. Why Haiku commits are not on top of upstream ones? How to rebase it on top of WebKit upstream? Should I use splashed branch?

Rebasing is not a maintainable strategy for a codebase this big and active. My experiments with QtWebEngine (and therefore Chromium) showed that pretty clearly - I spent almost as much time porting the patches to newer releases as initially writing them.

Commits are mixed because that’s how GitHub displays them - lack of visual indication of where commits come from is not great. That’s on GitHub though. BitBucket has that feature and most if not all desktop tools do too.

How to rebase them? Your best bet is to checkout both haiku repo and WebKit upstream. Find latest common ancestor for both of them, checkout WebKit to that, and then use regular diff to “distill” Haiku changes to a single patch.

2 Likes

You can make a diff between the “rebased” branch and the matching webkit branch (the latest one merged in rebased).

It’s not maintained as patches because… well, because git is a convenient way to manage things and it’s a lot easier for me to manage merging the changes with webkit upstream this way. Git manages the merge conflicts for the most part and lets me fix the remaining ones. The webkit codebase changes often enough that patches would rarely rebase cleanly from one version to another (and the changes are often nontrivial). So a patchset would not bring much, we could still only apply it to a single fixed revision of webkit and need major rework evrytime we rebase it.

Note that current webkit upstream also requires a newer version of ICU than what we currently ship, which is why the repo is stopped at that particular commit and I did not merge later ones. I plan to get back to this after beta2 is released and I can work on updating our version of ICU.

The “splashed” branch was an attempt of mine at cleaning up the patch history and seeing if there were any “small” changes we could upstream. It does not build and I never got around to cleaning it up.

In general, yes, repositories as large as WebKit can only be maintained through merge commits from upstream, not rebasing. There are over a thousand Haiku-specific commits in that repository, rebasing them every few months would be insanity.

I reviewed against upstream showing around 2200 new commits as of today.The last Haiku Webkit built ok for me. Will review build with recent merges…

Yes, however it may not make sense to look at it in terms of commits, because a lot of these are just merging our branch with upstream, and the rest is a lot of changing the same code over and over again to adjust to the changes in webkit. As a patchset it would indeed be much smaller, but it would need constant care to keep it up to date against upstream.

Ideally we would have 2-3 people giving some attention to webkit, and then it would be reasonable to start upstreaming our changes again. But our past experience shows that this is not possible when there is a single developer who is also quite busy with other parts of the OS.

2 Likes

I created a visual list for quick review against upstream here: Haiku webkit upstream compare

Bugfix-wise… I’ll await your testsuite review.

@X512 - I merged my haiku-webkit rebase branch into latest Webkit as of 2020-06-02 (today).

Patchset seems possible to upstream…

4 Likes

The hard part of upstreaming our work to webkit is not updating to current version or generating a patch.
The requirements are:

  • Proper copyright assignment in all the code we changed,
  • Each commit must be submitted separately, with a clearly limited functional change, and new unit tests or justification of why unit tests are not needed,
  • If adding a new platform, we must supply buildbots for it so that other developers can avoid breaking our build,
  • We must have a team large enough to ensure continuous maintenance of the port,
  • They will probably not want to merge our work in WebKitLegacy but only in WebKit2.

In the past we had upstreamed our work in webkit (around 2010 or so) and it was removed after being unmaintained for 2 years.

1 Like

We could actually get Haiku supported as a buildbot with either fosshost or sr.ht which offloads the creation of buildbots to them instead of us doing it. If Plan 9 is able to be ‘supported’ for builds, surely Haiku support as a buildbot should be possible.

As for building, we really should be looking at cross-compiling WebKit or other ports to Haiku in order to speed up the compilation process.

Review on 2020-08-12:

  1. Forked haiku/webkit
  2. Updated rebased - cleanly merged with webkit-master (upstream, 2020-08-12).
  3. Update to webkit2 - had 19 file conflicts from my updated rebased.
  4. splashed directory - remove ?

Plan:

  1. Review the rebased build.
  2. Review webkit2 file conflicts
6 Likes

The “webkit2” branch does not result in a working browser. You can merge it if you want but it will still result in a non-working browser.

I think it’s better to look at what the commits there did, and cherry-pick them or rewrite them with the current state of WebKit. Preferably starting from the gsoc2019 tag, which at least managed to start a browser. The webkit2 branch was an attempt to rebase the gsoc2019 work to a newer version of webkit, but the rebasing approach failed.

webpositive-610.2.2

Reviewed on 2020-09-11:

  1. Forked haiku/webkit
  2. Built and tested x86_64 haikuwebkit package (WebKit 610.2.2). :+1:
  3. Updated forked rebased to 2020-09-11. Repatched RunLoop.h
  4. Reviewing WebCore-Fetch (memmem usage on Haiku; WebKit 611.1).
9 Likes

webpositive-webkit-611.1.1_cocobean
NOTE: WebPositive using WebKit 611.1.1.

Reviewed on 2020-09-13:
Re-forked haiku/webkit
Built and tested x86_64 haikuwebkit package (WebKit 611.1.1). :+1:
Updated forked rebased and master to latest upstream 2020-09-13.

5 Likes

Did this also include the lighttpd bump where there is an open PR for it?

It is only used when running the webkit testsuite (not part of the normal webkit build process because it takes more than 24 hours to run currently…)

Even then @PulkoMandy, it would be good to know if it would crash as the next version did for the testsuite (regarding the PR)

Yes, but I mean, no one will run into that problem by accident unless they actually try to run the tests.