1519936 - Make sure unified builds of Thunderbird use the same unified files as Firefox

Status: RESOLVED FIXED

(Thunderbird :: Build Config, defect, P1)

Description

[Jorg K (CEST = GMT+2)]

From bug 1442647 comment #30:

... you need to figure out (hopefully with assistance from build people) why there is a difference and eliminate it. Otherwise, you're always going to be fighting an uphill battle, ...

---

The problem is, that the build system glues multiple source files into a unified source file, says: file1 + file2 + file3.

If an include is missing in file3, but provided in file1 or file2, this compilation will succeed. Thunderbird seems to slice up the cake differently, so if it glues together file0 + file1 + file2, and file3 + file4, the missing include in file3 is noticed and the compilation fails.

That's very painful, since TB is busted and we need to take some quick action to fix M-C code, which is always stressful since it needs review and approval, not mentioning finding the issue in the first place.

Another job for you, Rob.

Comment 1

[Jorg K (CEST = GMT+2)]

Can we accelerate this a bit. I had two compile bustages in the last 24 hours since M-C doesn't compile unless we compile it exactly the way they do, see bug 1588065 and bug 1588290.

It's annoying every time having to chase M-C review to fix their compile errors and it always causes an outage.

Comment 2

[Rob Lemley [:rjl]]

I think I see what you mean now. Do you happen to have changeset combos that went with those two most recent bugs? (I can probably figure them out not a big deal.) I'd just like to get a sense of the errors you're getting and I didn't see builds in Treeherder with these problems.

I took the suggestion in bug 1442647 comment #30, and did a simple compare of the current state of the generated Unified* files for --application=browser and --application=comm/mail from the same source tree.

Of the 850+ files in question, the vast majority look okay based on simple md5 checksum comparison.
The Thunderbird build generates six additional files that come out of comm/calendar, comm/common/saxparser, and comm/mailnews.
The Firefox build generates ./memory/replace/dmd/Unified_cpp_memory_replace_dmd0.cpp.

Then there's dom/bindings/UnifiedBindings{1..24}.cpp
Of the 24, 20 don't seem to line up. So that seems to be the hotspot to look at for problems for the moment.

I still don't quite understand what you're looking for as an outcome to this bug though. I think what you're asking for is to find all the files that turn into Unified*.cpp files for Firefox and make sure they get included in a Thunderbird build. IMHO, that's a slippery slope. Aren't there things that make sense in a web browser that an email client doesn't need? WebRTC? Video?

My read on the history of this is that different sets of files may end up being unified depending on the application being built. And to me, that seems perfectly valid given the above assumption.

I think the way to go is to develop some sort way to test a non-unified build. I get that Firefox doesn't want to deal with it and doesn't want to support that type of build, but if there's something that can be run on-demand perhaps that makes the haystack smaller, possibly narrowing it down to an offending file does that help?

For my reference:

MOZCONFIG=mozconfig.empty MOZ_OBJDIR=/tmp/browser mach configure
MOZCONFIG=mozconfig.empty MOZ_OBJDIR=/tmp/mail mach configure --application=comm/mail
find /tmp/browser -name \*Unified\* -exec md5sum '{}' \; > /tmp/browser.txt
find /tmp/mail -name \*Unified\* -exec md5sum '{}' \; > /tmp/mail.txt
meld /tmp/browser.txt /tmp/mail.txt

Comment 3

[Jorg K (CEST = GMT+2)]

Then there's dom/bindings/UnifiedBindings{1..24}.cpp
Of the 24, 20 don't seem to line up. So that seems to be the hotspot to look at for problems for the moment.

Yes, it always breaks in DOM, last breakage here:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=e72e040cbe8e404ce81f9487210abd35255818e6
The red B's, it's green after I fixed M-C and started another Daily.

Comment 4

[Jorg K (CEST = GMT+2)]

Maybe we could make M-C use the unified slicing-up used by C-C compiles. That has found quite a few of their compile errors.

Comment 5

[Jorg K (CEST = GMT+2)]

Also, I notice that every time I do a build, all of dom/ is recompiled which is very time consuming. I can't prove it, but I think there's something wrong.

Comment 6

[Jorg K (CEST = GMT+2)]

Look, eight minutes spent on recompiling something that I think didn't need recompiling. Some dependencies are probably wrong.

9:03.84 dom/abort
[snip: 121 lines deleted]
17:13.56 dom/presentation

Comment 7

[Magnus Melin [:mkmelin]]

(In reply to Rob Lemley [:rjl] from comment #2)

I still don't quite understand what you're looking for as an outcome to this bug though. I think what you're asking for is to find all the files that turn into Unified*.cpp files for Firefox and make sure they get included in a Thunderbird build. IMHO, that's a slippery slope. Aren't there things that make sense in a web browser that an email client doesn't need? WebRTC? Video?

We want it all. Even if we wouldn't it would make it be too difficult to track down problems due to subtle dependencies.

Comment 8

[Rob Lemley [:rjl]]

I ran across this clang include-checking tool earlier. Saving here so I don't lose it.
https://github.com/include-what-you-use/include-what-you-use

Comment 9

[Rob Lemley [:rjl]]

Here is your culprit:
https://searchfox.org/mozilla-central/source/dom/webidl/moz.build#1101-1104

if CONFIG['MOZ_BUILD_APP'] in ['browser', 'mobile/android', 'xulrunner']:
    WEBIDL_FILES += [
        'External.webidl',
    ]

If I change that "xulrunner" to "comm/mail" (it's dead, right?) and rerun my configure commands, I get nearly identical Unified c++ files. The remaining exceptions are the ones listed in comment 2.

External.webidl looks like the AddSearchProvider interface?

Comment 10

[Jorg K (CEST = GMT+2)]

Great, please do a patch adding comm/mail or replacing xulrunner.

Comment 11

[Rob Lemley [:rjl]]

Attachment: Bug 1519936 - Include External.webidl for Thunderbird.

This is to addreess periodic build bustage for Thunderbird that's related
to missing header includes in Unified c++ files. Currently the Unified files
that get generated for Thunderbird are different than for Firefox. Bug 1442647
suggested that the best long term fix would be to find the reason for the
difference and eliminate it.

Comment 12

[Jorg K (CEST = GMT+2)]

Happened again today:
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=3eca33a2f36b1950834d1b1d7ac33a8061a64cbd
Fix is: https://hg.mozilla.org/integration/autoland/rev/b05f555b78d8

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38cfc02d15de
Include External.webidl for Thunderbird.  r=bzbarsky

Comment 14

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/38cfc02d15de

Comment 15

[Jorg K (CEST = GMT+2)]

Looks like this wasn't fixed, see bug 1602113 comment #3.

Comment 16

[Rob Lemley [:rjl]]

Yes, that was a unified issue, but the files that were being generated and compiled were the same for both Firefox and Thunderbird. Something different was going wrong. I don't understand the C++ internals enough to explain more than that.