Merge mozbuild files into tor-launcher.git

added component::applications/tor launcher owner::brade parent::24856 priority::medium resolution::wontfix severity::normal status::closed type::enhancement labels

I'm a little worried about jar.mn and chrome.manifest becoming out of sync, but I can also write a small script that generates the chrome.manifest file from jar.mn, and it can run automatically in during the pkg-prepare make target. It's something to consider.

Trac:
Cc: mcs, arthuredelstein, gk to mcs, arthuredelstein, gk, sukhbir

First attempt is here: https://github.com/sysrqb/tor-launcher/tree/bug25260_mozbuild

It adds files:

moz.build
jar.mn

It Modifies Makefile:

adding a tb-prepare target

It moves defaults/preferences/prefs.js -> defaults/preferences/torlauncher.js

This is necessary so during tor-browser build, torlauncher's prefs.js doesn't overwrite or get overwritten by other prefs file.

Just a thought: have we looked at or are considering the use of extensions.legacy.exceptions to make XUL/XPCOM legacy add-ons still work? Sorry for commenting without actually investigating it but I had planned to do it for TorBirdy as well but at least for that it is no longer required since Thunderbird is not dropping support for legacy add-ons, at least not in the current beta till Thunderbird 60. (From https://wiki.mozilla.org/Thunderbird/Add-ons_Guide_57).

Do we know if this preference will be enough to continue supporting legacy add-ons (Firefox 58 seems to be using it) or are further architectural changes required?

Replying to sukhbir:

Just a thought: have we looked at or are considering the use of extensions.legacy.exceptions to make XUL/XPCOM legacy add-ons still work? Sorry for commenting without actually investigating it but I had planned to do it for TorBirdy as well but at least for that it is no longer required since Thunderbird is not dropping support for legacy add-ons, at least not in the current beta till Thunderbird 60. (From https://wiki.mozilla.org/Thunderbird/Add-ons_Guide_57).

Do we know if this preference will be enough to continue supporting legacy add-ons (Firefox 58 seems to be using it) or are further architectural changes required?

This seems definitely worth investigating. Of course, to choose this option, it would need to work for mobile as well.

After talking with Arthur about this, we decided it was worthwhile investigating if we can include the tor-launcher extension as an isolated file, while with the above branch, the extension is built into the browser omni.ja. If we continue building and distributing tor-launcher as an separate component within the installation directory, this allows for easier updating (if it is ever needed). Specifically, the Mozilla provides a capability of including "features" within the tree and these are loaded as system addons (see toolkit/mozapps/extensions/docs/SystemAddons.rst for more details).

As it turns out, this is extremely easy. In fact, it seems we don't need to patch tor-launcher or firefox for this. Instead of copying tor-launcher into the profile extensions directory, we can copy it directly into browser/features/ (currently there is only e10srollout extension there). I tested this on a fresh tor browser build on Linux. It'd be good if someone can confirm this on OS X and Windows, as well. I also tested this on Android and it works (in so far as Orfox loads and TorLauncher renders a popup saying it can't execute the tor process).

Replying to sukhbir:

Do we know if this preference will be enough to continue supporting legacy add-ons (Firefox 58 seems to be using it) or are further architectural changes required?

I haven't looked at setting extensions.legacy.exceptions closely, but after a quick look at [[https://dxr.mozilla.org/mozilla-central/search?q=legacy.exception&redirect=false|m-c]] it looks like this may work, but we'll need two pieces: append the extensions in the list of extensions.legacy.exceptions and enable extensions.legacy.enabled.

I dug a little deeper on this, and I build the patch from mozilla-central. It seems some changes were introduced since ESR52. 1) At build time all system addons are added into a file, and that file is checked at application startup[0], so we won't be able to simply copy the extension into browser/features/ post-build, we'll need to modify this file too [1]. This is the situation on desktop. On mobile, I don't see where this file is created, so right now I think "features" are broken on mobile. I'll dig into this some more.

[0] https://bugzilla.mozilla.org/show_bug.cgi?id=1348981 [1] obj-*/dist/bin/browser/chrome/browser/content/browser/built_in_addons.json [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1277920

Replying to sysrqb:

I dug a little deeper on this, and I build the patch from mozilla-central. It seems some changes were introduced since ESR52. 1) At build time all system addons are added into a file, IDs of add-ons and that file is checked at application startup[0], so we won't be able to simply copy the extension into browser/features/ post-build, That's the goal [0] was created for. we'll need to modify this file too [1]. It's auto-generated, no? This is the situation on desktop. On mobile, I don't see where this file is created, so right now I think "features" are broken on mobile. I'll dig into this some more. If you add Tb and Tl to "features", they'll become part of firefox build, no post-build modifications are required. [0] https://bugzilla.mozilla.org/show_bug.cgi?id=1348981 [1] obj-*/dist/bin/browser/chrome/browser/content/browser/built_in_addons.json [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1277920 Generally, as Tb and Tl don't require updates, the better way to integrate them is mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1212059#c5

Replying to cypherpunks:

Replying to sysrqb:

I dug a little deeper on this, and I build the patch from mozilla-central. It seems some changes were introduced since ESR52. 1) At build time all system addons are added into a file, IDs of add-ons

Yes

and that file is checked at application startup[0], so we won't be able to simply copy the extension into browser/features/ post-build, That's the goal [0] was created for. we'll need to modify this file too [1]. It's auto-generated, no?

