558800 - Allow partner-repack script to repack either signed or unsigned Windows builds

Status: RESOLVED FIXED

(Release Engineering :: Release Requests, defect, P2)

Description

[Chris Cooper [:coop] (he/him)]

As a stop-gap solution until bug 554321 is fixed, we should allow the partner-repack script to work with either signed or unsigned source builds. The process for handling already-signed builds is more elaborate, but is already codified in the catlee's script that kev has been using. We should fold this logic into the main script.

Comment 1

[Chris Cooper [:coop] (he/him)]

Attachment: Automate catlee's script, i.e. allow repack script to process already-signed builds

Not as big a patch as it seems, honest! Mostly I'm just putting catlee's script and associated parts under version control and there's a largish binary file (7zSD.sfx.upx) in there.

I've renamed catlee's script from repack.sh to repack-signed.sh for clarity and put the actual signing code in a conditional than only runs if the signcode tool is present. The script also renames the *.exe.repack files back to .exe at the end, which is something that kev had to do by hand before. 

The partner-repack.py script runs the repack-signed.sh script if the --signed command-line option is specified and win32 is in the list of platforms repacked.

Comment 2

[[e-mail] (:bhearsum)]

Comment on attachment 439568 [details] [diff] [review]
Automate catlee's script, i.e. allow repack script to process already-signed builds

Ideally, we would use the 7z stub and app.tag from the sourceRepo, but I see that that's not even passed in here, so probably out of scope for this bug....I also can't find this specific 7z stub anywhere.

The SIGNCODE part of the repack script should be dropped IMHO -- it doesn't make sense as part of the partner repack script. r=bhearsum with that change.

Comment 3

[Chris Cooper [:coop] (he/him)]

Attachment: Automate catlee's script, i.e. allow repack script to process already-signed builds, v2

