Fix nsis builds for Windows 64
We need to do Windows 64 builds of nsis.
Activity
I tried a build with the following patch:
diff --git a/projects/nsis/build b/projects/nsis/build index af19dd7..c75dced 100755 --- a/projects/nsis/build +++ b/projects/nsis/build @@ -16,7 +16,7 @@ do [ -f debian/patches/$patch ] && patch -p1 < debian/patches/$patch done [% SET scons_args = 'APPEND_CCFLAGS="-fgnu89-inline" VERSION=' _ c("version") - _ " SKIPUTILS='NSIS Menu' XGCC_W32_PREFIX=i686-w64-mingw32-" + _ " SKIPUTILS='NSIS Menu' XGCC_W32_PREFIX=" _ c("arch") _ "-w64-mingw32-" _ ' PREFIX=/var/tmp/dist/nsis' -%] scons [% scons_args %] scons [% scons_args %] installWhich failed with the following error:
x86_64-w64-mingw32-g++ -s -mwindows -Wl,--file-alignment,512 -Wl,-Map,build/release/Banner/Banner.map -static-libgcc -static-libstdc++ -Wl,-e_DllMain@12 -nostdlib -Wl,-- exclude-libs,msvcrt.a -shared -o build/release/Banner/Banner.dll build/release/Banner/Banner.o -Lbuild/release/api/nsis -lpluginapi -lkernel32 -luser32 -Wl,--out-implib, build/release/Banner/libBanner.a -Wl,--output-def,build/release/Banner/Banner.def /var/tmp/dist/mingw-w64/lib/gcc/x86_64-w64-mingw32/5.4.0/../../../../x86_64-w64-mingw32/bin/ld: warning: cannot find entry symbol _DllMain@12; defaulting to 000000006cf0 1000 x86_64-w64-mingw32-g++ -o build/release/BgImage/BgImage.o -c -fgnu89-inline -Os -Wall -fno-strict-aliasing -DNSISCALL='__attribute__((__stdcall__))' -Ibuild/release/api Contrib/BgImage/BgImage.cpp cc1plus: warning: command line option '-fgnu89-inline' is valid for C/ObjC but not for C++ Contrib/BgImage/BgImage.cpp: In function 'void SetBg(HWND, int, char*, stack_t**, extra_parameters*)': Contrib/BgImage/BgImage.cpp:134:21: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] (LPSTR)(DWORD)atomClass, ^ Contrib/BgImage/BgImage.cpp:152:49: error: 'GWL_WNDPROC' was not declared in this scope oldProc = (void *)SetWindowLong(hwndParent, GWL_WNDPROC, (long)WndProc); ^ Contrib/BgImage/BgImage.cpp:152:68: error: cast from 'LRESULT (*)(HWND, UINT, WPARAM, LPARAM) {aka long long int (*)(HWND__*, unsigned int, long long unsigned int, long long int)}' to 'long int' loses precision [-fpermissive] oldProc = (void *)SetWindowLong(hwndParent, GWL_WNDPROC, (long)WndProc); ^ Contrib/BgImage/BgImage.cpp: In function 'void AddText(HWND, int, char*, stack_t**, extra_parameters*)': Contrib/BgImage/BgImage.cpp:296:39: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] newImg->hFont = (HFONT)myatoi(szTemp); ^ Contrib/BgImage/BgImage.cpp: In function 'void Destroy(HWND, int, char*, stack_t**, extra_parameters*)': Contrib/BgImage/BgImage.cpp:348:31: error: 'GWL_WNDPROC' was not declared in this scope SetWindowLong(hwndParent, GWL_WNDPROC, (long)oldProc); ^ Contrib/BgImage/BgImage.cpp:348:50: error: cast from 'void*' to 'long int' loses precision [-fpermissive] SetWindowLong(hwndParent, GWL_WNDPROC, (long)oldProc); ^ Contrib/BgImage/BgImage.cpp: In function 'LRESULT WndProc(HWND, UINT, WPARAM, LPARAM)': Contrib/BgImage/BgImage.cpp:394:5: error: invalid conversion from 'long int (*)(HWND, unsigned int, unsigned int, long int) {aka long int (*)(HWND__*, unsigned int, unsi gned int, long int)}' to 'WNDPROC {aka long long int (*)(HWND__*, unsigned int, long long unsigned int, long long int)}' [-fpermissive] ); ^ In file included from /var/tmp/dist/mingw-w64/x86_64-w64-mingw32/include/windows.h:72:0, from Contrib/BgImage/BgImage.cpp:1: /var/tmp/dist/mingw-w64/x86_64-w64-mingw32/include/winuser.h:2092:29: note: initializing argument 1 of 'LRESULT CallWindowProcA(WNDPROC, HWND, UINT, WPARAM, LPARAM)' WINUSERAPI LRESULT WINAPI CallWindowProcA (WNDPROC lpPrevWndFunc, HWND hWnd, UINT Msg, WPARAM wParam, LPARAM lParam); ^ scons: *** [build/release/BgImage/BgImage.o] Error 1 scons: building terminated because of errors.-
The branch
bug_23561in my git repo has a patch for review: https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_23561&id=03a5abef9e76f9e7464e0110f068b9e65e9a841dWith this patch we use a 32bit nsis for the 64bit bundle. This create a 32bit installer to install the 64bit bundle.
Trac:
Status: assigned to needs_review -
I pushed a more simple version of the patch in branch
bug_23561_v2: https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_23561_v2&id=2740676bf72c1bd41cbef098a1018a31eaf3be87The tor-browser output filename is the same in both branches, so the result should be the same:
./rbm/rbm showconf tor-browser filename --target torbrowser-windows-i686 --target alpha tor-browser-7.5a7-windows-i686-328442 -
Replying to boklm:
I pushed a more simple version of the patch in branch
bug_23561_v2: https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_23561_v2&id=2740676bf72c1bd41cbef098a1018a31eaf3be87Actually this patch doesn't work as expected. This
windows-x86_64: windows-i686line only apply inside projects/nsis, but not on the dependencies, such as the compiler, so it will be using the 64bit mingw-w64.So we should use the patch from
bug_23561. Sorry for the confusion.The tor-browser output filename is the same in both branches, so the result should be the same: {{{ ./rbm/rbm showconf tor-browser filename --target torbrowser-windows-i686 --target alpha tor-browser-7.5a7-windows-i686-328442 }}}
I used
--target torbrowser-windows-i686instead of--target torbrowser-windows-x86_64here. If I use--target torbrowser-windows-x86_64then I get a different tor-browser filename in the 2 branches. 
Alright, pushed the workaround to
master(commit 51e32622701c743727d9903c67e38579c1bc7b8b). I leave this ticket open and on our radar for the right fix, though.Trac:
Parent: #20636 (moved) to N/A
Status: needs_review to assigned
Owner: boklm to tbb-team
Keywords: TorBrowserTeam201711R deleted, TorBrowserTeam201711 added-
sukhbir: once you are done with the Windows bundles for esr60, this would be a worthwhile bug spending your time on as we still need to workaround it which is annoying, see: #26363 (moved).
For review:
https://github.com/azadi/tor-browser-build-1/commits/bug-23561
From the commit message:
Bug 23561: Fix NSIS builds for Windows 64 This commit adds support for building the 64-bit version of NSIS, and also bumps the version to 3.03. Doing this enables us to build MAR files in a 64-bit container for the 64-bit version of Tor Browser; see bug 26363 and bug 24477. The pe_checksum_fix.py doesn't work in a 64-bit container with the bundled python-pefile version so we build its latest version to fix this issue. This change is borrowed from commit bb32ec9 and updates python-pefile to 2017.11.5. The Debian package and the patches are no longer required as all changes were merged upstream in 3.01-1. (See the nsis changelog in Debian.)After building Tor Browser nightly 32- and 64-bit, we get the respective NSIS installers. (Tested on Windows 10.)
Trac:
Status: assigned to needs_review-
In
projects/nsis/buildthere is a line[% IF c("var/windows") %], but thisIFis always true as we only buildnsisfor Windows. So I think theIFcan be removed.Otherwise this commit looks good to me. I have started builds on 2 machines to see if I get the same result.
Trac:
Status: needs_review to needs_revision -
Replying to boklm:
In
projects/nsis/buildthere is a line[% IF c("var/windows") %], but thisIFis always true as we only buildnsisfor Windows. So I think theIFcan be removed.In
projects/nsis/configthe lineenable: '[% c("var/windows") %]'can also be removed. Replying to boklm:
Replying to boklm:
In
projects/nsis/buildthere is a line[% IF c("var/windows") %], but thisIFis always true as we only buildnsisfor Windows. So I think theIFcan be removed.In
projects/nsis/configthe lineenable: '[% c("var/windows") %]'can also be removed.Thanks, I removed the
c("var/windows")guard for zlib in both the build and the config files. (Same branch for review.)https://github.com/azadi/tor-browser-build-1/commits/bug-23561
Trac:
Status: needs_revision to needs_review-
One thing I wondered on IRC is how to avoid depending on another external party, files.pythonhosted.org, not being down once we build + about whether rebuilding
python-futureandpython-pefileevery time is a smart idea.I guess we could try at least for
python-futurethe 0.15.2 package from Debian. Which leaves the other one. FWIW: Do we have a policy for when we build own projects and when it is okay to build a project during another one? I.e.: should be buildpython-futureandpython-pefilein own projects to avoid building them over and over again (in case they indeed need to get built)? -
Replying to gk:
One thing I wondered on IRC is how to avoid depending on another external party, files.pythonhosted.org, not being down once we build + about whether rebuilding
python-futureandpython-pefileevery time is a smart idea.I think we could mirror those files on some reliable hosting. Related to that I opened #25835 (moved) as we have the same issue when people.tpo is down.
I guess we could try at least for
python-futurethe 0.15.2 package from Debian.It looks like a good idea to try that. I think this package didn't exist in the Ubuntu version we were using when we started doing this.
Which leaves the other one. FWIW: Do we have a policy for when we build own projects and when it is okay to build a project during another one? I.e.: should be build
python-futureandpython-pefilein own projects to avoid building them over and over again (in case they indeed need to get built)?I can think of the following reasons for wanting to move builds into a separate project (maybe there are more):
- the build of that project takes some time, and the other project in which we could include it is something we rebuild more often (such as firefox)
- we need to clone a git repository (we can have only one per project)
- moving it to a separate project makes things more simple or more clear
Running
time python setup.py install --userforpython-futureandpython-pefileon my laptop, I get this:future: real 0m0.811s user 0m0.535s sys 0m0.238s pefile: real 0m0.261s user 0m0.140s sys 0m0.110sBuilding them takes less than 2 seconds, so it doesn't seem worth moving them to a separate project.
-
Replying to gk:
I guess we could try at least for
python-futurethe 0.15.2 package from Debian.This fixup commit is using the
python-futurepackage fromjessie-backports: https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_23561_v4&id=8ecea4a935a8f03754234656c94b18ff3b5af568I have two matching x86_64 builds, and am waiting for i686 builds to finish to check if they match too.
-
While looking at the commit again, I noticed a small issue: the
pe_checksum_fix.pyfile is no longer included only in the Windows builds. This is fixed by this fixup commit: https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_23561_v4&id=e2a135f648cbd0ce3dbc366c62a431f42efdf2b8 -
Branch
bug_23561_v5has the commit with the two fixups: https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_23561_v5&id=0b884dc0c06b5467ef91e23ea0128c9e2c0169bdMy two builds for x86_64 and i686 are matching, so it seems the build is reproducible.
-
Branch
bug_23561_v6is based on the updated commit from comment 25: https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_23561_v6&id=951b1a7e01000f3dc1529c497b64d32cc0e58339 mentioned in issue #24477 (moved)
mentioned in issue #26363 (moved)
mentioned in issue tpo/applications/tor-browser#24477 (closed)