Re: [webkit-dev] Deployment of new EWS Non-Unified builder

[Emilio Cobos Álvarez via webkit-dev]

If it's useful as a data point, in Gecko we used not to care about 
non-unified builds. This worked kind of ok, because the file 
arrangements were mostly deterministic by directory. However, folks 
running with weird build configurations always ended up hitting these 
issues (and they might not know that they're not their fault).


Furthermore, various IDEs and tooling rely on dependencies being right / 
files building in non-unified mode to provide useful diagnostics while 
editing the files (e.g, we have a code review bot that runs clang-tidy 
on the files you touched, and if the dependencies are not right it just 
fails).


To that end, we have a bot that builds a "hybrid" build configuration, 
and runs on CI. It's "hybrid" in the sense there's a per-directory opt 
out; there are some directories that still haven't been fixed to build 
without unified builds, but the goal is that eventually everything 
should build in non-unified builds.


See [1] for the details. In my experience, the occasional fix-up for 
non-unified builds is better / simpler than having to untangle the mess 
after the fact (see [2] for a recent example).


Cheers,

 -- Emilio

[1]: 
https://firefox-source-docs.mozilla.org/build/buildsystem/unified-builds.html#building-outside-of-the-unified-environment

[2]: https://bugzilla.mozilla.org/show_bug.cgi?id=1772513

On 5/21/22 06:17, dpino via webkit-dev wrote:

Hi,

Last year we started a thread to discuss the possibility of deploying a
new EWS bot that builds WebKit with Non-Unified sources [1]. This thread
explains the technical reasons why a non-unified build bot is desirable.
If you're aware of the problems introduced by unified sources
compilation, skip the next paragraph.

Unified sources compilation consists of merging several source files
together into one big file to improve building times. Usually the same
sources files are grouped together, but compilation flags or downstream
changes can create different source file groups. When that happens, you
may hit a build error. Since unified sources group files together,
missing headers in one file can be satisfied by another file in the same
group. What's happening in WebKit development currently is that source
code that breaks non-unified compilation frequently lands without even
being noticed. The breaks are usually related to missing headers.
Embedders that need to support WebKit builds with different flags or
maintain downstream changes are more likely to hit these compilation
errors.

Last year's attempt to deploy an EWS Non-Unified builder ended up in a
deployment of a post-commit bot plus two workers running in the UAT. It
was actually hard to get the Non-Unified builder working [2], something
that we achieved at the beginning of this year (kudos to Adrián and
Lauro). Since then we have been maintaining the post-commit bot very
closely. There are periods of time where the bot has remained green for
a long period of time.

But lately maintaining the bot green has become harder and harder,
specially due to the big refactorizations that have happened in WebKit
source code lately. The bot very rarely stays green longer than 2 or 3
days without close maintenance. We believe that we all should share the
responsibility of keeping the non-unified build working, and therefore
we want to proceed with the deployment of a EWS Non-Unified builder.

Initially, we'll provide two workers on a modern host with 8 cores
assigned each. We have found this setup faster for most patches than
many of the existing EWS queues, as it only runs a build in non-unified
mode, without test or upload steps. If this were not fast enough, we
would consider increasing the number of workers in the queue. We set up
a page[3] with a rough comparison to the regular (unified) bot and the
build times are pretty similar.

Diego

[1]
https://www.mail-archive.com/webkit-dev@lists.webkit.org/msg30077.html
[2] https://bugs.webkit.org/show_bug.cgi?id=226088
[3] https://people.igalia.com/lmoura/webkit-bots-dashboard/unified.html
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Deployment of new EWS Non-Unified builder

[Darin Adler via webkit-dev]

> On Jun 6, 2022, at 1:09 PM, Olmstead, Don  wrote:
> 
> If it isn’t it should be considering how many times I’ve had a cq- come from 
> an AppleWin build that is in no way affected by my patch.

Yes, we have a problem with that AppleWin bot, and it’s one that bothers me 
quite a bit, but I don’t want to design our policy around that problem.

— Darin
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Deployment of new EWS Non-Unified builder

[Olmstead, Don via webkit-dev]

If it isn’t it should be considering how many times I’ve had a cq- come from an 
AppleWin build that is in no way affected by my patch.

From: Geoffrey Garen via webkit-dev 
Sent: Monday, June 6, 2022 10:40 AM
To: Mark Lam 
Cc: Darin Adler ; webkit-dev@lists.webkit.org
Subject: Re: [webkit-dev] Deployment of new EWS Non-Unified builder

As such, I also think that the non-unified EWS being green should not be a 
blocker to landing a patch. But I think having it there for information will 
help the situation. At minimum, even if every engineer simply ignores the 
non-unified EWS, it also makes it easier for someone trying to fix a trim 
missing include build issue to scan through PRs to look for this EWS failure in 
order to narrow down on which patches (and therefore possible includes) to 
focus on.

Is this the proposal on the table — to have an EWS bot, but also not block 
patches on it?

That’s surprising to me, and not how EWS bots usually work. If we just want an 
optional record of where a particular build configuration started failing, 
Isn’t that just… a not-EWS bot?

Thanks,
Geoff

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Proposal: Mandatory Commit and Merge Queue

[Geoffrey Garen via webkit-dev]

Thanks for gathering this data!

>> What are some notable cases of recent regressions that have landed because 
>> of non-use of commit queue and caused serious problems?
> 
> Some recent examples of regressions that would have been prevented by 
> mandatory commit/merge-queue as proposed:
> 
> https://commits.webkit.org/250940@main 
>  (PR did use Merge-Queue, but we’re 
> missing the feature that would have caught the problem)

What feature specifically?

> https://commits.webkit.org/250343@main 
>  (Commit did use Commit-Queue, but 
> skipped layout tests because of a previous iteration passed EWS)

Are you also proposing that merge-queue should re-run layout tests 
unconditionally? Do we know how long that takes on average? Do we have the 
hardware to support that?

> https://commits.webkit.org/250791@main 
>  (Should have failed EWS, but flakey 
> tests had different names, cross referencing trunk results would have made 
> that obvious)

How would merge-queue cross reference with trunk results or otherwise resolve 
this confusion?

> https://commits.webkit.org/248624@main 
>  (Landed manually, broke the build)

Seems like this would have been avoided by Merge-Queue alone. That’s nice.

For the other three cases, whether the proposal as-is would have resolved them 
or not seems muddy. Might need revision.

Thanks,
Geoff

>> Do you have any data on how frequent such regressions are, compared to the 
>> base rate of regressions that have landed despite use of commit queue?
> 
> Commit and Merge queue have basically prevented us from breaking the Mac 
> build, the only example of a broken Mac build I can find come from by-passing 
> Commit and Merge Queue. Layout test breakages are quite a bit more difficult 
> to reason about because breakages tend to not just be platform specific, but 
> configuration specific as well.
> 
>> 
>> Do you have any data on how frequently regressions are resolved by patches 
>> that land outside commit queue?
> 
> In the last week, we had 200 commits. 50 of those were made through the 
> Unsafe-Merge-Queue. Break down is here:
> 
> 15 were feature work that should have gone through Merge Queue
> 12 were test gardening
> 9 were build or test fixes
> 3 were 3rd party imports
> 3 were reverts
> 3 were tooling changes
> 2 were buildbot changes
> 2 were contributors.json changes
> 1 was a documentation change
> 
> My proposal would have sent the feature work, gardening work, imports, 
> tooling change and documentation change to Merge-Queue rather than 
> Unsafe-Merge-Queue. The build and test fixes would have needed modification 
> to their commit messages, but were the kind of changes I would want landed 
> via Unsafe-Merge-Queue.
> 
>> 
>>> and reduce demands of post-commit test gardening.
>> 
>> Is this goal distinct from preventing regressions from landing? If so, how?
> 
> I suppose I should have said:
> "The goal is to increase the stability of the build, speed up EWS and reduce 
> demands of post-commit test gardening by preventing regressions from 
> landing”, since build instability, slow EWS and post-commit test gardening 
> are all consequences of regressions landing.
> 
>> 
 What problem are you trying to solve, and with what level of urgency?
>>> 
>>> Urgency is not high. I started this with the expectation it would be a 
>>> somewhat long and contentious discussion. The motivating change is that the 
>>> GitHub transition makes this proposal possible, from a technical 
>>> perspective, in a way it is not while the project is still backed by 
>>> Subversion.
>> 
>> I don’t understand the premise here. There are lots of ways to enforce 
>> commit policy on a Subversion repository.
> 
> Kind of. The problem here is that we want to provide enough escape hatches so 
> that any committer can quickly repair a broken build, so we have to check not 
> just the committer, but the commit itself. This is more analogous to ensuring 
> that commit has “Reviewed by” in it’s commit message (something that 
> Subversion does not enforce in our repository, despite it being policy) than 
> it is to ensuring that the committer has certain privileges. We’ve always 
> implemented these kind of checks in buildbot, not the Subversion server, and 
> it’s much easier to implement complicated logic that takes into consideration 
> the content of the commit in buildbot than it is Subversion hooks.
> 
>> On the meta level, while we are still dealing with serious regressions in 
>> our workflow caused by git and GitHub, I recommend that we do not push 
>> forward with more unrelated sweeping changes to the project and its 
>> workflow. Just like in software development, where we need to fix 
>> regressions before we can move forward with major feature work, so too in 
>> tooling we need to do the same. Otherwise we just pile chaos 

Re: [webkit-dev] Proposal: Mandatory Commit and Merge Queue

[Jonathan Bedard via webkit-dev]

> On Jun 3, 2022, at 7:59 AM, Geoffrey Garen  wrote:
> 
>>> What is the goal of this proposal?
>> 
>> The goal is to increase the stability of the build
> 
> Is this goal distinct from preventing regressions from landing? If so, how?
> 
>> , speed up EWS by preventing regressions from landing
> 
> What are some notable cases of recent regressions that have landed because of 
> non-use of commit queue and caused serious problems?

Some recent examples of regressions that would have been prevented by mandatory 
commit/merge-queue as proposed:

https://commits.webkit.org/250940@main  
(PR did use Merge-Queue, but we’re missing the feature that would have caught 
the problem)
https://commits.webkit.org/250343@main  
(Commit did use Commit-Queue, but skipped layout tests because of a previous 
iteration passed EWS)
https://commits.webkit.org/250791@main  
(Should have failed EWS, but flakey tests had different names, cross 
referencing trunk results would have made that obvious)
https://commits.webkit.org/248624@main  
(Landed manually, broke the build)

> 
> Do you have any data on how frequent such regressions are, compared to the 
> base rate of regressions that have landed despite use of commit queue?

Commit and Merge queue have basically prevented us from breaking the Mac build, 
the only example of a broken Mac build I can find come from by-passing Commit 
and Merge Queue. Layout test breakages are quite a bit more difficult to reason 
about because breakages tend to not just be platform specific, but 
configuration specific as well.

> 
> Do you have any data on how frequently regressions are resolved by patches 
> that land outside commit queue?

In the last week, we had 200 commits. 50 of those were made through the 
Unsafe-Merge-Queue. Break down is here:

15 were feature work that should have gone through Merge Queue
12 were test gardening
9 were build or test fixes
3 were 3rd party imports
3 were reverts
3 were tooling changes
2 were buildbot changes
2 were contributors.json changes
1 was a documentation change

My proposal would have sent the feature work, gardening work, imports, tooling 
change and documentation change to Merge-Queue rather than Unsafe-Merge-Queue. 
The build and test fixes would have needed modification to their commit 
messages, but were the kind of changes I would want landed via 
Unsafe-Merge-Queue.

> 
>> and reduce demands of post-commit test gardening.
> 
> Is this goal distinct from preventing regressions from landing? If so, how?

I suppose I should have said:
"The goal is to increase the stability of the build, speed up EWS and reduce 
demands of post-commit test gardening by preventing regressions from landing”, 
since build instability, slow EWS and post-commit test gardening are all 
consequences of regressions landing.

> 
>>> What problem are you trying to solve, and with what level of urgency?
>> 
>> Urgency is not high. I started this with the expectation it would be a 
>> somewhat long and contentious discussion. The motivating change is that the 
>> GitHub transition makes this proposal possible, from a technical 
>> perspective, in a way it is not while the project is still backed by 
>> Subversion.
> 
> I don’t understand the premise here. There are lots of ways to enforce commit 
> policy on a Subversion repository.

Kind of. The problem here is that we want to provide enough escape hatches so 
that any committer can quickly repair a broken build, so we have to check not 
just the committer, but the commit itself. This is more analogous to ensuring 
that commit has “Reviewed by” in it’s commit message (something that Subversion 
does not enforce in our repository, despite it being policy) than it is to 
ensuring that the committer has certain privileges. We’ve always implemented 
these kind of checks in buildbot, not the Subversion server, and it’s much 
easier to implement complicated logic that takes into consideration the content 
of the commit in buildbot than it is Subversion hooks.

> On the meta level, while we are still dealing with serious regressions in our 
> workflow caused by git and GitHub, I recommend that we do not push forward 
> with more unrelated sweeping changes to the project and its workflow. Just 
> like in software development, where we need to fix regressions before we can 
> move forward with major feature work, so too in tooling we need to do the 
> same. Otherwise we just pile chaos on top of chaos, and there is no way to 
> know if things are improving or getting worse, and no way to hold ourselves 
> accountable for improvement. 
> 
> Thanks,
> Geoff
> 
>> 
>> Jonathan
>> 
>>> 
>>> Thanks,
>>> Geoff
>>> 
>>> 
 On Jun 2, 2022, at 2:35 PM, Jonathan Bedard via webkit-dev 
 mailto:webkit-dev@lists.webkit.org>> wrote:
 
 As we move to GitHub, I would like 

Re: [webkit-dev] Deployment of new EWS Non-Unified builder

[Geoffrey Garen via webkit-dev]

> As such, I also think that the non-unified EWS being green should not be a 
> blocker to landing a patch. But I think having it there for information will 
> help the situation. At minimum, even if every engineer simply ignores the 
> non-unified EWS, it also makes it easier for someone trying to fix a trim 
> missing include build issue to scan through PRs to look for this EWS failure 
> in order to narrow down on which patches (and therefore possible includes) to 
> focus on.

Is this the proposal on the table — to have an EWS bot, but also not block 
patches on it? 

That’s surprising to me, and not how EWS bots usually work. If we just want an 
optional record of where a particular build configuration started failing, 
Isn’t that just… a not-EWS bot?

Thanks,
Geoff

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Request for position on import.meta.resolve()

[Domenic Denicola via webkit-dev]

Hi webkit-dev,

We're proposing a small additional function to add to the HTML Standard, named 
import.meta.resolve(). See the PR [1] and a mini-explainer [2]. Gecko folks 
have reviewed the PR and contributed to the discussion, but any thoughts from 
WebKit would be very welcome.

We're hoping to ship in Chrome within the next few weeks, and I imagine web 
developers would love it if you were to also implement. We have a basic 
implementation and web platform tests you may find useful [3].

Thanks,
Domenic

[1]: https://github.com/whatwg/html/pull/5572
[2]: https://gist.github.com/domenic/f2a0a9cb62d499bcc4d12aebd1c255ab
[3]: https://chromium-review.googlesource.com/c/chromium/src/+/3456729
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Add CODEOWNERS to WebKit

[Jonathan Bedard via webkit-dev]

> On Jun 2, 2022, at 2:53 PM, Alexey Proskuryakov  wrote:
> 
> 
> It seems like we still need to host watchlist CC service though, to support 
> patch workflow - and we'll have separate lists in GitHub and in Bugzilla, 
> unless watchlists are reimplemented.

We would still need to host the current watchlist service, yes. Although we 
wouldn’t have to teach this watchlist how to interact with GitHub. It would 
also be possible (and not too hard) to teach our watchlist service to read the 
CODEOWNERS file, at least in cases where the individual being CCed wasn’t a 
group.

> 
> I believe that while there are stale watchlists entries, newer additions are 
> valid.
> 
> - Alexey
> 
>> 2 июня 2022 г., в 2:19 PM, Jonathan Bedard via webkit-dev 
>> mailto:webkit-dev@lists.webkit.org>> 
>> написал(а):
>> 
>> Porting is possible, but that would be something that would happen, at the 
>> earliest, a few weeks from now. I does seem to me that the watchlist is out 
>> of date, so a forced-cleanup doesn’t seem like the worst thing.
>> 
>> Jonathan
>> 
>>> On Jun 2, 2022, at 2:21 PM, Chris Dumez >> > wrote:
>>> 
>>> I’m kind of ambivalent/neutral about this. One question though:
>>> 
>>> When adopting CODEOWNERS, will our existing watchlists get ported, or will 
>>> each contributor have to modify CODEOWNERS themselves to match whatever was 
>>> in the watchlists for them?
>>> 
>>> Thanks,
>>> Chris.
>>> 
 On Jun 2, 2022, at 1:12 PM, Jonathan Bedard via webkit-dev 
 mailto:webkit-dev@lists.webkit.org>> wrote:
 
 Hey folks,
 
 Yusuke is interested in adding the CODEOWNERS file to the WebKit project 
 (see https://github.com/WebKit/WebKit/pull/1137 
 ). There have been some 
 objections to this, the two ones I’m aware of:
 - WebKit doesn’t assign “ownership” to pieces of code, so the name is 
 unfortunate
 - The last match “wins”, so if you subscribed to a generic directory early 
 in the file, more specific matches would override that subscription
 
 Despite these downsides, I think adding the CODEOWNERS file is good for 
 the project for a few reasons:
 - It’s a standard natively supported by GitHub, BitBucket and GitLab and 
 will be familiar to developers
 - File format is more simple than existing watchlist
 - Support for groups and individuals
 - No need for WebKit to host a service (other implementations of 
 auto-CCing would require this)
 
 I intend to work with Yusuke to land 
 https://github.com/WebKit/WebKit/pull/1137 
  and start adopting CODEOWNERS 
 on Monday absent objections that folks think overshadow the benefits of 
 the CODEOWNERS file.
 
 Jonathan
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org 
 https://lists.webkit.org/mailman/listinfo/webkit-dev 
 
>>> 
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org 
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Request for Apple Position on Shared Storage

[eric trouton via webkit-dev]

Hi Webkit-Dev Team,

We've been working on the Shared Storage API that supports a variety of use
cases that may be impacted by 3rd Party Cookie Deprecation.  The idea is to
provide a storage API (named Shared Storage) that is intended to be
unpartitioned. Origins can write to it from their own contexts on any page.
To prevent cross-site tracking of users, data in Shared Storage may only be
read in a restricted environment that has carefully constructed output
gates. Over time, we hope to design and add additional gates.

We would like to hear what you think about it.  Chrome is implementing
(available in Chrome Canary on M104) but open to evolving the API over time
and are appreciative of your feedback.


explainer: 
https://github.com/pythagoraskitty/shared-storage/

chromestatus: 
https://chromestatus.com/feature/6256348582903808

spec: TBD

Thank you,

Eric
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev