Closed Bug 1773772 Opened 2 years ago Closed 2 years ago

Port Bug 1773770: Remove XPCOM binary Module infrastructure

Categories

(Thunderbird :: Upstream Synchronization, task)

Tracking

(thunderbird_esr102 unaffected)

RESOLVED FIXED
104 Branch
Tracking Status
thunderbird_esr102 --- unaffected

People

(Reporter: kmag, Assigned: darktrojan)

References

Details

Attachments

(21 files, 1 obsolete file)

10.37 KB, text/x-python
Details
2.70 KB, application/x-shellscript
Details
5.42 KB, application/octet-stream
Details
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

In bug 1773770, I'm removing the old module loading infrastructure, meaning that mozilla::Module and XRE_AddStaticComponent will no longer exist. Thunderbird still appears to be using it for most of its components.

Ideally, those components would be converted to static registration. But the module loading code is not very tightly integrated with the component manager, so it should also be fairly easy for Thunderbird to implement a stub to register its classes/contracts/categories manually using the existing declarations.

No longer regressed by: 1773770
See Also: → 1773770
Type: defect → task
Component: General → Upstream Synchronization
Keywords: regression
Summary: Migrate Thunderbird components away from XRE_AddStaticComponent → Port Bug 1773770: Remove XPCOM binary Module infrastructure

ATM results in
ld.lld: error: duplicate symbol: nsMorkFactoryService::AddRef()

Preparations:

export NAME=NS_MORK_CONTRACTID
export CID=NS_MORK_CID
export VALUE='"@mozilla.org/db/mork;1"'

# Remove CONTRACTID define
grep -rEl $NAME --exclude-dir=.hg --exclude-dir=suite --include="*.cpp" --include="*.h" --exclude=".*" . | xargs xargs sed -i -e "/define $NAME/d"

# Remove CID define
grep -rEl $NAME --exclude-dir=.hg --exclude-dir=suite --include="*.cpp" --include="*.h" --exclude=".*" . | xargs xargs sed -i -e "/NS_DEFINE_NAMED_CID\($CID\)/d"

# Replace CONTRACTID with its string value
grep -rEl $NAME --exclude-dir=.hg --exclude-dir=suite --include="*.cpp" --include="*.h" --exclude=".*" . | xargs xargs sed -i -e "s#$NAME#$VALUE#g"

export HFILE=nsMorkCID.h

# Remove .h file inclusion and remove the .h file
grep -rEl $HFILE --exclude-dir=.hg --exclude-dir=suite --include="*.cpp" --include="*.h" --include="moz.build" --exclude=".*" . | xargs xargs sed -i -e "/$HFILE/d"

find . -name $HFILE | xargs rm

For what it's worth, when I did this for mozilla-central, I added some logging code to the component manager to dump all of the registrations from each module, and then wrote a script and some vim macros to generate components.conf files from that, and then did some verification after the fact. I unfortunately can't find the patch with the logging or the script, but I don't think they were too difficult to write.

Actually, I take that back. I found the generation script. But it's somewhat cryptic and has several old iterations in disabled branches.

Just for a little more context, I believe what that script did to get symbol names was to take the address of a function, and then format the address of that function as if it were a stack frame, and symbolicate the output so the script could parse it. The stack frame printing was similar to https://searchfox.org/mozilla-central/source/toolkit/xre/nsSigHandlers.cpp#72-77 or https://searchfox.org/mozilla-central/source/mozglue/misc/StackWalk.cpp#1013-1016

