Closed Bug 1287881 Opened 5 years ago Closed 5 years ago

add mach repackage so windows builds can be signed as separate step from build with move to taskcluster


(Firefox Build System :: General, defect)

Not set


(firefox50 affected, firefox54 fixed)

Tracking Status
firefox50 --- affected
firefox54 --- fixed


(Reporter: kmoir, Assigned: grenade)




(5 files, 2 obsolete files)

From bug 1277591 re creating a windows signing worker (aki's notes)

To be explicit:
* `mach repackage`, or something like it, will be needed to do windows signing as a separate step from builds.
** unsigned windows packages need to be exploded, all the internal binaries signed separately, then repackaged, and the external package also signed.
* `mach repackage` is not written yet
* I'm not certain who is assigned the task of writing `mach repackage`.  Its absence blocks this bug.  In my mind, the actual windows signingscript + scriptworker implementations/deployment are relatively easy once linux signing scriptworker is deployed.  The main difference is `mach repackage`.
Blocks: 1277591
Raw notes:

10:12 <ted> is still the starting point
10:12 <ted> but it calls into to do a lot of the work nowadays

make-package -> make-package-internal
make-package-internal: prepare-package make-sourcestamp-file make-buildinfo-file make-mozinfo-file        @echo 'Compressing...'
        cd $(DIST) && $(MAKE_PACKAGE)


(The above directory contains a packager directory, which contains an, so mozbuild does seem to have l10n support. )
Related: bug 922241 and bug 576098.

I think the ideal workflow might be something like

windows build (installer, stub installer?)
  |--> tests that test installer
  |--> installer + stub installer signing, with repackage

in some order.  It's easy to debate whether tests should test the nightly signed bits, or test unsigned bits, or test CI/test-signed bits and re-sign with the nightly key once tests pass, and then trigger more tests around the new signature.  The key differences in the above model are a) signing happens outside the build process, b) we drop the zipfile and test the installers.

However, since switching all the tests to use the installers is a non-trivial task, and dropping the zipfile is a non-trivial social change with plenty of room for bikeshedding, I'm proposing we continue forward here with all 3 packages, and deal with dropping the zipfile as a non-blocking external task.
from #mozbuild
3:50 PM <aki> mshal: any 'mach repackage' progress or issues? we were talking about that in today's #tcmigration mtg, which unsurfaced and . i'm leaning towards dealing with those after our nightly tier 1 deadline
3:52 PM — aki adds comment to bug 1287881
4:36 PM <mshal> aki: gah, thanks for reminding me. So it turns out nobody in the build team really knows about the installer repackaging we do for l10n, so I have a little further digging to do. Too bad the easier package type is the one going away :)
4:37 PM <aki> :) ok, thanks for digging!
4:42 PM <aki> if we put off those two bugs, we'll likely need to sign the zipfile, but that should hopefully be relatively easy
4:43 PM <aki> easier than switching tests over to the installer and getting rid of the zipfile, at least :)
4:45 PM <mshal> yeah, signing the zip file looks super easy
Hi Mike 

Coop asked me to scope out the work that would be required for mach repackage for q4 (after we get our nightly deliverables out of the way this quarter)

Is there an update on this issue.  I note from one of the tcmigration meetings notes that you created a python script for signing, is this attached to a bug somewhere? From meeting notes:

"mach repackage (windows signing) - mshal came back with a small python script that uses 7zip directly on the exe.  this isn't in-tree and isn't verified good compared to the standard packaging process."

Are there notes on how the make files need to be updated as well?
Flags: needinfo?(mshal)
Attached file
I'm attaching the test script for reference, but I don't think we should be creating a new standalone script like this without de-duplicating some of the packaging logic. By this I mean I don't want to clean this script up, check it in, and only use it for 'mach repackage', since then we'll have 5 or so different ways of creating Windows installers.

Note that is really responsible for moving binaries & libraries into place, setting up omni.ja files, etc, but it doesn't know how to actually build a package. That logic is in the Makefiles, mostly as INNER_MAKE_PACKAGE (which is part of MAKE_PACKAGE). The rough workflow looks like this:

[source tree] -> 'mach build' -> [build products] -> '' -> [staged package] -> '$(MAKE_PACKAGE) from the Makefile' ->

For getting signing in Taskcluster, my understanding is that you want to go from back to the staged package, sign things, and then create a new (and/or Windows installer, etc).

