420506 - (mail-killrdf) Remove RDF use from Thunderbird

Status: RESOLVED FIXED

(Thunderbird :: General, task)

Description

[Joey Minta]

This is a tracking bug for my work getting rid of rdf in mail code.  See generally http://wiki.mozilla.org/Thunderbird:Thoughts_on_Removing_RDF

Some useful mxr searches:
http://mxr.mozilla.org/seamonkey/search?string=nsrdfcid&find=mail&findi=&filter=&tree=seamonkey (42 at the moment)
http://mxr.mozilla.org/seamonkey/search?string=nc-rdf&find=mail&findi=&filter=&tree=seamonkey (952 at the moment)
http://mxr.mozilla.org/seamonkey/search?string=rdfservice&find=mail&findi=&filter=&tree=seamonkey (424 at the moment)

Comment 1

[Joey Minta]

For those wishing to follow along at home, a lot of this development is now going to happen in a public hg repo: http://hg.mozilla.org/users/jminta_gmail.com/kill-rdf/

Comment 2

[Joey Minta]

Attachment: current patch

I'm attaching the current diff of the kill-rdf repository, since we reached a major milestone today: nsMsgDBFolder no longer inherits from nsRDFResource. The datasources have been uncoupled and are no longer packaged. After some more bug-fixing and testing, we should be ready for review

Note: This patch also includes a bunch of changes from jcranmer's subscribe de-rdf work, which is planned to land separately (before this lands).

Comment 3

[Serge Gautherie (:sgautherie)]

Comment on attachment 343843 [details] [diff] [review]
current patch


What is this patch status?

Comment 4

[Joshua Cranmer [:jcranmer]]