The '[grelk]' lines were from a function called Gralk (because I understoond, and I wasn't submitting for review...) defined in NS_DECL_ISUPPORTS, that usually gave the filename of the header the class was defined in, useful for knowing the class name and where to write the components.conf file.

The [hilg] lines were constructor functions (like mozilla::storage::VacuumManager::getSingleton).

The [constructor] lines came from the NS_GENERIC_FACTORY_CONSTRUCTOR macro, and its friends. The [cid] and contract lines came from the actual module entries when we iterated over them at startup.

All of which means we missed things which were behind #ifdefs, which I manually found and added after the automatic generation.

This can be used to replace part of the usages, to get some sanity... I didn't have time to finish (and no times next 2 weeks), so just uploading what I had laying around. Will upload the components.conf for it next. WIP untested, though I think complete.

Comment on attachment 9282968 [details]
script to replace nsMsgCompCID.h things across the codebase

For what it's worth, I'd suggest giving these 'name' properties in components.conf, and then they can be accessed via mozilla::components::ComponentName::Service() rather than contract ID. It's also more efficient, since it bypasses a hashtable lookup.

Assignee: nobody → geoff
Status: NEW → ASSIGNED

We don't need to keep using this and it's getting in the way, so rather than figure out how to deal with it, I'm going to remove it.

Attachment #9283584 - Attachment description: WIP: Bug 1773772 part 10 - Convert MIME binary components registration to components.conf. → WIP: Bug 1773772 part 10 - Convert MAPI binary components registration to components.conf.

I'm going to land what I have so we can get going again. There's a still a few problems which I'll come back to:

  • There's a bit of calendar drag/drop weirdness from the calendar changes in part 0.
  • The shell service on Linux is broken and I can't work out why.
  • The S/MIME tests are playing up because of the changes needed to make OpenPGP work properly. I think it's just the tests.
  • I've had to disable the Outlook address book and Outlook import to make Windows build.
Keywords: leave-open
Target Milestone: --- → 104 Branch
Attachment #9283535 - Attachment description: WIP: Bug 1773772 part 0 - Stop using and remove CreateNewSyncStreamListener. → Bug 1773772 part 0 - Stop using and remove CreateNewSyncStreamListener. r=aleca
Attachment #9283536 - Attachment description: WIP: Bug 1773772 part 1 - Convert calendar binary components registration to components.conf. → Bug 1773772 part 1 - Convert calendar binary components registration to components.conf. rs=aleca
Attachment #9283537 - Attachment description: WIP: Bug 1773772 part 2 - Convert Mork binary components registration to components.conf. → Bug 1773772 part 2 - Convert Mork binary components registration to components.conf. rs=aleca
Attachment #9283538 - Attachment description: WIP: Bug 1773772 part 3 - Convert mail binary components registration to components.conf. → Bug 1773772 part 3 - Convert mail binary components registration to components.conf. rs=aleca
Attachment #9283539 - Attachment description: WIP: Bug 1773772 part 4 - Convert address book binary components registration to components.conf. → Bug 1773772 part 4 - Convert address book binary components registration to components.conf. rs=aleca
Attachment #9283579 - Attachment description: WIP: Bug 1773772 part 5 - Convert compose binary components registration to components.conf. → Bug 1773772 part 5 - Convert compose binary components registration to components.conf. rs=aleca
Attachment #9283580 - Attachment description: WIP: Bug 1773772 part 6 - Convert mailnews base binary components registration to components.conf. → Bug 1773772 part 6 - Convert mailnews base binary components registration to components.conf. rs=aleca
Attachment #9283581 - Attachment description: WIP: Bug 1773772 part 7 - Convert protocol binary components registration to components.conf. → Bug 1773772 part 7 - Convert protocol binary components registration to components.conf. rs=aleca
Attachment #9283582 - Attachment description: WIP: Bug 1773772 part 8 - Convert remaining mailnews binary components registration to components.conf. → Bug 1773772 part 8 - Convert remaining mailnews binary components registration to components.conf. rs=aleca
Attachment #9283583 - Attachment description: WIP: Bug 1773772 part 9 - Convert import binary components registration to components.conf. → Bug 1773772 part 9 - Convert import binary components registration to components.conf. rs=aleca
Attachment #9283584 - Attachment description: WIP: Bug 1773772 part 10 - Convert MAPI binary components registration to components.conf. → Bug 1773772 part 10 - Convert MAPI binary components registration to components.conf. rs=aleca
Attachment #9283585 - Attachment description: WIP: Bug 1773772 part 11 - Convert remaining binary components registration to components.conf. → Bug 1773772 part 11 - Convert remaining binary components registration to components.conf. rs=aleca
Flags: needinfo?(geoff)
Attachment #9283971 - Attachment description: Bug 1773772 - Fix initialisation of nsGNOMEShellService, nsWindowsShellService and nsMailWinSearchHelper. r=#thunderbird-reviewers → Bug 1773772 part 13 - Fix initialisation of nsGNOMEShellService, nsWindowsShellService and nsMailWinSearchHelper. r=#thunderbird-reviewers

I didn't do this bit with the others as it was already somewhat broken (bug 1562313).

See Also: → 1562313

Pulsebot missed a push:
Bug 1773772 part 13 - Fix initialisation of nsGNOMEShellService, nsWindowsShellService and nsMailWinSearchHelper.
https://hg.mozilla.org/comm-central/rev/40b247165baa05a2bb80f4624d8f9c9ef4878bc3

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/b8b5282c9312
part 14 - Convert JS account binary components registration to components.conf. r=freaktechnik
  • The shell service on Linux is broken and I can't work out why.

Fixed by part 13.

  • I've had to disable the Outlook address book and Outlook import to make Windows build.

I'm going to re-enable the import as that didn't need disabling after all.

For the other things, I'm going to file follow-up bugs and close this bug. I'm needed elsewhere at this point.

Regressions: 1778040
Regressions: 1778041

Can you please answer comment #24 and comment #25. What was the motivation to remove "@mozilla.org/network/sync-stream-listener;1" instead of moving it to static registration? Using "@mozilla.org/network/sync-stream-listener;1" seems to be the standard way to read a message in WE experiments and there are a few that will break. Moving from a sync to an async call adds to the complication.

What about comment #21 (quote): There's a bit of calendar drag/drop weirdness from the calendar changes in part 0 [part 0 being the said removal].

Would you accept a patch to restore "@mozilla.org/network/sync-stream-listener;1"? and backout part 0. If so, please file a bug for it. NI'ing John for the add-on impact.

Flags: needinfo?(john)

(In reply to Barbara from comment #31)

Would you accept a patch to restore "@mozilla.org/network/sync-stream-listener;1"? and backout part 0. If so, please file a bug for it. NI'ing John for the add-on impact.

I intend to do just that. However I can't do everything at once so you'll need to be patient.

Flags: needinfo?(john)
Flags: needinfo?(geoff)

Thank you for the answer (it wasn't clear from the proceedings here, hence I asked).

Backs out part 0 (revision 0b2e593b3fe8) and registers nsSyncStreamListener using components.conf

Depends on D150998

The S/MIME tests need the default (PGP) handler replaced before they pass.

Depends on D150999

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/9c5379af221d
part 15 - Restore nsSyncStreamListener XPCOM registration. rs=me
https://hg.mozilla.org/comm-central/rev/af731ad25843
part 16 - Restore Outlook import. rs=me
Keywords: leave-open

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/c290c74d2ab3
part 17 - Fix signed message handler registration and tests. r=kaie
https://hg.mozilla.org/comm-central/rev/f9d2da9881a6
part 18 - Don't skip PGP component registering when reloading a recent message. r=darktrojan

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/64874c960394
part 17.1 - Fix win32 build. rs=bustage-fix DONTBUILD
Attachment #9282444 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: