Closed Bug 420506 (mail-killrdf) Opened 14 years ago Closed 2 years ago

Remove RDF use from Thunderbird

Categories

(Thunderbird :: General, task)

task
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 71.0

People

(Reporter: jminta, Assigned: benc)

References

(Blocks 1 open bug)

Details

(Keywords: helpwanted, meta)

Attachments

(2 files, 2 obsolete files)

Depends on: 421382
Depends on: 421428
Depends on: 421443
Depends on: 421786
Depends on: 422474
Depends on: 422572
Depends on: 422845
Depends on: 435804
Depends on: 436630
Depends on: 436673
Depends on: 436677
Depends on: 436718
Depends on: 437869
Depends on: 439236
Depends on: 439364
Depends on: 439373
Depends on: 441437
Depends on: 442802
Depends on: 449260
Depends on: 453820
Depends on: 453908
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/
Depends on: 457333
Attached patch current patchSplinter Review
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).
Depends on: 460952
Blocks: 460953
Depends on: 464710
Comment on attachment 343843 [details] [diff] [review]
current patch


What is this patch status?
(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.
(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?)
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.
Depends on: 541854
Depends on: 652855
Blocks: 657604
Blocks: 657607
No longer blocks: 657604
No longer blocks: 460953
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?)
Depends on: 732106
Assignee: jminta → nobody
Keywords: helpwanted
Summary: jminta's war on rdf → Remove RDF use from Thunderbird
Depends on: 878604
No longer depends on: 442802
Blocks: cc-backlog
Depends on: 1056649
Blocks: 211804
Depends on: 1467238
Depends on: 1495101
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.
Attached patch WIP-remove-rdf-from-fls.patch (obsolete) — Splinter Review

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 -

Assignee: nobody → benc
Depends on: 1527764
Depends on: 1527772
Attachment #9035873 - Attachment is obsolete: true
Depends on: 1532173
Depends on: 1532177
Depends on: 1532179
Depends on: 1534163
Depends on: 1534530
Attached patch remove-rdf-1.patch (obsolete) — Splinter Review

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.

Blocks: 1517464, 1543183
No longer blocks: 1543183
Type: defect → task

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 ...

(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.

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.

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

Attachment #9055386 - Attachment is obsolete: true
Attachment #9095068 - Flags: review?(jorgk)
Comment on attachment 9095068 [details] [diff] [review]
remove-rdf-2.patch

Thanks. Mark 2019-09-25 on the calendar ;-)
Attachment #9095068 - Flags: review?(jorgk) → review+

Pushed by mozilla@jorgk.com:
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

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 71.0
See Also: → SM-killrdf
You need to log in before you can comment on or make changes to this bug.