Yes https://dxr.mozilla.org/mozilla-central/source/browser/app/Makefile.in#110

This is the situation on desktop. On mobile, I don't see where this file is created, so right now I think "features" are broken on mobile. I'll dig into this some more. If you add Tb and Tl to "features", they'll become part of firefox build, no post-build modifications are required. [0] https://bugzilla.mozilla.org/show_bug.cgi?id=1348981 [1] obj-*/dist/bin/browser/chrome/browser/content/browser/built_in_addons.json [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1277920 Generally, as Tb and Tl don't require updates, the better way to integrate them is mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1212059#c5

Adding the torlauncher extension into the omni.ja remains an option. However, considering I haven't found where the builtin-addon manifest is created during the mobile build process, I'm not sure system addons actually work on Android.

After a few more days with this, and chatting with some Mozilla devs, I made some more progress on this. In the short term, we can bundle tor-launcher as a system add-on and it should (mostly) just-work. This will require a small patch in tor-browser.git for adding the new directory into the moz build system, and tor-browser-builder won't build tor-launcher independently anymore. This ticket is still relevant for merging build config into tor-launcher.git.

I found Mozilla introduced some breaking changes over the last few releases [0][1], so I patched those in TL, as well. Currently system add-ons do not work on android, so I opened a ticket (with a patch) for that[2].

The current tor-launcher patch branch is enhancement25260 [3]

It creates a features extension. The trickiest part of this desktop and android expect the features/ directory in different locations. On desktop, is it browser/features/ but on mobile it's only features/. I created a build-time variable that defines in which directory the extension should be stored, then I set DIST_SUBDIR as the correct location.

[0] https://bugzilla.mozilla.org/show_bug.cgi?id=1149830 [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1374847 [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1440789 [3] https://github.com/sysrqb/tor-launcher/tree/enhancement25260

sysrqb, I see that the tor-launcher is still using general.useragent.locale and intl.locale.matchOS. Looks like a new XPCOM method was added LocaleService::GetRequestedLocales[1] to replace them.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1346616

These patches look good to me in general (though I haven't tested them yet). I'm wondering if it's necessary to add moz.build file, or if we could still build with the traditional makefile inside torlauncher.git, and then just copy the .xpi to the appropriate place when we bundle everything together in tor-browser-build.git?

Keeping the torlauncher and torbutton extension builds independent of the Mozilla build system (if possible) seems like it will give us more flexibility. For example, I sometimes hack on torbutton.git without having a clone of mozilla-central present. I just copy the XPI to an existing Tor Browser build. And I occasionally post the .XPI for other people to test.

Replying to igt0:

sysrqb, I see that the tor-launcher is still using general.useragent.locale and intl.locale.matchOS. Looks like a new XPCOM method was added LocaleService::GetRequestedLocales[1] to replace them.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1346616

Oh, yes, good find. I'll correct this.

Replying to arthuredelstein:

These patches look good to me in general (though I haven't tested them yet). I'm wondering if it's necessary to add moz.build file, or if we could still build with the traditional makefile inside torlauncher.git, and then just copy the .xpi to the appropriate place when we bundle everything together in tor-browser-build.git?

Unfortunately there are two reasons integrating this into the moz build process is useful. The first is because now at packaging-time a manifest file is created containing metadata about all system add-ons. If we package the xpi separately and copy it into the bundle then rbm will need to modify the json file. The second, which is similar to the first, is that when we add Android packaging into this build system we'll have additional jar manifest files which contain file hashes, so we'll need to add/update a few files there, too - so that'll be an additional pain.

Keeping the torlauncher and torbutton extension builds independent of the Mozilla build system (if possible) seems like it will give us more flexibility. For example, I sometimes hack on torbutton.git without having a clone of mozilla-central present. I just copy the XPI to an existing Tor Browser build. And I occasionally post the .XPI for other people to test.

This shouldn't have any impact on being able to drop in new versions. The system add-on manifest file doesn't prevent modifying the add-on, it simply restricts which system add-ons are loaded at start-up (there aren't any (cryptographic) hashes associated with it). So, we can build everything together and you can update tor-button and tor-launcher XPIs as needed afterward. I'm specifically looking for a seamless way of integrating these extensions. This probably only buys us 6-12 months of legacy support before we need another solution.

Replying to sysrqb:

Replying to igt0:

sysrqb, I see that the tor-launcher is still using general.useragent.locale and intl.locale.matchOS. Looks like a new XPCOM method was added LocaleService::GetRequestedLocales[1] to replace them.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1346616

Oh, yes, good find. I'll correct this.

I looked into this a bit more. We're not actually providing dynamic locale selection (#21920 (moved)), so I don't think this is a priority at this moment and we don't need to worry about it in esr60. We should adjust this if/when we begin supporting this (#17400 (moved)).

For future reference: https://groups.google.com/forum/#!msg/mozilla.dev.platform/DAxXcr1rh2c/KUUwoPM2AwAJ https://groups.google.com/forum/#!topic/mozilla.dev.platform/0FnHBfOPKdk https://hg.mozilla.org/mozilla-central/file/tip/intl/locale/mozILocaleService.idl#l19

That's not needed anymore. We'll go with Orbot for now which should be fine.

Trac:
Resolution: N/A to wontfix
Status: new to closed

closed

mentioned in issue #25750 (moved)

mentioned in issue #28044 (moved)

moved to tpo/applications/tor-launcher#25260 (closed)

Merge mozbuild files into tor-launcher.git

Child items ...

Activity