Opened 3 years ago

Closed 2 years ago

#25260 closed enhancement (wontfix)

Merge mozbuild files into tor-launcher.git

Reported by: sysrqb Owned by: brade
Priority: Medium Milestone:
Component: Applications/Tor Launcher Version:
Severity: Normal Keywords:
Cc: mcs, arthuredelstein, gk, sukhbir Actual Points:
Parent ID: #24856 Points:
Reviewer: Sponsor:

Description

Step 1 of using TorLauncher with TBA is merging TorLauncher into tor-browser. As a side effect of this, we avoid rewriting it using the Webextensions API which provides limited functionality compared to the legacy XPCOM interface.

I see (at least) three options:

  • Copy the current tor-launcher.git src/ directory code into tor-browser.git and maintain it in tor-browser
  • Copy the entire tor-launcher.git repo into tor-browser.git as a nested git repo (preserving the .git/ from tor-launcher), continue maintaining tor-launcher.git but update the version in tor-browser.git by git pull from tor-launcher.git whenever needed
  • Add tor-launcher.git as a git submodule

I'm currently favoring 2 or 3 because this means the other XPCOM/XUL apps can continue using it and we don't run into a problem with tor-launcher-in-tor-browser becoming out of sync with tor-launcher.git.

If we follow 2), then it adds an additional step whenever new code is merged into tor-launcher.git because then it will need to be pulled into tor-browser.git, too. One advantage of this is it simplifies the build process and tor-browser-build won't need a tor-launcher project. On the other hand, this means we'll need a new build flag for enabling/disabling including tor-launcher (otherwise tor-launcher will be included and run at startup by default, always).

If we follow 3), then tor-launcher remains a separate project that is cloned and integrated only when it is needed (much like it is currently at build time). This choice comes with the disadvantages of using a submodule, but it keeps the two repos decoupled.

From these three choices, it seems like using a submodule is the best choice, so we should merge the moz.build and jar.mn into tor-launcher.git.

Child Tickets

Change History (15)

comment:1 Changed 3 years ago by sysrqb

Cc: sukhbir added

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.

comment:2 Changed 3 years ago by sysrqb

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.

comment:3 Changed 3 years ago by 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?

comment:4 in reply to:  3 Changed 3 years ago by arthuredelstein

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.

comment:5 in reply to:  3 Changed 3 years ago by sysrqb

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

comment:6 Changed 3 years ago by 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, 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

comment:7 in reply to:  6 ; Changed 3 years ago by 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

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

comment:8 in reply to:  7 Changed 3 years ago by sysrqb

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.

comment:9 Changed 3 years ago by sysrqb

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

comment:10 Changed 3 years ago by 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

comment:11 Changed 3 years ago by 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?

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.

comment:12 in reply to:  10 ; Changed 2 years ago by 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.

comment:13 in reply to:  11 Changed 2 years ago by sysrqb

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.

comment:14 in reply to:  12 Changed 2 years ago by sysrqb

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

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

comment:15 Changed 2 years ago by gk

Resolution: wontfix
Status: newclosed

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

Note: See TracTickets for help on using tickets.