Adapt tor-browser-build firefox and tor-browser project for ESR 60 Windows build

We need to do a bunch of changes to the firefox and tor-browser projects in tor-browser-build to get the ESR60-based Windows builds going.

added TorBrowserTeam201806R component::applications/tor browser ff60-esr owner::tbb-team priority::very high resolution::fixed severity::normal status::closed type::task labels

CCing the real sukhe

Trac:
Cc: sukhe to sukhbir

Trac:
Cc: sukhbir to sukhbir, arthuredelstein

Moving our tickets to June 2018

Trac:
Keywords: TorBrowserTeam201805 deleted, TorBrowserTeam201806 added

For review:

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

This should address #26205 (moved), #26206 (moved), #25837 (moved), #26326 (moved), #26204 (moved).

Some observations:

1. We have a working build of Windows x86_64 with this branch. I verified it on Windows 10.

2. I think we can do better for #26205 (moved) (commit 93f89a735792e329bd7853bcbd0db056426e32d6). Please review and suggest. Also, should this patch go in Firefox (as part of git_url) or as separate patches which we add during the build (current approach)?

3. For fxc2, we are pointing to my fork of the project. We will switch to mozilla/fxc2 when https://github.com/mozilla/fxc2/pull/1 is merged.

4. For Rust, I made the change below; let me know if we should remove the guards altogether:

-[% IF ! c("var/windows") %]
+[% IF ! c("var/windows-i686") %]

5. For Firefox, I based gecko-dev/tree/win-ff60 (commit be179ca1a0c7812a332b16a4f1b87eb68c629d01) on tor-browser-60.0.1esr-8.0-1 and tjr's patches, including the ones from https://treeherder.mozilla.org/#/jobs?repo=try&revision=781b545807dfa9d3de23054bf413ec232a9ce1c1 (Sandbox).

Trac:
Status: new to needs_review

Trac:
Keywords: TorBrowserTeam201806 deleted, TorBrowserTeam201806R added
Cc: sukhbir, arthuredelstein to sukhbir, arthuredelstein, boklm

Replying to sukhbir:

For review:

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

Thanks. I will review, and suggest some fixup commits if needed.

2. I think we can do better for #26205 (moved) (commit 93f89a735792e329bd7853bcbd0db056426e32d6). Please review and suggest. Also, should this patch go in Firefox (as part of git_url) or as separate patches which we add during the build (current approach)?

I think the patch should go in tor-browser.git (but adding it as a patch file is useful for testing before it is merged in tor-browser.git).

4. For Rust, I made the change below; let me know if we should remove the guards altogether:

{{{ -[% IF ! c("var/windows") %] +[% IF ! c("var/windows-i686") %] }}}

I think we can remove the guards altogether, as we will want it at some point, and the build fails without rust.

We actually need libwinpthread-1.dll in Browser\TorBrowser\Tor it seems and not in Browser. Which means we need it for the expert bundle as well.

Replying to gk:

We actually need libwinpthread-1.dll in Browser\TorBrowser\Tor it seems and not in Browser. Which means we need it for the expert bundle as well. This is #26206 (moved). And you need to ship it with every executable you built with your patched gcc. But that's not great, and you don't do that for msvcr100.dll, fwiw.

sukhbir: Why do we need pre_pkginst: dpkg --add-architecture i386 for the 64bit case? Compiling without it works for me.

Replying to gk:

sukhbir: Why do we need pre_pkginst: dpkg --add-architecture i386 for the 64bit case? Compiling without it works for me.

Without adding it, I was getting something like it looks like multiarch needs to be enabled. Also, I based this on https://wiki.debian.org/Wine, which says:

On 64-bit systems you should enable a 32-bit architecture for multiarch. This is needed for running 32-bit Windows applications (many modern apps are still 32-bit), but also for large parts of the Windows subsystem itself. If in doubt, you do need it!

But maybe Wine64 is all we need if it works without that.

Replying to sukhbir:

Replying to gk:

sukhbir: Why do we need pre_pkginst: dpkg --add-architecture i386 for the 64bit case? Compiling without it works for me.

Without adding it, I was getting something like it looks like multiarch needs to be enabled. Also, I based this on https://wiki.debian.org/Wine, which says:

{{{ On 64-bit systems you should enable a 32-bit architecture for multiarch. This is needed for running 32-bit Windows applications (many modern apps are still 32-bit), but also for large parts of the Windows subsystem itself. If in doubt, you do need it! }}}

But maybe Wine64 is all we need if it works without that.

Isn't that the error that you were having when you were trying to use the 32bit d3dcompiler_47.dll? When using the 64bit version of d3dcompiler_47.dll I don't think we need the 32bit version of wine.

Replying to boklm:

Replying to sukhbir:

Replying to gk:

sukhbir: Why do we need pre_pkginst: dpkg --add-architecture i386 for the 64bit case? Compiling without it works for me.

Without adding it, I was getting something like it looks like multiarch needs to be enabled. Also, I based this on https://wiki.debian.org/Wine, which says:

{{{ On 64-bit systems you should enable a 32-bit architecture for multiarch. This is needed for running 32-bit Windows applications (many modern apps are still 32-bit), but also for large parts of the Windows subsystem itself. If in doubt, you do need it! }}}

But maybe Wine64 is all we need if it works without that.

Isn't that the error that you were having when you were trying to use the 32bit d3dcompiler_47.dll? When using the 64bit version of d3dcompiler_47.dll I don't think we need the 32bit version of wine.

OK, if that's the case and it does compile without it, we should remove it. Should I update the patch?

Replying to sukhbir:

OK, if that's the case and it does compile without it, we should remove it. Should I update the patch?

I posted a new version of the patch, including this change and a few other changes in #25837 (moved) for review.

I looked over https://github.com/azadi/gecko-dev and all the cherry-picked patches. I omitted the following patches when pushing the changes to tor-browser-60.0.1esr-8.0-1:

1411401: That's only for Mozilla infra and we are essentially getting this patch by using mingw-w64's ee9fc3d0b8c8868280e38384edd968067b8e71fc. 1434316: That's only for Mozilla infra. 1460620: Let's leave this out for now and bundle the dll ourselves (as your patch is doing) 1461421: just cherry-pick the actual fix (the other two commits are stricly speaking not needed and I want to keep the patch number as small as possible)

I pushed an updated version of the tor-browser-build patches in branch bug_26203_v6: https://gitweb.torproject.org/user/boklm/tor-browser-build.git/log/?h=bug_26203_v6

We are done here, thanks all!

Trac:
Status: needs_review to closed
Resolution: N/A to fixed

closed

moved to tpo/applications/tor-browser#26203 (closed)