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

[Jonathan Bedard via webkit-dev]

> On Jun 2, 2022, at 4:41 PM, Geoffrey Garen  wrote:
> 
> 
>> As we move to GitHub, I would like to propose we strengthen our protections 
>> on `main` by making MergeQueue and CommitQueue mandatory. 
> 
> 
> What is the goal of this proposal?

The goal is to increase the stability of the build, speed up EWS by preventing 
regressions from landing and reduce demands of post-commit test gardening.

> 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.

Jonathan

> 
> Thanks,
> Geoff
> 
> 
>> On Jun 2, 2022, at 2:35 PM, Jonathan Bedard via webkit-dev 
>>  wrote:
>> 
>> As we move to GitHub, I would like to propose we strengthen our protections 
>> on `main` by making MergeQueue and CommitQueue mandatory. This would mean 
>> that with a few exceptions, all changes would need to be built and run 
>> layout tests before they are landed. To spell out what the exceptions I had 
>> in mind are:
>> 
>> - Revert commits, identified by a commit message that starts with 
>> “Unreviewed, revering…” would be exempt
>> - Changes which only modify files that do not effect building or testing 
>> WebKit would be exempt. These files specifically are:
>>  .github/
>>  JSTests/
>>  ManualTests/
>>  metadata/
>>  PerformanceTests/
>>  Tools/
>>   CISuport/
>>   EWSTools/
>>   WebKitBot
>> Websites/
>> - Emergency build and infrastructure fixes, identified by a commit message 
>> that starts with “Emergency build fix” or “Emergency infrastructure fix” 
>> would be exempt
>> - A reviewer who is not the commit author can overwrite this protection by 
>> adding `unsafe-merge-queue` instead of the commit author
>> - Changes which passed an EWS layout test queue within the last 7 days would 
>> skip the layout test check
>> 
>> These exceptions are designed to provide contributors for a way to by-pass 
>> potentially slow checks if extraordinary situations, or in ones where CI has 
>> already validated the change. I think we should keep the ability for any 
>> committer to deploy an emergency fix, because our project has many 
>> contributors in different timezones and with different holiday schedules.
>> 
>> We know that this policy change would potentially slow down development, so 
>> I think these 3 improvements block making MergeQueue and CommitQueue 
>> mandatory:
>> 
>> - run-webkit-tests consulting results.webkit.org to avoid retrying known 
>> flakey or failing tests
>> - Another MergeQueue bot
>> - Xcode workspace builds to speed up incremental builds
>> 
>> Jonathan Bedard
>> WebKit Continuous Integration
>> 
>> ___
>> 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] Proposal: Mandatory Commit and Merge Queue

[Chris Dumez via webkit-dev]

> On Jun 2, 2022, at 6:29 PM, Michael Catanzaro  wrote:
> 
> 
> 
> On Thu, Jun 2 2022 at 04:06:42 PM -0700, Chris Dumez via webkit-dev 
>  wrote:
>> I am concerned by this proposal given how slow EWS and the merge queue are 
>> these days.
> 
> Hopefully Jonathan's three proposed blockers will address this:
> 
>> - run-webkit-tests consulting results.webkit.org to avoid retrying known 
>> flakey or failing tests
>> - Another MergeQueue bot
>> - Xcode workspace builds to speed up incremental builds

It’s all speculative until we get actual merge queue times with those changes 
implemented. Implementing.a new restrictive policy based on speculative data 
sounds risky to me. Let’s not “hope”, let’s gather data.
___
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

[Michael Catanzaro via webkit-dev]

On Thu, Jun 2 2022 at 04:06:42 PM -0700, Chris Dumez via webkit-dev 
 wrote:
I am concerned by this proposal given how slow EWS and the merge 
queue are these days.




Hopefully Jonathan's three proposed blockers will address this:

- run-webkit-tests consulting results.webkit.org to avoid retrying 
known flakey or failing tests

- Another MergeQueue bot
- Xcode workspace builds to speed up incremental builds


Michael


___
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

[Chris Dumez via webkit-dev]

I am concerned by this proposal given how slow EWS and the merge queue are 
these days. 

I think the issue is likely related to failures on the bots and the retries 
needed to determine if those failures are new.

