1406499 - Lightning not working because unpacked extension are not supported, remove unpack:true

Status: RESOLVED FIXED

(Calendar :: General, enhancement)

Description

[Richard Marti (:Paenglab)]

With bug 1404697 we have more small files which can degrade the startup performance if the profile is located on a network device or slow hard disks.

Maybe we need to package Lightning in one file to be faster.

Comment 1

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Do you mean unpack:false? It is currently already true due to the former binary component

Comment 2

[Richard Marti (:Paenglab)]

Not tested, bur changing this to 'false' should do the trick: https://dxr.mozilla.org/comm-central/source/calendar/lightning/install.rdf#43. Not sure if this also would pack Lightning in the distribution folder.

Comment 3

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

I think it should. The only thing that comes to mind is that I believe window icons don't work with a packed xpi, but I could be mistaking. Do we have any measurements if this really brings a performance benefit? On a local disk I can imagine extracting a 5mb zip on startup might counter the performance benefit.

If the window icons work and this has a measurable performance benefit, fine with me.

Comment 4

[Richard Marti (:Paenglab)]

I think the window icons are not a problem as in earlier times all XPIs stayed packed and then the icons worked.

About the performance I can't say anything. I opened the bug on MakeMyDay's request.

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

MakeMyDay, do you have concrete measurements? I can imagine the speed gain over a network drive, but we should also keep the regular case in mind since it is more common.

Comment 6

[[:MakeMyDay]]

Not offhand yet, but I recall several complaints for synced Windows profiles and network stored TB profiles back when dealing with the file sync issue at the time we started bundling Lightning. But as this bug about considering the switch, capturing some data first is the right thing to do.

A fairly easy test approach would be to measure startup time with a fresh profile (since we just care about the Lightning files here, not the other data in the TB profile) without additional calendars configured and comparing times with Lightning (a) disabled, (b) packed and (c) unpacked. The tests should be repeated several times each to get a stable measurement. For (b) and (c), the tested package should be over-installed if tested with Daily to make sure Lightning is taken from the user profile.

Maybe it would be helpful to do such tests in different machine setups with a local profile to get a grasp of a standard user's impact (Maybe a devs machine is not a good reference point here, since average users may have less powerful machines in terms of storage type, memory and cpu power).

Additionally, we would need to get the impact tested on a synced Windows user profile and a network stored TB profile to make sure that this has the expected benefit.

Probably we should provide two xpis here (with unpack true and false) and ask for some community support to do such measurement. For the latter scenario, we probably could ask on the enterprise list.

Comment 7

[Jorg K (CEST = GMT+2)]

Since support for "unpack" has just been removed here:
https://hg.mozilla.org/mozilla-central/rev/b8b11082ff88#l2.48 (bug 1444502)
you might as well just remove it.

Lightning, Gdata and two more on the C-C tree:
https://dxr.mozilla.org/comm-central/search?q=em%3Aunpack&redirect=false

Comment 8

[Jorg K (CEST = GMT+2)]

Attachment: 1406499-unpack-true-removal.patch

Comment 9

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8957889 [details] [diff] [review]
1406499-unpack-true-removal.patch

Review of attachment 8957889 [details] [diff] [review]:
-----------------------------------------------------------------

r+ provided mozmill/jsbridge still works afterwards. It would be great if you could determine why mozmill/jsbridge needed this so we know it is safe to remove.

Comment 10

[Jorg K (CEST = GMT+2)]

Well, M-C ignore this now and we have a Daily run with no errors. So this is just a clean-up to remove the option which is no longer supported.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/6654b9108d7e
remove unpack:true from all extensions in C-C after removal of the feature in bug 1444502. r=philipp

Comment 12

[Jorg K (CEST = GMT+2)]

That caused a heap of test failures in Calendar:
JavaScript error: resource://calendar/modules/calUtils.jsm, line 79: ReferenceError: __LOCATION__ is not defined.

I'm reverting the calendar changes.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/50cd20fcaba7
Partly backed out changeset 6654b9108d7e (calendar parts) for causing test failures. a=backout

Comment 14

[Stefan Sitter [:ssitter]]

I tested Lightning 6.2b4 / Thunderbird 60.0b4 (candidate build1 that should include dependent bug 905097) with unpack true removed. Lightning xpi can be installed but during the required restart Thunderbird crashes. Afterwards Lightning is non-functional.

[@ cal::getTimezoneService ] 
MOZ_CRASH(Could not load timezone service, brace yourself and prepare for crash)

https://crash-stats.mozilla.com/report/index/bp-742937b4-12e8-4e78-a512-7ba3d0180421
https://crash-stats.mozilla.com/report/index/bp-d0f5204f-1a68-456d-abed-455f00180421

Comment 15

[Worcester12345]

Perhaps it is time to re-read the initial comment in https://bugzilla.mozilla.org/show_bug.cgi?id=401779 and fix this once and for all.

Comment 16

[Wayne Mery (:wsmwk)]

bug 401779 really is not related to this breakage.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fa20f27405ec
remove unpack:true from Lightning after removal of the feature in bug 1444502. r=philipp

Comment 18

[Jorg K (CEST = GMT+2)]

Comment on attachment 8957889 [details] [diff] [review]
1406499-unpack-true-removal.patch

Approval for the Calendar hunks only, the rest has already landed on TB 60.

Comment 19

[Jorg K (CEST = GMT+2)]

Beta (TB 60 beta 6, Calendar 6.2):
https://hg.mozilla.org/releases/comm-beta/rev/b2d63636dca84397d55784504b141e450b6c8e2a