Port Bug 1773770: Remove XPCOM binary Module infrastructure
Categories
(Thunderbird :: Upstream Synchronization, task)
Tracking
(thunderbird_esr102 unaffected)
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.
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Comment 1•2 years ago
|
||
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
Comment 2•2 years ago
|
||
We have a lot of them... https://searchfox.org/comm-central/search?q=CID.h&path=
Reporter | ||
Comment 3•2 years ago
|
||
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.
Reporter | ||
Comment 4•2 years ago
|
||
Actually, I take that back. I found the generation script. But it's somewhat cryptic and has several old iterations in disabled branches.
Reporter | ||
Comment 5•2 years ago
|
||
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 #ifdef
s, which I manually found and added after the automatic generation.
Comment 6•2 years ago
|
||
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 7•2 years ago
|
||
Reporter | ||
Comment 8•2 years ago
|
||
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 | ||
Updated•2 years ago
|
Assignee | ||
Comment 9•2 years ago
|
||
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.
Assignee | ||
Comment 10•2 years ago
|
||
Depends on D150669
Assignee | ||
Comment 11•2 years ago
|
||
Depends on D150670
Assignee | ||
Comment 12•2 years ago
|
||
Depends on D150671
Assignee | ||
Comment 13•2 years ago
|
||
Depends on D150672
Assignee | ||
Comment 14•2 years ago
|
||
Depends on D150673
Assignee | ||
Comment 15•2 years ago
|
||
Depends on D150709
Assignee | ||
Comment 16•2 years ago
|
||
Depends on D150710
Assignee | ||
Comment 17•2 years ago
|
||
Depends on D150711
Assignee | ||
Comment 18•2 years ago
|
||
Depends on D150712
Assignee | ||
Comment 19•2 years ago
|
||
Depends on D150713
Assignee | ||
Comment 20•2 years ago
|
||
Depends on D150714
Updated•2 years ago
|
Assignee | ||
Comment 21•2 years ago
|
||
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.
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Updated•2 years ago
|
Assignee | ||
Comment 22•2 years ago
|
||
bugherder |
https://hg.mozilla.org/comm-central/rev/0b2e593b3fe8
https://hg.mozilla.org/comm-central/rev/debe008f7001
https://hg.mozilla.org/comm-central/rev/1347dba0f399
https://hg.mozilla.org/comm-central/rev/1730225ec53c
https://hg.mozilla.org/comm-central/rev/ceb162c71d59
https://hg.mozilla.org/comm-central/rev/03f46acb409a
https://hg.mozilla.org/comm-central/rev/83c02e86ae91
https://hg.mozilla.org/comm-central/rev/56d32ad022a4
https://hg.mozilla.org/comm-central/rev/3a8a7b3f6943
https://hg.mozilla.org/comm-central/rev/3215271f9748
https://hg.mozilla.org/comm-central/rev/7ddca370a09e
https://hg.mozilla.org/comm-central/rev/acda4b051fe2
Assignee | ||
Comment 23•2 years ago
|
||
Pulsebot is sleeping apparently.
https://hg.mozilla.org/comm-central/rev/3e86564b260176f87f67dd22e3bdcb5e8f432d02
Comment 24•2 years ago
|
||
Why did you remove "@mozilla.org/network/sync-stream-listener;1"? The pEp Project uses that in a code section that cannot be made asynchronous since it's running as part of the PGP Proxy:
https://pep-security.lu/gitlab/thunderbird/pEpForThunderbird/-/blob/a6bab3b4fcc51ef87ebcd8dc0580c01bfd2be676/addon/content/pEpForThunderbird.js#L445
https://pep-security.lu/gitlab/thunderbird/pEpForThunderbird/-/blob/a6bab3b4fcc51ef87ebcd8dc0580c01bfd2be676/addon/content/pEpMimeDecrypt.js#L42
https://pep-security.lu/gitlab/thunderbird/pEpForThunderbird/-/blob/a6bab3b4fcc51ef87ebcd8dc0580c01bfd2be676/addon/content/pEpMimeDecrypt.js#L77
The onStopRequest at line 42 is part of the PGP Proxy API and is synchronous.
https://searchfox.org/comm-central/rev/b23997597a3a775a528d91627744eeba6dec7bf5/mailnews/mime/public/nsIPgpMimeProxy.idl#27
nsIStreamListener inherits from nsIRequestObserver.
Please consider restoring this.
Comment 25•2 years ago
|
||
Assignee | ||
Comment 26•2 years ago
|
||
Updated•2 years ago
|
Assignee | ||
Comment 27•2 years ago
|
||
I didn't do this bit with the others as it was already somewhat broken (bug 1562313).
Comment 28•2 years ago
|
||
Pulsebot missed a push:
Bug 1773772 part 13 - Fix initialisation of nsGNOMEShellService, nsWindowsShellService and nsMailWinSearchHelper.
https://hg.mozilla.org/comm-central/rev/40b247165baa05a2bb80f4624d8f9c9ef4878bc3
Comment 29•2 years ago
|
||
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
Assignee | ||
Comment 30•2 years ago
|
||
- 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.
Comment 31•2 years ago
|
||
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.
Assignee | ||
Comment 32•2 years ago
|
||
(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.
Comment 33•2 years ago
|
||
Thank you for the answer (it wasn't clear from the proceedings here, hence I asked).
Assignee | ||
Comment 34•2 years ago
|
||
Backs out part 0 (revision 0b2e593b3fe8) and registers nsSyncStreamListener using components.conf
Assignee | ||
Comment 35•2 years ago
|
||
Depends on D150998
Assignee | ||
Comment 36•2 years ago
|
||
The S/MIME tests need the default (PGP) handler replaced before they pass.
Depends on D150999
Comment 37•2 years ago
|
||
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
Comment 38•2 years ago
|
||
Assignee | ||
Updated•2 years ago
|
Comment 39•2 years ago
|
||
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
Comment 40•2 years ago
|
||
Pushed by geoff@darktrojan.net: https://hg.mozilla.org/comm-central/rev/64874c960394 part 17.1 - Fix win32 build. rs=bustage-fix DONTBUILD
Updated•1 year ago
|
Description
•