We’ve never had this restriction before. Seems we are becoming overly strict 
with the move to GitHub and development speed will suffer as a result.

Chris Dumez

> On Jun 2, 2022, at 3:41 PM, Geoffrey Garen via webkit-dev 
>  wrote:
> 
> 
>> As we move to GitHub, I would like to propose we strengthen our protections 
>> on `main` by making MergeQueue and CommitQueue mandatory. 
> 
> 
> What is the goal of this proposal?
> 
> What problem are you trying to solve, and with what level of urgency?
> 
> Thanks,
> Geoff
> 
> 
>> On Jun 2, 2022, at 2:35 PM, Jonathan Bedard via webkit-dev 
>>  wrote:
>> 
>> As we move to GitHub, I would like to propose we strengthen our protections 
>> on `main` by making MergeQueue and CommitQueue mandatory. This would mean 
>> that with a few exceptions, all changes would need to be built and run 
>> layout tests before they are landed. To spell out what the exceptions I had 
>> in mind are:
>> 
>> - Revert commits, identified by a commit message that starts with 
>> “Unreviewed, revering…” would be exempt
>> - Changes which only modify files that do not effect building or testing 
>> WebKit would be exempt. These files specifically are:
>>  .github/
>>  JSTests/
>>  ManualTests/
>>  metadata/
>>  PerformanceTests/
>>  Tools/
>>   CISuport/
>>   EWSTools/
>>   WebKitBot
>> Websites/
>> - Emergency build and infrastructure fixes, identified by a commit message 
>> that starts with “Emergency build fix” or “Emergency infrastructure fix” 
>> would be exempt
>> - A reviewer who is not the commit author can overwrite this protection by 
>> adding `unsafe-merge-queue` instead of the commit author
>> - Changes which passed an EWS layout test queue within the last 7 days would 
>> skip the layout test check
>> 
>> These exceptions are designed to provide contributors for a way to by-pass 
>> potentially slow checks if extraordinary situations, or in ones where CI has 
>> already validated the change. I think we should keep the ability for any 
>> committer to deploy an emergency fix, because our project has many 
>> contributors in different timezones and with different holiday schedules.
>> 
>> We know that this policy change would potentially slow down development, so 
>> I think these 3 improvements block making MergeQueue and CommitQueue 
>> mandatory:
>> 
>> - run-webkit-tests consulting results.webkit.org to avoid retrying known 
>> flakey or failing tests
>> - Another MergeQueue bot
>> - Xcode workspace builds to speed up incremental builds
>> 
>> Jonathan Bedard
>> WebKit Continuous Integration
>> 
>> ___
>> 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

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

[Geoffrey Garen via webkit-dev]

> As we move to GitHub, I would like to propose we strengthen our protections 
> on `main` by making MergeQueue and CommitQueue mandatory. 


What is the goal of this proposal?

What problem are you trying to solve, and with what level of urgency?

Thanks,
Geoff


> On Jun 2, 2022, at 2:35 PM, Jonathan Bedard via webkit-dev 
>  wrote:
> 
> As we move to GitHub, I would like to propose we strengthen our protections 
> on `main` by making MergeQueue and CommitQueue mandatory. This would mean 
> that with a few exceptions, all changes would need to be built and run layout 
> tests before they are landed. To spell out what the exceptions I had in mind 
> are:
> 
> - Revert commits, identified by a commit message that starts with 
> “Unreviewed, revering…” would be exempt
> - Changes which only modify files that do not effect building or testing 
> WebKit would be exempt. These files specifically are:
>  .github/
>  JSTests/
>  ManualTests/
>  metadata/
>  PerformanceTests/
>  Tools/
>CISuport/
>EWSTools/
>WebKitBot
> Websites/
> - Emergency build and infrastructure fixes, identified by a commit message 
> that starts with “Emergency build fix” or “Emergency infrastructure fix” 
> would be exempt
> - A reviewer who is not the commit author can overwrite this protection by 
> adding `unsafe-merge-queue` instead of the commit author
> - Changes which passed an EWS layout test queue within the last 7 days would 
> skip the layout test check
> 
> These exceptions are designed to provide contributors for a way to by-pass 
> potentially slow checks if extraordinary situations, or in ones where CI has 
> already validated the change. I think we should keep the ability for any 
> committer to deploy an emergency fix, because our project has many 
> contributors in different timezones and with different holiday schedules.
> 
> We know that this policy change would potentially slow down development, so I 
> think these 3 improvements block making MergeQueue and CommitQueue mandatory:
> 
> - run-webkit-tests consulting results.webkit.org  
> to avoid retrying known flakey or failing tests
> - Another MergeQueue bot
> - Xcode workspace builds to speed up incremental builds
> 
> Jonathan Bedard
> WebKit Continuous Integration
> 
> ___
> 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] Add CODEOWNERS to WebKit

