I think Ross’ point about supported ports being bitten by missing includes is
valid. I also agree that it can take more time (possibly a lot more time) for
an engineer stepping on the issue later in time to fix the missing includes vs
the original author fixing it if a non-unified EWS points out where the issue
is.
I have personally encountered this myself on Mac builds. The build error seemed
non-sensical at first as it had nothing to do with the patch I was developing.
This wasted some time for me to figure out that it wasn’t because I
accidentally made a bad edit somewhere (especially when my patch is necessarily
large). Once I realized that it was due to skew in the unification boundaries,
figuring out what include needed to be added also wasn’t obvious (at least at
the time where I encountered this years ago). I will grant that it appears to
not happen very often anymore (at least in my experience, I hasn’t happened to
me since), but there is an argument to be made that this may be thanks to
Italia’s effort of fixing them on a regular basis, and preventing it from
becoming a problem.
Anecdotal data aside, I would like to make a few points:
1. Build problems with skew in the unified files will only occur when one adds
new .cpp files into the unified mix. This means:
a. It will not affect most engineers who are just modifying code in
existing files.
b. It may affect engineers who are building features that require adding
new .cpp files (not .h files).
c. It may affect engineers who are refactoring code when it requires adding
new .cpp files.
d. It may affect engineers working on non-supported ports that are not
using unified ports.
The activities in (a) and (b) are common. (c) is more rare. (d), we agree we
shouldn’t care about (in the sense that it should not be a burden on engineers
of supported ports). As someone who does a lot of (b) and (c) activities, I
appreciate having this non-unified EWS.
Note that (a), (b), and (c) applies to supported ports working with unified
ports. This is not a port-specific issue, nor is just an unsupported port
issue.
2. Unlike build issues due platform specific code, build issues due to missing
includes tend to be easy to fix if identified by a non-unified EWS because:
a. The error will point exactly at the .cpp file that is failing to compile
which need the missing include (as opposed to a unified file with many .cpp
files).
b. The missing include is directly relevant to the patch being worked by
the engineer at the time. Hence, it is very likely the author of the patch
will immediately recognize the type / variable declaration that is missing, and
know which header file to include.
There is a rare chance that the non-unified build error involves some platform
specific code. In this case, I think it is fair to just ping the platform
maintainers on slack to give them a heads up about it, and ask them to help
resolve it. It does not need to block the patch from landing. Even so, this
makes it way easier for the platform maintainers to figure out the issue rather
than unsuspectingly stepping on it much later after hundreds of patches have
landed.
Some more thoughts in response to Darin below ...
> On Jun 4, 2022, at 5:42 AM, Darin Adler via webkit-dev
> wrote:
>
> Yes, I don’t oppose adding another EWS bot, and I was not trying to argue
> against that proposal. I did intend to express my disagreement with many
> points made in follow-up replies that seem quite wrong to me.
>
> Three other thoughts:
>
> 1) Even though I don’t object to adding a new bot, I will say that the idea
> that a single “non-unified” bot will add a lot of value at very little cost
> to the WebKit project doesn’t sound right to me. These arguments about how
> long things will take don’t seem correct. My experience is that it’s quite
> difficult to find, understand, and resolve errors seen in bot builds, far
> more difficult than resolving errors I can see locally as I make code
> changes. In my view every EWS bot we add helps weigh down the project, making
> each change more difficult.
I agree that this is true in general. However, it the build issue is due to a
missing include, I think this is not true (see my point (2) above).
> 2) In my contributions, I don’t just want to add missing includes, I want to
> remove unnecessary ones taking full advantage of forward declarations and
> moving code out of headers. Too many includes and too much dependency has a
> dramatic bad effect on the project, making colossal project build times even
> worse.
I too try to do this when writing my patches. I would argue that a non-unified
EWS gives me greater confidence that a forward declaration is sufficient, and
that I’m not just mistakenly thinking that it is only because some other file
in the same unified unit also happen to include the header. So, again, a
non-unified EWS is helping here.
> 3) We