(In reply to comment #3)
> (From update of attachment 343843 [details] [diff] [review])
> 
> What is this patch status?

The patch was a diff of the repo, many of the changes of which were committed via other bugs.

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to comment #4)
> The patch was a diff of the repo, many of the changes of which were committed
> via other bugs.

Can it be obsoleted? (Or updated?)

Comment 6

[Joshua Cranmer [:jcranmer]]

Updating the links in the description:

RDF contract header (nsrdfcid):
http://mxr.mozilla.org/comm-central/search?string=nsrdfcid&find=mail&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central
32 matching lines in 31 files (nsAddrDatabase does it twice?)

NC-RDF property specifier (nc-rdf):
http://mxr.mozilla.org/comm-central/search?string=nc-rdf&find=mail&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central
418 matching lines in 45 files:
+ 319 lines in 18 files are for suite/mailnews
+  43 lines in 10 files are for mail/
+  56 lines in 17 files are for mailnews/

RDF service (rdf-service):
http://mxr.mozilla.org/comm-central/search?string=rdfservice&find=mail&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central
352 matching lines in 83 files (mostly in mailnews)

Probably the lowest-hanging fruit with de-RDF now is the address book code.

Comment 7

[Joshua Cranmer [:jcranmer]]

What the hey, it's been 2 years since the last update, which was itself 2 years since the previous one, might as well bless this as a recurring event :-)

RDF contract header is matched by 22 lines in 22 files: (-10 / -9)
+  22 lines in 22 files are for mailnews/

nc-rdf is matched by 392 lines in 42 files:             (-26 / -3)
+ 315 lines in 17 files are for suite/mailnews          ( -4 / -1)
+  39 files in 13 files are for mailnews/               ( -4 / +3)
+  38 lines in 12 files are for mail/                   (-18 / -5)

rdfservice is matched by 259 lines in 61 files:         (-93 /-18)
+  12 lines in  7 files are for suite/mailnews
+ 226 lines in 44 files are for mailnews/
+  21 lines in 10 files are for mail/

If you want the low-hanging fruit:
* RSS internals
* Subscribe dialog
* Bring back the folder lookup service
* Pick your favorite UI widget and remove RDF from it. Especially if you do it in SeaMonkey
* The account manager (maybe?)

Comment 8

[Ben Campbell]

Just wanted to add a link to Bug 1512612, which mentions folders provided by extensions.
I have to admit I'm currently a little ignorant when it comes to extensions, but presumably this means if RDF is ditched there'll be some extension-related work too.

Comment 9

[Ben Campbell]

Attachment: WIP-remove-rdf-from-fls.patch

Posting this mainly to get my thoughts and findings organised.
Very much work-in-progress.

This patch is my experimental attempt to remove RDF from the folder-lookup-service.
Built on top of my c++ patch in Bug 453908 (still in review).
It doesn't call RDF at all, instead it mimics just the behaviour of RDF GetResource() by:

• maintaining its own cache of already-created folders
• using the uri 'scheme' part to pick the right ctor to instantiate new folders.
• calling the folder's Init() function

This should also work fine for folder implementations supplied by extensions.

This all basically works, and so theoretically it's just a matter of removing the nsIRDFResource inheritance from nsMsgDBFolder, which doesn't look hard.

In practice there are a bunch of details to iron out still:

• I think the FLS cache is ditching some folders prematurely (using weak references)

• my C++ patch in Bug 453908 pushes the use of GetExistingFolder(), which never creates folders or returns dangling (parentless) folders. This is causing some friction, and I might have to loosen this a bit.
  I still think the use of dangling folders is really confusing and needs some attention, but I now think we can finish removing the RDF code first.

• braindump ends -

Comment 10

[Ben Campbell]

Attachment: remove-rdf-1.patch

First pass at a patch to remove rdf from the build config and delete the entire rdf directory.

Depends upon the two patches for Bug 1527764, the one in Bug 1527772, and the two in Bug 1534163.

Comment 11

[Jorg K (CEST = GMT+2)]

Let's not attach another patch to remove the rdf/ directory. I can do that with the strike of a pen. Please only attach a patch for the mailnews bits and any other bits, like undoing these:
https://hg.mozilla.org/comm-central/rev/13e2fda05aa3#l1.12
https://hg.mozilla.org/comm-central/rev/02f531f15649

Now for a good laugh, read bug 1459748 comment #8 written on 2018-05-08: We should fix the referenced bugs so that this is only temporarily needed for a few days.

Days was the target in May 2018, not weeks or months or in fact, more than a year. If we had a prize for wishful thinking ...

Comment 12

[Jorg K (CEST = GMT+2)]

Looking at https://hg.mozilla.org/try-comm-central/rev/efba9cba1562639ea83572f1eecb58d3ee9af408, you need to add the reversal of https://hg.mozilla.org/comm-central/rev/02f531f15649.

Comment 13

[Ben Campbell]

(In reply to Jorg K (GMT+2) from comment #12)

Looking at https://hg.mozilla.org/try-comm-central/rev/efba9cba1562639ea83572f1eecb58d3ee9af408, you need to add the reversal of https://hg.mozilla.org/comm-central/rev/02f531f15649.

Doh. Just kicked off another one before reading this (I forgot the feeds patches). Anyway, I've got the other patches rebased, will sort out the details tomorrow.

Comment 14

[Jorg K (CEST = GMT+2)]

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=636be929a2c6645d57a99deb21f5f8d5aa5e3af5
is looking good. The X4 is not your fault and fixed now, the X1 on Windows is also fixed now.

Comment 15

[Ben Campbell]

Attachment: remove-rdf-2.patch

A slimmed-down version to remove the rdf code from the build.
After this, can delete rdf/ and also mailnews/base/src/nsMsgRDFUtils.[h|cpp]

I did one more try build with all the patches piled on, and it looks good:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=9dc8408751e102e4925741e6c49f29da23fa1b01

Comment 16

[Jorg K (CEST = GMT+2)]

Comment on attachment 9095068 [details] [diff] [review]
remove-rdf-2.patch

Thanks. Mark 2019-09-25 on the calendar ;-)

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/62176929a3c9
Disable building of RDF code. r=jorgk
https://hg.mozilla.org/comm-central/rev/86a80299d215
Remove RDF code forked in bug 1459748 and mailnews/base/src/nsMsgRDFUtils.{cpp|h}. r=jorgk