[Alexey Proskuryakov via webkit-dev]

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.

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 mailto:cdu...@apple.com> > 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] Proposal: Mandatory Commit and Merge Queue

[Jonathan Bedard via webkit-dev]

As we move to GitHub, I would like to propose we strengthen our protections on 
`main` by making MergeQueue and CommitQueue mandatory. This would mean that 
with a few exceptions, all changes would need to be built and run layout tests 
before they are landed. To spell out what the exceptions I had in mind are:

- Revert commits, identified by a commit message that starts with “Unreviewed, 
revering…” would be exempt
- Changes which only modify files that do not effect building or testing WebKit 
would be exempt. These files specifically are:
 .github/
 JSTests/
 ManualTests/
 metadata/
 PerformanceTests/
 Tools/
 CISuport/
 EWSTools/
 WebKitBot
Websites/
- Emergency build and infrastructure fixes, identified by a commit message that 
starts with “Emergency build fix” or “Emergency infrastructure fix” would be 
exempt
- A reviewer who is not the commit author can overwrite this protection by 
adding `unsafe-merge-queue` instead of the commit author
- Changes which passed an EWS layout test queue within the last 7 days would 
skip the layout test check

These exceptions are designed to provide contributors for a way to by-pass 
potentially slow checks if extraordinary situations, or in ones where CI has 
already validated the change. I think we should keep the ability for any 
committer to deploy an emergency fix, because our project has many contributors 
in different timezones and with different holiday schedules.

We know that this policy change would potentially slow down development, so I 
think these 3 improvements block making MergeQueue and CommitQueue mandatory:

- run-webkit-tests consulting results.webkit.org  
to avoid retrying known flakey or failing tests
- Another MergeQueue bot
- Xcode workspace builds to speed up incremental builds

Jonathan Bedard
WebKit Continuous Integration

___
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]

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 
>>  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

Re: [webkit-dev] Add CODEOWNERS to WebKit

[Chris Dumez via webkit-dev]

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 
>  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

Re: [webkit-dev] 안광림 is out of office.

[Ling Ho via webkit-dev]

I have set up a couple of rules to catch and hold based on 
"X-AutoResponse: AutoResponse" and "Auto-submitted: auto-generated". Not 
sure of the latter is needed. We'll have to see if they work tomorrow.

...
ling

On 6/2/22 11:00 AM, Klaus Weidner via webkit-dev wrote:
Short version, you may want to set up a local mail filter to hide 
messages from this list with that subject.


Unfortunately the interactions between mailing lists and 
autoresponders aren't very well standardized. The list messages have a 
List-Id: header which the user agent should use as an indication not 
to send an autoresponse, but that's apparently being ignored in this 
case. On the flip side, a proper vacation autoresponse should set a 
header which the mailing list software should use to discard it, but 
that also doesn't necessarily happen.


https://datatracker.ietf.org/doc/html/rfc3834 defines the 
Auto-Submitted: field, there's obsolescent versions such as 
Precedence: bulk, and many more variants such as these: 
https://www.arp242.net/autoreply.html. As far as I can tell the 
mailing list doesn't use any of them, but it's unclear if it would 
help since mail clients aren't necessarily going to respect them.


On Thu, Jun 2, 2022 at 10:20 AM Alemar via webkit-dev 
 wrote:


These out of office messages are getting out of hand. Is there
anything we can do?

El jue, 2 jun 2022 a las 10:43, 안광림 via webkit-dev
() escribió:


Period : 2022/05/23 00:00 ~ 2022/07/15 00:00 [Korea Standard Time]

부재중입니다.
___
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 mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

[webkit-dev] Add CODEOWNERS to WebKit

[Jonathan Bedard via webkit-dev]

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

