Closed Bug 1600545 Opened 3 months ago Closed 3 months ago

Mechanically remove useless inclusions of header files generated from IDL templates

Categories

(Core :: General, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla73
Tracking Status
firefox73 --- fixed

People

(Reporter: gsvelto, Assigned: gsvelto)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

As per the title. I made a crude script to remove as many useless inclusions as possible and patched up the rest by hand.

Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Priority: -- → P2

Can't attach my patch because our Phabricator instance rejects it as too large... I'll have to split it out in multiple parts.

I fixed up a bunch of breakage that crept in during the rebases and I'm running a last try run. If it turns out green I'll land this right away; down with the useless inclusions!

Pushed by gsvelto@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f6e53d1c6518
Remove useless inclusions of header files generated from IDL files in dom/ r=Ehsan

This is odd, I didn't encounter this issue locally nor in my try builds. I'll investigate a bit further before relanding.

Flags: needinfo?(gsvelto)

What's even stranger is that most build failures seem to be caused by missing the nsIXPConnectWrappedJS definition in toolkit/components/sessionstore/SessionStoreListener.cpp, however my patch was explicitly include nsIXPConnect.h in that file. Maybe lando mis-merged something before actually landing? The log only shows the first patch being landed (the one containing the changes to the files under dom/) so it's even odder.

OK, now I get it. If I build with only the first patch applied the build breaks, that's why. I always tested with all three patches or none. Fixing it now.

Pushed by gsvelto@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d235a06ca6be
Remove useless inclusions of header files generated from IDL files in dom/ r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/73820a049c64
Remove useless inclusions of header files generated from IDL files in accessible/, browser/, caps/, chrome/, devtools/, docshell/, editor/, extensions/, gfx/, hal/, image/, intl/, ipc/, js/, layout/, and media/ r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/1f438b01c780
Remove useless inclusions of header files generated from IDL files in modules/, netwerk/, parser/, security/, startupcache/, storage/, toolkit/, tools/, uriloader/, widget/, xpcom/ and xpfe/ r=Ehsan
Status: ASSIGNED → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/integration/autoland/rev/207d9b1b2b6c
add back some includes needed to un-bust comm-central. r=sylvestre

(In reply to Magnus Melin [:mkmelin] from comment #13)

This caused some comm-central breakage. Looks like this: https://hg.mozilla.org/try/rev/4c97a9896dae515dd254d3c9b3240af53099ef00 helps.
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=ed6f5f34c520d586059e7ed1080a53f036a5a867

Sorry for the breakage.

No worries. We thought we had fixed the Thunderbird build process so it wouldn't run into problems with missing includes, but apparently there is still work to do there.

FYI, that patch is a solution, but may not be the ideal solution. (I wrote it.) I just blindly put back some lines where they were removed, but I suspect they should be elsewhere, and possibly including something other than nsIDocShell.h. You may want to follow up.

Depends on: 1603919
You need to log in before you can comment on or make changes to this bug.