Opened 6 months ago

Closed 6 months ago

Last modified 6 months ago

#26319 closed defect (fixed)

Don't package up the whole Tor Browser in the `mach package` step

Reported by: gk Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: ff60-esr, TorBrowserTeam201806R, tbb-rbm
Cc: sukhbir Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

When running mach package not only the symlinks are resolved and the omni.ja files compressed but it's attempted to create a final package, be it e.g. a .dmg file for macOS or an .exe file for Windows. We don't want that in the Firefox build step because our bundle is not ready then: we need to add tor, the pluggable transports etc. first and bundle the whole thing up when that's ready.

We achieved this previously by setting INNER_MAKE_PACKAGE=true when running the packaging step but that does not seem to be doable anymore with the switch to mach.

We should write a patch, if necessary, to get the old behavior back. Meanwhile, for macOS, we ship a workaround (see #24632) as the build breaks without it. Windows might be affected here, too (and even on Linux a final .bz2 file is created which is not necessary for our purposes).

Child Tickets

Change History (12)

comment:1 Changed 6 months ago by sukhbir

Cc: sukhbir added

comment:2 Changed 6 months ago by sukhbir

For review:

https://github.com/azadi/tor-browser-build-1/commits/bug-26319

From the commit message:

Bug 26319: Don't package up the whole Tor Browser in the `mach package` step

This commit sets `mach build stage-package` instead of `mach package` in
the Firefox build; this helps us to use the useful properties of `mach
package` but avoids creating the final DMG or EXE during the Firefox
build which we don't want as we do that later when building Tor Browser.

We no longer need to patch `installer.py` (bug 26205) but we still need
the patch for the uninstaller otherwise the Firefox build fails trying
to find the required NSIS files.

comment:3 Changed 6 months ago by sukhbir

Status: newneeds_review

comment:4 Changed 6 months ago by gk

Keywords: TorBrowserTeam201806R added; TorBrowserTeam201806 removed

comment:5 Changed 6 months ago by gk

Keywords: TorBrowserTeam201806 added; TorBrowserTeam201806R removed
Status: needs_reviewneeds_revision

It seems we do not need my workaround for the .dmg case either anymore? If so, could you get rid of that in the patch for this bug as well?

comment:6 Changed 6 months ago by sukhbir

For review:

https://github.com/azadi/tor-browser-build-1/tree/bug-26319-rev1

I removed the no-dmg.patch and tested with a macOS build. Everything looks fine.

comment:7 Changed 6 months ago by sukhbir

Status: needs_revisionneeds_review

comment:8 Changed 6 months ago by gk

Keywords: TorBrowserTeam201806R added; TorBrowserTeam201806 removed

comment:9 in reply to:  6 Changed 6 months ago by gk

Keywords: TorBrowserTeam201806 added; TorBrowserTeam201806R removed
Status: needs_reviewneeds_revision

Replying to sukhbir:

For review:

https://github.com/azadi/tor-browser-build-1/tree/bug-26319-rev1

I removed the no-dmg.patch and tested with a macOS build. Everything looks fine.

It seems the no-dmg.patch is not deleted in your patch. Could you fix that?

comment:10 Changed 6 months ago by sukhbir

Status: needs_revisionneeds_review

comment:11 Changed 6 months ago by gk

Keywords: TorBrowserTeam201806R added; TorBrowserTeam201806 removed
Resolution: fixed
Status: needs_reviewclosed

Looks good and merged to master (commit 8d1c4c396034b2ab0b7c55982e1900236f5031a6). Could you file a follow-up ticket for the remaining NSIS related patch? We should find a proper workaround and/or an upstreamable patch and move either of them into tor-browser to not lose track of it for upstreaming purposes.

comment:12 Changed 6 months ago by sukhbir

For reference, #26537 is the bug for the uninstaller patch.

Note: See TracTickets for help on using tickets.