Re: [webkit-dev] 안광림 is out of office.

[Klaus Weidner via webkit-dev]

Short version, you may want to set up a local mail filter to hide messages
from this list with that subject.

Unfortunately the interactions between mailing lists and autoresponders
aren't very well standardized. The list messages have a List-Id: header
which the user agent should use as an indication not to send an
autoresponse, but that's apparently being ignored in this case. On the flip
side, a proper vacation autoresponse should set a header which the mailing
list software should use to discard it, but that also doesn't necessarily
happen.

https://datatracker.ietf.org/doc/html/rfc3834 defines the
Auto-Submitted: field,
there's obsolescent versions such as Precedence: bulk, and many more
variants such as these: https://www.arp242.net/autoreply.html. As far as I
can tell the mailing list doesn't use any of them, but it's unclear if it
would help since mail clients aren't necessarily going to respect them.

On Thu, Jun 2, 2022 at 10:20 AM Alemar via webkit-dev <
webkit-dev@lists.webkit.org> wrote:

> These out of office messages are getting out of hand. Is there anything we
> can do?
>
> El jue, 2 jun 2022 a las 10:43, 안광림 via webkit-dev (<
> webkit-dev@lists.webkit.org>) escribió:
>
>>
>> Period : 2022/05/23 00:00 ~ 2022/07/15 00:00 [Korea Standard Time]
>>
>> 부재중입니다.
>> ___
>> 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

Re: [webkit-dev] 안광림 is out of office.

[Alemar via webkit-dev]

These out of office messages are getting out of hand. Is there anything we
can do?

El jue, 2 jun 2022 a las 10:43, 안광림 via webkit-dev (<
webkit-dev@lists.webkit.org>) escribió:

>
> Period : 2022/05/23 00:00 ~ 2022/07/15 00:00 [Korea Standard Time]
>
> 부재중입니다.
> ___
> 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] 안광림 is out of office.

[안광림 via webkit-dev]

Period : 2022/05/23 00:00 ~ 2022/07/15 00:00 [Korea Standard Time]

부재중입니다.
___
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

[Ryosuke Niwa via webkit-dev]

On Thu, Jun 2, 2022 at 4:28 AM Claudio Saavedra via webkit-dev
 wrote:
>
> On Wed, 2022-06-01 at 16:39 -0700, Ryosuke Niwa via webkit-dev wrote:
> > One day per month for one beginner sounds like a really low
> > maintenance cost compared to having every WebKit developer fix non-
> > unified builds at all times.
>
> I'm sorry, but this is not about having "every WK developer fix non-
> unified builds at all times", it's about everyone making sure that
> their patches are correct. A patch that uses API in one file without
> making sure the required headers are included is not a correct patch
> and unified builds are only hiding the issues. I don't see how this can
> be controversial.

We probably disagree on what we mean by "correct".
As far as I'm concerned, a patch is correct if all relevant ports can be built.

There is theoretical correctness of cpp or headers files including all
the necessary headers but that's merely theoretical outside of
non-unified builds. I don't think we necessarily want to spend all our
time thinking about & fixing theoretical problems until they actually
surface.

> Also, there is no pool of beginner developers who can be fixing missing
> includes all the time; even if we had spare manpower, it's more
> beneficial for the project (and themselves) if they spend that time
> doing gardening or fixing actual bugs.

Sorry, that was a typo / bad auto correction of the word "engineer".

- R. Niwa
___
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

[Claudio Saavedra via webkit-dev]

On Wed, 2022-06-01 at 16:39 -0700, Ryosuke Niwa via webkit-dev wrote:
> One day per month for one beginner sounds like a really low 
> maintenance cost compared to having every WebKit developer fix non-
> unified builds at all times.

I'm sorry, but this is not about having "every WK developer fix non-
unified builds at all times", it's about everyone making sure that
their patches are correct. A patch that uses API in one file without
making sure the required headers are included is not a correct patch
and unified builds are only hiding the issues. I don't see how this can
be controversial.

Also, there is no pool of beginner developers who can be fixing missing
includes all the time; even if we had spare manpower, it's more
beneficial for the project (and themselves) if they spend that time
doing gardening or fixing actual bugs.

Claudio

___
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

[dpino via webkit-dev]

Hi Alexey,

