Port changes from bug 1458314 to Thunderbird

RESOLVED FIXED in Thunderbird 65.0

Status

defect
RESOLVED FIXED
10 months ago
9 months ago

People

(Reporter: rstrong, Assigned: Paenglab)

Tracking

unspecified
Thunderbird 65.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Bug 1458314 changes the update directory on Windows.
Robert, many thanks to file the bug. Please could you check this is correct and we need no other changes?

This is a direct port of https://hg.mozilla.org/mozilla-central/rev/3d22697d9c23#l1.12.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #9019819 - Flags: review?(jorgk)
Attachment #9019819 - Flags: feedback?(robert.strong.bugs)
Robert, while you're here, could you please cast an eye onto bug 1501940. Something wrong with this test:
toolkit/mozapps/update/tests/unit_service_updater/marStageSuccessCompleteSvc.js
Checking. I *think* the directory under ProgramData is different due to Thunderbird not defining vendor or something similar and will comment later today.
Comment on attachment 9019819 [details] [diff] [review]
1501792-installer-location.patch

In bug 1501940 comment #1 I did a try with this patch:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f4f5f2f0e706f73bfa37abdbd5909cbd467698ee

It doesn't fix the test failure which we understand better now, see bug 1501940 comment #4.

That said, looking at the log file
https://taskcluster-artifacts.net/QGpScou5TiWwJSOLwA0qvA/0/public/logs/live_backing.log
we can see the path used:
C:\\ProgramData\\Thunderbird\\updates\\marStageSuccessCompleteSvc\\updates\\0\\update.status
so I don't think the patch is correct. If anything, we should create a "Thunderbird" directory, since that's where updates are placed, for example:
C:\Users\jorgk\AppData\Local\Thunderbird\updates\A88C53A6619672D1\updates\0\update.status
That's a live example from updating TB 52.8 to TB 52.9.1.
Attachment #9019819 - Flags: review?(jorgk)
I'll try "Thunderbird" as directory instead of "Mozilla".
Try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f6fe6dd65d65b0d8388d02ae4a931d54f8607ebc
Attachment #9019819 - Attachment is obsolete: true
Attachment #9019819 - Flags: feedback?(robert.strong.bugs)
Thunderbird is correct but the test is failing for a different reason than the installer change.
OK, I'll take a patch with "Thunderbird".
Attachment #9020407 - Flags: review?(jorgk)
Comment on attachment 9020407 [details] [diff] [review]
1501792-installer-location.patch

OK, thanks.
Attachment #9020407 - Flags: review?(jorgk) → review+
Keywords: checkin-needed
I'm investigating the test failure and I think I will need to put the Thunderbird update directory under Mozilla since other fixes could be exploited. Removing checkin-needed
checkin-needed removed.
Keywords: checkin-needed
I have a fix for the test failure and should have it fixed by no later than Monday.
Thank you very much Robert. Please let's discuss the test failure in bug 1501940. So are you saying that something like
https://github.com/mozilla-releng/OpenCloudConfig/commit/89327b90f836308baf2689de6bb670b99a2f6445
isn't needed for C:\ProgramData\Thunderbird?
Correct, it isn't needed for C:\ProgramData\Thunderbird. We are going to go with C:\ProgramData\Mozilla since the complexity of doing C:\ProgramData\Thunderbird would make it easy to exploit without there being a way to mitigate the exploit.
Okay, like this?
Attachment #9020407 - Attachment is obsolete: true
Attachment #9020571 - Flags: review?(jorgk)
Attachment #9020571 - Flags: feedback?(robert.strong.bugs)
Comment on attachment 9020571 [details] [diff] [review]
1501792-installer-location.patch v3

That is correct but I haven't had a chance to go through the patches in bug 1458314 to verify that there aren't more changes needed.
Attachment #9020571 - Flags: feedback?(robert.strong.bugs) → feedback+
Comment on attachment 9020571 [details] [diff] [review]
1501792-installer-location.patch v3

Thanks, rs=jorgk, pending comment #16. It's really hard for me to review since I know nothing about this, see my incorrect comment #7 as a proof :-(

Maybe in the future we can r? the M-C expert directly as I frequently do when porting M-C changes in DOM, Netwerk, etc.
Attachment #9020571 - Flags: review?(jorgk) → review+
Robert, after landing of bug 1502681 which fixed our test failures, is still something needed or should this bug be okay to land?
Flags: needinfo?(robert.strong.bugs)
I scanned the changes today and the patch should suffice for the changes in bug 1458314
Flags: needinfo?(robert.strong.bugs)
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/13e1bf3609be
Port bug 1458314 to TB: Move the update directory to an installation specific location. rs=jorgk f=rstrong
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 65.0
You need to log in before you can comment on or make changes to this bug.