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

[Adrian Perez de Castro]

Hi list,

On Thu, 08 Sep 2022 02:27:53 +0000 Alexey Proskuryakov via webkit-dev <webkit-dev at lists.webkit.org> wrote:
> Ross, I didn't mean any disrespect.
 
> I absolutely agree that this issue is not about the project supporting
> multiple platforms.
 
> What I disagree with is the statement that omitting includes necessary for
> non-unified build is a mistake, or that it needs to be made visible. Fixing
> all of these is very time consuming, whether it's done pre- or post-commit.
> In my mind, it is a logical conclusion that this shouldn't be done at all,
> neither pre- nor post-commit. The downside is that patches are more likely
> to break the build, but overall it's less work, because only a small
> percentage of "missing" includes will ever cause problems. So, ignoring
> non-unified build saves work over time, and saves the 6 days per month that
> its maintenance has been costing.
> 
> Perhaps that's incorrect, and the cost situation is the opposite? So that
> ignoring non-unified builds is costlier overall? It is a real cost, in
> particular because it sometimes requires fixing issues in unfamiliar code,
> but that cost hasn't been quantified AFAICT.

My personal experience is that building locally with non-unified builds while
working on making a patch practically does not add any overhead -- most of the
time I always work with non-unified builds.

What takes more time is to fix the non-unified build caused by other people's
patches if the non-unified build has been broken since the last time I pulled
from the repository. More precisely: sometimes it takes a lot of time, and
often it does not take much to fix the build itself; but even for the simple
ones having to write commit logs, waiting for the EWS, and then some more
time while commit-queue does its thing easily adds 30 to 40 minutes before
I can go on with what I initially planned to work on.

> In other words, the choices are:
> 
> With non-unified EWS:
> - many patches get rejected for breaking it;
> - it's easy for the patch author to add the includes.

  - ensures that patches we land are as correct as possible

Patches rejected for breaking it are patches which are technically wrong. I am
sure we all agree that unified builds are a clever, but disgusting workaround
to speed up builds. If went back to 2017 for a moment, when we did not have
unified builds [1], We would be rejecting these patches anyway, because they
do break the build. And nobody would disagree.

> Without non-unified EWS, or anyone fixing non-unified build manually:
> - a smaller number of patches gets rejected for breaking existing EWS builders;
> - it's sometimes harder to fix, because the errors could be in unfamiliar code (although how hard can it be to add an include, on average).

Not fixing the build ever would not work. If we don't land patches which are
technically correct there will be always situations in which the build will
randomly break.

Example: Buildbot (both EWS and post-commit) build with DEVELOPER_MODE=ON and
ENABLE_EXPERIMENTAL_FEATURES=ON, while end user builds disable those. The
changed options can result in shifted sources in the unified build compilation
units. What passes as "yeah, it builds" in the bots could still fail later
when producing releases. This applies to *all* ports.

> Without non-unified EWS, but someone fixes non-unified build manually:
> - an even smaller number of patches gets rejected due to missing includes;
> - it's a huge investment on the part of those who keep fixing the non-unified build.

Cheers,
—Adrián

---
[1] https://bugs.webkit.org/show_bug.cgi?id=177190

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20220908/b71ca453/attachment.bin>