On 6/2/22 06:26, Alexey Proskuryakov wrote: 

> Hi, 
> 
> I'm not sure if we have a consensus on whether it is a project goal to keep 
> non-unified build working at all times. As discussed last year, setting up 
> post-commit bots is a pre-requisite for having EWS, so this part is resolved. 
> But proactively maintaining the non-unified build is strictly more work than 
> fixing it on demand. So the immediate response continues to be that we 
> shouldn't be doing more work when we can do less.

Sorry, I disagree with your point of view. 

Now that we have the post-commit non-unified build it has become evident
to me that actively maintaining non-unified builds is less work than
doing it on demand. When the post-commit non-unified is green and a
build fails is trivia to spot which commit introduced a regression and
what it might be the causes. When maintaining the bot on demand, errors
eventually accumulate and becomes less trivia how to fix them. A
non-unified EWS bot has the same benefits as the post-commit non-unified
EWS, plus is the developer who attempts to land the patch who is in
charge of fixing the build. This is more efficient that offloading this
work to other developer. Take for instance, this non-unified build fix I
recently landed:
https://github.com/webkit/webkit/compare/3a5a57d060ee%5E..3a5a57d060ee
It took me a long time to figure out I had to include "JSNodeCustom.h"
in some files, but possibly for the developer authorizing the patch that
introduced the regression, this build error was trivial.

> You mention that embedders who build with non-default flags are more likely 
> to hit issues. Building with non-default flags would be resulting in missing 
> includes for non-unified builds too, do you have an estimate of how much this 
> problem grows due to unified builds?

I don't have stats to answer how often it happens satisfactorily, but I
know that both in WebKitGTK, WPE and other WebKit ports build errors
will eventually happen while building with unified sources. IMHO the
earlier you can find them the better. We have even made WebKitGTK
releases that failed due to unified sources build errors
https://stackoverflow.com/questions/64744576/incomplete-webkitgtk-build.


> How do we decide if everyone is responsible for the convenience of downstream 
> embedders?

I think there's a wrong perception that non-unified build errors are
errors happening in the WebKit ports part of the code. Most of the times
they are build errors in WebCore, JavaScriptCore and many other parts of
the codebase. If you build Mac with non-unified compilation probably
you'll discover a bunch of build errors. I have fixed a bunch of
non-unified build errors in the Mac port while building the Playwright's
version of WebKit for Mac: 

  - [macOS] Unreviewed, non-unified build fixes
https://bugs.webkit.org/show_bug.cgi?id=239889 
  - [macOS] Unreviewed, non-unified build fixes
https://bugs.webkit.org/show_bug.cgi?id=237586 
  - [macOS] Non-unified build fixes
https://bugs.webkit.org/show_bug.cgi?id=236752 

These patches landed upstream, they're not errors in the downstream code
of Playwright. 

So it's not a downstream embedder who introduces a build error in the
codebase, simply they're more likely to discover it because the
introduced downstream changes causes the UnifiedSource files to be
arranged in a different way. Who is the responsible for the build error?
The developer who introduced the build error obviously, not the person
who discovered it. 

>From my point of view, a project's code should be guaranteed to be
always buildable under any supported configuration and it shouldn't rely
on collateral effects to build successful (which is the current
situation with unified sources). I understand the convenience of unified
sources and the advantage they bring. A EWS non-unified bot will
guarantee the entire codebase is always buildable while letting us keep
using unified sources. From time to time, the EWS non-unified bot will
report a build error, and the most suitable person to fix that error
will be in charge of it. And we know, thanks to the post-commit
non-unified bot we deployed, the non-unified bot won't introduce delays
as many times it's even faster than the equivalent unified sources build
bot, and of course it's always faster than the EWS LayoutTests bots. 

> It sounds like none of actively supported ports builds non-unified by 
> default, which I understand from the fact that a special post-commit queue 
> had to be set up for that. 
> 
> Perhaps part of the argument is that even though proactively maintaining the 
> non-unified build is more work, this work is somehow easier than fixing 
> builds on demand. If so, can you elaborate on why this is the case?

Exactly, this how I see it. If we agree that a person authorizing a
patch is always in a better position to fix a build error than somebody
else stranger to that patch, and if we value the time of everybody
equally, then we will