IMO we should continue moving the INNER_MAKE_PACKAGE / INNER_UNMAKE_PACKAGE variables from into python files. Bug 1190522 is an example of this for the DMG package type (though it only did INNER_MAKE_PACKAGE - I think the UNMAKE should be in there as well so we can pack and unpack using just the python scripts). I'd guess a reasonable approach is to continue to add new <package-type>.py files in python/mozbuild/mozpack/

The Windows installer can be its own package type, which I think should be responsible for doing the 7zSD.sfx header and perhaps even setup.exe. Currently the logic for how to 7zip a Windows installer is repeated in a few places:


This presents a problem when changing the zip parameters or package format - for example, bug 879467 tuned the compression parameters, but partner repacks still use the old parameters! So moving this logic into a, and using it from all of these locations means we'll have a single source of truth for unpacking and re-packing a Windows installer (and ideally all other package types as well).

With those python scripts in place, then I think it becomes pretty straightforward to add a hook in mach to do 'unpackage; resign; repackage' for any package type, including the installer.

Let me know if you have further questions, or if you want to chat about how to tackle this.
Flags: needinfo?(mshal)

Thanks for all the information, very helpful!  

Is this in a private repo, I can't see it

When you say 

IMO we should continue moving the INNER_MAKE_PACKAGE / INNER_UNMAKE_PACKAGE variables from into python files. 