(In reply to comment #2)
> (From update of attachment 439568 [details] [diff] [review])
> Ideally, we would use the 7z stub and app.tag from the sourceRepo, but I see
> that that's not even passed in here, so probably out of scope for this bug....I
> also can't find this specific 7z stub anywhere.
 
If I can grab the individual files from the source repo, we could do it that way. I don't really want to clone the whole repo if I don't have to though.

> The SIGNCODE part of the repack script should be dropped IMHO -- it doesn't
> make sense as part of the partner repack script. r=bhearsum with that change.

Yeah, we're never going to run it that way, so I just took that section out.

Comment 4

[[e-mail] (:bhearsum)]

(In reply to comment #3)
> Created an attachment (id=440497) [details]
> Automate catlee's script, i.e. allow repack script to process already-signed
> builds, v2
> 
> (In reply to comment #2)
> > (From update of attachment 439568 [details] [diff] [review] [details])
> > Ideally, we would use the 7z stub and app.tag from the sourceRepo, but I see
> > that that's not even passed in here, so probably out of scope for this bug....I
> > also can't find this specific 7z stub anywhere.
> 
> If I can grab the individual files from the source repo, we could do it that
> way. I don't really want to clone the whole repo if I don't have to though.

Yeah, I'd be talking about individual files. We do this for signing: http://hg.mozilla.org/build/tools/file/tip/release/signing/Makefile#l106

But if the stub file we need isn't in the repo, I'm not sure we can do it. If you want to give it a try, and see if those ones work feel free to, but I'm not going to hold back this bug for that.

> > The SIGNCODE part of the repack script should be dropped IMHO -- it doesn't
> > make sense as part of the partner repack script. r=bhearsum with that change.
> 
> Yeah, we're never going to run it that way, so I just took that section out.

Sounds good.

Comment 5

[Chris Cooper [:coop] (he/him)]

(In reply to comment #4)
> But if the stub file we need isn't in the repo, I'm not sure we can do it. If
> you want to give it a try, and see if those ones work feel free to, but I'm not
> going to hold back this bug for that.

The included stub file is simply a compressed version of other-licenses/7zstub/firefox/7zSD.sfx. That file is identical across branches AFAICT, as is app.tag. 

We'd need to install upx on the mac slaves to be able to compress the in-tree .sfx file. Is that a dependency we want to create/fill?

Comment 6

[[e-mail] (:bhearsum)]

(In reply to comment #5)
> (In reply to comment #4)
> > But if the stub file we need isn't in the repo, I'm not sure we can do it. If
> > you want to give it a try, and see if those ones work feel free to, but I'm not
> > going to hold back this bug for that.
> 
> The included stub file is simply a compressed version of
> other-licenses/7zstub/firefox/7zSD.sfx. That file is identical across branches
> AFAICT, as is app.tag. 
> 
> We'd need to install upx on the mac slaves to be able to compress the in-tree
> .sfx file. Is that a dependency we want to create/fill?

It seems like the right thing to do. It avoids complication down the road if/when the sfx gets updated. Again though, if you don't want to hold back this bug for that, that's fine.

Comment 7

[Chris Cooper [:coop] (he/him)]

Comment on attachment 440497 [details] [diff] [review]
Automate catlee's script, i.e. allow repack script to process already-signed builds, v2

(In reply to comment #6) 
> It seems like the right thing to do. It avoids complication down the road
> if/when the sfx gets updated. Again though, if you don't want to hold back this
> bug for that, that's fine.

OK, let's do this the right way then.

Comment 8

[Chris Cooper [:coop] (he/him)]

Attachment: Automate catlee's script, i.e. allow repack script to process already-signed builds, v3

The script now pulls the 7zSD.sfx and app.tag from the current RELEASE tag. If you don't specify a tag on the command line, the script tries to build one based on the (mandatory) version. This allows the script to work with the existing release automation step without changes.

We've already pushed out upx to the mac build slaves, so no issues there.

Comment 9

[[e-mail] (:bhearsum)]

Comment on attachment 460661 [details] [diff] [review]
Automate catlee's script, i.e. allow repack script to process already-signed builds, v3

>diff --git a/scripts/partner-repacks.py b/scripts/partner-repacks.py
>--- a/scripts/partner-repacks.py
>+++ b/scripts/partner-repacks.py
>@@ -1,23 +1,36 @@
> #!/usr/bin/env python
> 
> import os, re, sys
> from shutil import copy, copytree, move
> import subprocess
> from subprocess import Popen
> from optparse import OptionParser
> 
>-PARTNERS_DIR = "../partners"
>-BUILD_NUMBER = "1"
>-STAGING_SERVER = "stage.mozilla.org"
>-PKG_DMG = "pkg-dmg"
>+PARTNERS_DIR = '../partners'
>+BUILD_NUMBER = '1'
>+STAGING_SERVER = 'stage.mozilla.org'
>+HGROOT = 'http://hg.mozilla.org'
>+REPO = 'releases/mozilla-1.9.2'

Could you add a comment to indicate that these are defaults?

>+PKG_DMG = 'pkg-dmg'
>+SEVENZIP_BIN = '7za'
>+UPX_BIN = 'upx'
>+
> SBOX_HOME = '/scratchbox/users/cltbld/home/cltbld/'
> SBOX_PATH = '/scratchbox/moz_scratchbox'
> 
>+SEVENZIP_BUNDLE = 'app.7z'
>+SEVENZIP_APPTAG = 'app.tag'
>+SEVENZIP_APPTAG_PATH = os.path.join('browser/installer/windows', SEVENZIP_APPTAG)
>+SEVENZIP_HEADER = '7zSD.sfx'
>+SEVENZIP_HEADER_PATH = os.path.join('other-licenses/7zstub/firefox', SEVENZIP_HEADER)
>+SEVENZIP_HEADER_COMPRESSED = SEVENZIP_HEADER + '.compressed'
>+
> #########################################################################
> # Source:
> # http://stackoverflow.com/questions/377017/test-if-executable-exists-in-python
> def which(program):
>     def is_exe(fpath):
>         return os.path.exists(fpath) and os.access(fpath, os.X_OK)
> 
>     fpath, fname = os.path.split(program)
>@@ -114,16 +127,20 @@ def isWin(platform):
> 
> #########################################################################
> def isMaemo(platform):
>     if (platform.find('maemo') != -1):
>         return True
>     return False
> 
> #########################################################################
>+def createTagFromVersion(version):
>+    return 'FIREFOX_' + str(version).replace('.','_') + '_RELEASE'
>+
>+#########################################################################
> def parseRepackConfig(file, platforms):
>     config = {}
> 
>     config['platforms'] = []
>     f= open(file, 'r')
>     for line in f:
>         line = line.rstrip("\n")
>         [key, value] = line.split('=',2)
>@@ -372,17 +389,17 @@ class RepackWin32(RepackBase):
>                                           build_dir, working_dir,
>                                           final_dir, repack_info)
>         self.platform = "win32"
> 
>     def copyFiles(self):
>         super(RepackWin32, self).copyFiles('nonlocalized')
> 
>     def repackBuild(self):
>-        zip_cmd = "7za a \"%s\" nonlocalized" % self.build
>+        zip_cmd = "%s a \"%s\" nonlocalized" % (SEVENZIP_BIN, self.build)
>         shellCommand(zip_cmd)
> 
> #########################################################################
> class RepackMaemo(RepackBase):
>     def __init__(self, build, partner_dir, build_dir, working_dir, final_dir,
>                  repack_info, sbox_path=SBOX_PATH, sbox_home=SBOX_HOME):
>         super(RepackMaemo, self).__init__(build, partner_dir,
>                                           build_dir, working_dir,
>@@ -440,16 +457,59 @@ class RepackMaemo(RepackBase):
>         self.unpackBuild()
>         self.copyFiles()
>         self.mungeControl()
>         self.repackBuild()
>         self.cleanup()
>         os.chdir(self.base_dir)
> 
> #########################################################################
>+def repackSignedBuilds(repack_dir):
>+    if not os.path.isdir(repack_dir):
>+        return False
>+    base_dir = os.getcwd()
>+
>+    if not os.path.exists(SEVENZIP_APPTAG):
>+        if not getSingleFileFromHg(SEVENZIP_APPTAG_PATH):
>+            print "Error: Unable to retrieve %s" % SEVENZIP_APPTAG
>+            sys.exit(1)
>+    if not os.path.exists(SEVENZIP_HEADER_COMPRESSED):
>+        if not os.path.exists(SEVENZIP_HEADER) and \
>+           not getSingleFileFromHg(SEVENZIP_HEADER_PATH):
>+            print "Error: Unable to retrieve %s" % SEVENZIP_HEADER
>+            sys.exit(1)
>+        upx_cmd = '%s --best -o \"%s\" \"%s\"' % (UPX_BIN,
>+                                                  SEVENZIP_HEADER_COMPRESSED,
>+                                                  SEVENZIP_HEADER)
>+        shellCommand(upx_cmd)
>+        if not os.path.exists(SEVENZIP_HEADER_COMPRESSED):
>+            print "Error: Unable to compress %s" % SEVENZIP_HEADER
>+            sys.exit(1)
>+
>+    for f in [SEVENZIP_HEADER_COMPRESSED, SEVENZIP_APPTAG, 'repack-signed.sh']:
>+        copy(f, repack_dir)
>+    
>+    os.chdir(repack_dir)
>+    print "Running repack.sh"
>+    shellCommand('./repack-signed.sh')
>+    for f in [SEVENZIP_HEADER_COMPRESSED, SEVENZIP_APPTAG, 'repack-signed.sh']:
>+        os.remove(f)
>+    os.chdir(base_dir)
>+
>+#########################################################################
>+def getSingleFileFromHg(file):
>+    local_file = os.path.basename(file)
>+    file_url = os.path.join(options.hgroot, options.repo, 
>+                            'raw-file', options.tag, file)
>+    wget_cmd = "wget -O \"%s\" \"%s\"" % (local_file, file_url)
>+    print wget_cmd
>+    shellCommand(wget_cmd)
>+    return os.path.exists(local_file)

Any reason not to use urllib.urlretrieve here? The less calling out the better, I think.


Looks fine to me otherwise.

Comment 10

[Chris Cooper [:coop] (he/him)]

Attachment: Automate catlee's script, i.e. allow repack script to process already-signed builds, v4

(In reply to comment #9) 
> Could you add a comment to indicate that these are defaults?

Done.

> Any reason not to use urllib.urlretrieve here? The less calling out the better,
> I think.

I've replaced the wget calls with calls to urllib.urlretrieve wrapped in some error handling.

Comment 11

[[e-mail] (:bhearsum)]

Comment on attachment 460891 [details] [diff] [review]
Automate catlee's script, i.e. allow repack script to process already-signed builds, v4

Great!

Comment 12

[Chris Cooper [:coop] (he/him)]

Comment on attachment 460891 [details] [diff] [review]
Automate catlee's script, i.e. allow repack script to process already-signed builds, v4

Landed:

http://hg.mozilla.org/build/partner-repacks/rev/38f80a681ae4