1501792 - Port changes from bug 1458314 to Thunderbird

Status: RESOLVED FIXED

(Thunderbird :: Installer, task)

Description

[Robert Strong (they/them - no direct email)]

Bug 1458314 changes the update directory on Windows.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: 1501792-installer-location.patch

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.

Comment 2

[Jorg K (CEST = GMT+2)]

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

Comment 3

[Robert Strong (they/them - no direct email)]

Checking. I *think* the directory under ProgramData is different due to Thunderbird not defining vendor or something similar and will comment later today.

Comment 4

[Jorg K (CEST = GMT+2)]

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.

Comment 5

[Richard Marti (:Paenglab)]

Attachment: 1501792-installer-location.patch

I'll try "Thunderbird" as directory instead of "Mozilla".
Try: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=f6fe6dd65d65b0d8388d02ae4a931d54f8607ebc

Comment 6

[Robert Strong (they/them - no direct email)]

Thunderbird is correct but the test is failing for a different reason than the installer change.

Comment 7

[Jorg K (CEST = GMT+2)]

OK, I'll take a patch with "Thunderbird".

Comment 8

[Jorg K (CEST = GMT+2)]

Comment on attachment 9020407 [details] [diff] [review]
1501792-installer-location.patch

OK, thanks.

Comment 9

[Robert Strong (they/them - no direct email)]

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

Comment 10

[Richard Marti (:Paenglab)]

checkin-needed removed.

Comment 11

[Robert Strong (they/them - no direct email)]

I have a fix for the test failure and should have it fixed by no later than Monday.

Comment 12

[Jorg K (CEST = GMT+2)]

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?

Comment 13

[Robert Strong (they/them - no direct email)]

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.

Comment 14

[Robert Strong (they/them - no direct email)]

The following calls
https://dxr.mozilla.org/comm-central/source/comm/mail/installer/windows/nsis/uninstaller.nsi#268
https://dxr.mozilla.org/comm-central/source/comm/mail/installer/windows/nsis/installer.nsi#253
need to be changed to

${CleanUpdateDirectories} "Thunderbird" "Mozilla\updates"

Comment 15

[Richard Marti (:Paenglab)]

Attachment: 1501792-installer-location.patch v3

Okay, like this?

Comment 16

[Robert Strong (they/them - no direct email)]

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.

Comment 17

[Jorg K (CEST = GMT+2)]

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.

Comment 18

[Richard Marti (:Paenglab)]

Robert, after landing of bug 1502681 which fixed our test failures, is still something needed or should this bug be okay to land?

Comment 19

[Robert Strong (they/them - no direct email)]

I scanned the changes today and the patch should suffice for the changes in bug 1458314

Comment 20

[Pulsebot]

Pushed by [e-mail]:
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