this refers to windows only, correct?
Flags: needinfo?(mshal)
(In reply to Kim Moir [:kmoir] from comment #6)
> Is this in a private repo, I can't see it
> sh#L16

Doh, apparently it is. I think I asked rail to get access before?

> When you say 
> IMO we should continue moving the INNER_MAKE_PACKAGE / INNER_UNMAKE_PACKAGE
> variables from into python files. 
> this refers to windows only, correct?

I think we'll eventually want to move them all, but if the initial purpose of this bug is for the Windows installer only, then we should just do that to start with.
Flags: needinfo?(mshal)
coop, can I get access to ? for research for this bug?
Flags: needinfo?(coop)
(In reply to Kim Moir [:kmoir] from comment #8)
> coop, can I get access to
> ? for research for this
> bug?

I granted you read access. Please let me know if you need r/w.
Flags: needinfo?(coop)
Depends on: 1305105
10:25 <catlee> one thing to consider with repackage / signing, is that it's very important that mar files and installers end up with exactly the same binaries at the end
10:26 <catlee> signing introduces time-dependent data into the files
10:26 <catlee> so we need to share the binaries between installers and mars
10:27 <catlee> ideally signed binaries can be shared across locales as well where appropriate
Kim: any update on progress here?

See also bug 1318505 about Mac signing which may have some design implications here.
Flags: needinfo?(kmoir)
still iterating on patches
Flags: needinfo?(kmoir)
So I have been testing my patches on my loaner but have found that the python pefile library (new requirement) is not available in the virtualenv when I run mach package.  My investigations lead me to think that this needs to be included in and mozillabuild needs to be rebuilt so I can test it.  RyanVM - do you have any pointers on how to add a new python package to it? I notice you have made most of the recent commits to this repo.

I couldn't find much other than

and the packaging scripts in the repo
Flags: needinfo?(ryanvm)
MozillaBuild is mostly dead (in that it's being deprecated by msys2 slowly but surely). It seems to me that the right path forward here would be to run |pip install pefile| from the MozillaBuild environment at some point during AMI generation.
Flags: needinfo?(ryanvm)
:grenade:  Do you have any guidance on how to |pip install pefile| from the MozillaBuild environment at some point during AMI generation?  I looked at the scripts for the b-2008 builders but didn't see any examples on adding python packages.  And to be honest, I haven't created windows amis before.
Flags: needinfo?(rthijssen)
here's an example of how we do it for *buildbot* windows slave amis:

you can pip install packages into *taskcluster* windows worker types during ami creation with a json block like this in the manifest for each worker type that needs it (maybe just gecko-3-b-win2012 if it's just for signing?):
for that, just submit a pr on the occ repo. once it's merged the amis will auto rebuild and it's available within an hour or two. however, that just installs the package on the system. i don't think it will be available to your venv.

if you're talking about installing a package into a venv during execution of a mach command within automation, you might not want the ami/system installation route. you might instead need something like this in-tree:
for this route, you'd also need to copy the package onto relengwebadmin (eg: `scp root@relengwebadm:/mnt/netapp/relengweb/pypi/pub/`) so that it's available from since taskcluster windows builds will only install packages from the mozilla public pypi repo. it looks like you can grab the zip file for pefile here:
Flags: needinfo?(rthijssen)
So I added the pefile and it is loaded into the venv when running mach package.  The problem now is that imports past.builtins always fails when running mach package

I have tried many different combinations of adding this library so it is added to the venv but it doesn't work yet. Any suggestions?
Flags: needinfo?(mshal)
talked to catlee and figured out the issue
Flags: needinfo?(mshal)
Attached patch packager3.patchSplinter Review
patches currently in progress (don't currently work but iterating through errors)

-added python future and pefile to
And virtualenv_packages.txt
Added libraries in tree
This is called by toolkit/mozapps/installer/ instead of the zip commands directly calls python/mozbuild/mozpack/ which creates headers on the zips using pefile
Had a meeting with mshal today

3:06 PM SFX_MODULE -> upx -> 7zSD.sfx
3:06 PM
3:06 PM 7zSD.sfx + package.7z -> package.exe
3:11 PM
3:11 PM
3:11 PM $(MAKE) setup.exe

He suggested that I shouldn't land the pefile libs in tree but rather leave use the existing 7zip utilities to unzip the file, extract the 7zip file, leave the old headers, resign then zip up to and create a new header. (This should be in my new python script that replaces the existing calls to the zip commands in  He's going to investigate how if this can be used to replace existing calls to etc
So I still think we'll want two new python files alongside ./python/mozbuild/mozbuild/action/, maybe (or and The pefile package was used in a standalone script as a way to pull out the sfx header, since it was assumed we wouldn't have access to the one in the tree. However, since we're doing this in-tree, we can just use the existing header and avoid the pefile package. To create the installer, I think it would look something like this:

# input can be '-r' to mean recursively zip up the current directory, or a filename (like setup-stub.exe)
# output is the exe, so for example $(PACKAGE) or whatever
# tagfile is either app.tag or stub.tag, which are files found in the m-c repo
make_installer(input, tagfile, output):
    upx --best -o [tmpdir]/7ZSD.sfx /path/to/srcdir/other-licenses/7zstub/firefox/7zSD.sfx
    7z a -t7z [tmpdir]/app.7z [input] -mx -m0=BCJ2 -m1=LZMA:d25 -m2=LZMA:d19 -m3=LZMA:d19 -mb0:1 -mb0s1:2 -mb0s2:3
    cat [tmpdir]/7ZSD.sfx [tagfile)] [tmpdir]/app.7z > [output.exe]
    chmod 0775 [output.exe]

    7z x [installer] core

The equivalent for the zip format is much simpler. The make variables for this currently look like:

    -x \*/.mkdir.done

So just zip with some flags, and unzip.

The first step I think is to write & test the scripts against some packages and installers, which can be done without any Makefile changes. Once those are working, then we can go in and update the Makefiles referenced in #c5 to de-duplicate the upx/7z/cat/chmod logic that we have in a bunch of places. I can help with the Makefile updates - the packaging code is not very straightforward here :/

One thing I'm not sure of yet is why the installer has a directory called "core", while the .zip package has a directory called "firefox" - both contain the same contents, and we have some steps in the Makefiles to move from firefox to core when handling the installer. I'm not sure if we actually need that, and if we do, if that logic should be inside the make_installer() / unmake_installer() calls.
Assignee: nobody → kmoir
Attached image IMG_2898.JPG
We are at a tc migration work week in the Toronto lab this week.  We discussed a new approach to packaging and signing the zip/dmg.  I've attached a photo of the whiteboard.  

Essentially the approach will be
3-4 task solution 
Build creates  unsigned tarball or zip, dmg or exe
Sign innards + signed zip and stub installer
Package task downloads signed innards
Sign package (windows only) signed exe and mar

There is also a list of tasks to complete in the trello board under the hard blockers heading
Attached patch packager6.patch (obsolete) — Splinter Review
wip patches because I'm switching focus to work on bug 1318505
i'm having some trouble getting the 7z archive command working. 7z is throwing an error which reads "The parameter is incorrect" but not sure which param it doesn't like. The error is from the try push at

14:03:07     INFO -    (cd firefox && z:/build/build/src/obj-firefox/_virtualenv/Scripts/python.exe z:/build/build/src/config/ && z:/build/build/src/obj-firefox/_virtualenv/Scripts/python.exe -m mozbuild.action.archive_exe 'firefox' 'app.tag' 'firefox-54.0a1.x-test.win32.exe'
14:03:07     INFO -                         Ultimate Packer for eXecutables
14:03:07     INFO -                            Copyright (C) 1996 - 2013
14:03:07     INFO -  UPX 3.91w       Markus Oberhumer, Laszlo Molnar & John Reiser   Sep 30th 2013
14:03:07     INFO -          File size         Ratio      Format      Name
14:03:07     INFO -     --------------------   ------   -----------   -----------
14:03:07     INFO -      123904 ->     71680   57.85%    win32/pe     7zSD.sfx
14:03:07     INFO -  Packed 1 file.
14:03:07     INFO -  7-Zip [32] 15.14 : Copyright (c) 1999-2015 Igor Pavlov : 2015-12-31
14:03:07     INFO -  Scanning the drive:
14:03:07     INFO -  11 folders, 107 files, 94148869 bytes (90 MiB)
14:03:07     INFO -  Creating archive: c:\users\task_1487683920\appdata\local\temp\tmpz1m4im\app.7z
14:03:07     INFO -  Items to compress: 118
14:03:07     INFO -  System ERROR:
14:03:07     INFO -  The parameter is incorrect.
14:03:07     INFO -  {'tagfile': 'app.tag', 'contents': 'firefox', 'package': 'firefox-54.0a1.x-test.win32.exe'}
14:03:07     INFO -  ['upx', '--best', '-o', 'c:\\users\\task_1487683920\\appdata\\local\\temp\\tmpz1m4im\\7zSD.sfx', '..\\..\\..\\other-licenses\\7zstub\\firefox\\7zSD.sfx']
14:03:07     INFO -  ['7z', 'a', '-r', '-t7z', 'c:\\users\\task_1487683920\\appdata\\local\\temp\\tmpz1m4im\\app.7z', 'firefox', '-mx', '-m0=BCJ2', '-m1=LZMA:d25', '-m2=LZMA:d19', ' -m3=LZMA:d19', '-mb0:1', '-mbs1:2', '-mb0s2:3']
14:03:07     INFO -  Traceback (most recent call last):
14:03:07     INFO -    File "c:\mozilla-build\python\Lib\", line 162, in _run_module_as_main
14:03:07     INFO -      "__main__", fname, loader, pkg_name)
14:03:07     INFO -    File "c:\mozilla-build\python\Lib\", line 72, in _run_code
14:03:07     INFO -      exec code in run_globals
14:03:07     INFO -    File "z:\build\build\src\python\mozbuild\mozbuild\action\", line 35, in <module>
14:03:07     INFO -      sys.exit(main(sys.argv[1:]))
14:03:07     INFO -    File "z:\build\build\src\python\mozbuild\mozbuild\action\", line 31, in main
14:03:07     INFO -      archive_exe(args[0], args[1], args[2])
14:03:07     INFO -    File "z:\build\build\src\python\mozbuild\mozbuild\action\", line 22, in archive_exe
14:03:07     INFO -      subprocess.check_call(command)
14:03:07     INFO -    File "c:\mozilla-build\python\Lib\", line 540, in check_call
14:03:07     INFO -      raise CalledProcessError(retcode, cmd)
14:03:07     INFO -  subprocess.CalledProcessError: Command '['7z', 'a', '-r', '-t7z', 'c:\\users\\task_1487683920\\appdata\\local\\temp\\tmpz1m4im\\app.7z', 'firefox', '-mx', '-m0=BCJ2', '-m1=LZMA:d25', '-m2=LZMA:d19', ' -m3=LZMA:d19', '-mb0:1', '-mbs1:2', '-mb0s2:3']' returned non-zero exit status 2
14:03:07     INFO -  z:/build/build/src/toolkit/locales/ recipe for target 'repackage-zip' failed
14:03:07     INFO -  mozmake.EXE[8]: *** [repackage-zip] Error 1
14:03:07     INFO -  mozmake.EXE[8]: Leaving directory 'z:/build/build/src/obj-firefox/browser/locales'
14:03:07     INFO -  Makefile:124: recipe for target 'repackage-win32-installer' failed
14:03:07     INFO -  mozmake.EXE[7]: *** [repackage-win32-installer] Error 2
14:03:07     INFO -  Makefile:137: recipe for target 'repackage-win32-installer-x-test' failed
14:03:07     INFO -  mozmake.EXE[6]: *** [repackage-win32-installer-x-test] Error 2
after a bit of trial and error the 7zip archive command is working (see last few try pushes). newest problem seems to be with the cat command trying to parse the output redirect bracket as a filename ???


:mshal: do you have any ideas about how i can get that last call to `cat` working?
Flags: needinfo?(mshal)
If you pass a list to `subprocess.Popen` it's going to pass that directly to CreateProcess or execv and bypass the shell. If you really want to use shell redirection like `cat x > y` you need to pass the whole thing as a string, which isn't great. It'd be better to just write Python code to do what those shell commands are doing. You can replace those calls to `mv` with `shutil.move`, and for the call to `cat`, which is just concatenating a few files you could do something like:
with open(package, 'wb') as o:
  for i in [os.path.join(...), ...]:
    shutil.copyfileobj(open(i, 'rb'), o)
cheers ted. i may have been running a little blinkered there... ;)
Flags: needinfo?(mshal)
Attached patch repackage (obsolete) — Splinter Review
hey kmoir, mshal, fwiw i've been hacking on the earlier patch here and cobbled this together. i don't know which tagfile should be used (app.tag or stub.tag) in the call from to 7z_exe_archive. is this still useful or does the other mechanism of multiple tc tasks remove the need for it?
Attachment #8842943 - Flags: feedback?(mshal)
Attachment #8842943 - Flags: feedback?(kmoir)
Comment on attachment 8842943 [details] [diff] [review]

This is looking pretty nice!

I'm pretty sure that the INNER_MAKE_PACKAGE call for the SFX7Z format is only used when doing l10n repacks, which only operate on the full installer and not the stub installer. So app.tag is the correct thing to use there.

The reason we want to have the tag passed in as a variable is so we can do a similar $(call py_action,7z_exe_archive) call when creating the stub installer, and there we will pass in stub.tag:

I didn't do a full review yet, but I noticed the os.path.join calls everywhere. Do things break if you just do '../../../other-licenses/7zstub/firefox/7zSD.sfx' instead of the big os.path.join call? Also for the cases where you do need a join because of a variable, I'd recommend using our internal mozpath instead for consistency with the rest of our build system code (which uses forward slashes for everything). Eg:

import mozpack.path as mozpath
mozpath.join(tmpdir, '7zSD.sfx')
Attachment #8842943 - Flags: feedback?(mshal) → feedback+
Comment on attachment 8842943 [details] [diff] [review]

I'll let mshal provide feedback so he is more familiar with this than I
Attachment #8842943 - Flags: feedback?(kmoir)
Comment on attachment 8843273 [details]
bug 1287881 - windows repackage refactored as actions;

This looks great - thanks for working on it! Just a few nits and then it should be ready to land.

::: python/mozbuild/mozbuild/action/
(Diff revision 1)
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at
> +
> +from __future__ import print_function
> +
> +import os

nit: os is unused

::: python/mozbuild/mozbuild/action/
(Diff revision 1)
> +
> +import os
> +import shutil
> +import sys
> +import subprocess
> +import tempfile

nit: tempfile is unused

::: toolkit/mozapps/installer/
(Diff revision 1)
>  endif
>  ifeq ($(MOZ_PKG_FORMAT),SFX7Z)
>    PKG_SUFFIX	= .exe
> -  INNER_MAKE_PACKAGE	= rm -f app.7z && \
> +  _ABS_MOZSRCDIR = $(shell cd $(MOZILLA_DIR) && pwd)

What is _ABS_MOZSRCDIR for? It looks like it might've been copied from the dmg case, but I don't think it does anything here.
Attachment #8843273 - Flags: review?(mshal) → review+
Attachment #8842943 - Attachment is obsolete: true
Attachment #8836808 - Attachment is obsolete: true
Pushed by
windows repackage refactored as actions; r=mshal
Keywords: checkin-needed
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Depends on: 1345422
I didn't fix this - so assigning to Rob.  Thanks Rob for driving this bug, so happy to see this resolved.
Assignee: kmoir → rthijssen
Blocks: 1360525
Depends on: 1362611
Component: mach → Build Config
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.