1347579 - Integrate and fully support OSX Signing in taskcluster

Status: RESOLVED FIXED

(Infrastructure & Operations Graveyard :: CIDuty, task)

Description

[Justin Wood (:Callek)]

The work being done by mshal, aki, and kmoir needs to be pulled together and verified, this bug tracks that.

Comment 1

[Justin Wood (:Callek)]

Attachment: Bug 1347579 - Add mozharness script (and docker build shell script) to allow running mach repackage.

(For "Integrate and fully support OSX Signing in taskcluster")

Written as a mozharness script rather than using bare ./mach command because we need to download the upstream artifact
and because we need to download artifacts from tooltool to do the packing back into a .dmg. Future ideal would be to get
rid of the mozharness script and use JUST ./mach.

This is using the ./mach repackage code being created in Bug 1347576. Taking a signed tarball from a dmg supported with
Bug 1346015, and the taskgraph work to schedule this is in Bug 1318505.

Review commit: https://reviewboard.mozilla.org/r/122784/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/122784/

Comment 2

[Justin Wood (:Callek)]

The above patch was tested on https://tools.taskcluster.net/task-inspector/#LJR_SWz5QqWzajWr9q_0rw/

Comment 3

[Aki Sasaki (not active)]

Comment on attachment 8850053 [details]
Bug 1347579 - Add mozharness script (and docker build shell script) to allow running mach repackage.

https://reviewboard.mozilla.org/r/122784/#review125144

There are some nits which we might want to fix if we wanted to keep this script around for a while.  If the goal is to use this for the short term and move to mach in the mid-term, this seems to work just fine.  (I'll leave you and mshal to determine whether configure is needed.)

::: testing/mozharness/configs/repackage/osx_signed.py:4
(Diff revision 1)
> +import os
> +
> +config = {
> +    "signed_input": os.environ['SIGNED_INPUT'],  # Required Env Var

I might have made the default `os.environ.get('SIGNED_INPUT')` inside the `config_options`, and then ensured `signed_input` existed in `self.config` at the end of `self.__init__`.  That would allow for overriding the value via cmdln option without setting the env var.  However, if we're not planning on making this script generally useful, this works in automation as is.

::: testing/mozharness/configs/repackage/osx_signed.py:14
(Diff revision 1)
> +    # ToolTool
> +    "tooltool_manifest_src": 'browser/config/tooltool-manifests/macosx64/cross-releng.manifest',
> +    "tooltool_url": 'http://relengapi/tooltool/',
> +    "tooltool_bootstrap": "setup.sh",
> +    'tooltool_script': ["/builds/tooltool.py"],
> +    'tooltool_cache': os.environ.get('TOOLTOOL_CACHE'),

same here.

::: testing/mozharness/scripts/repackage.py:81
(Diff revision 1)
> +        env = {
> +            'HFS_TOOL': os.path.join(dirs['abs_mozilla_dir'], config['hfs_tool']),
> +            'DMG_TOOL': os.path.join(dirs['abs_mozilla_dir'], config['dmg_tool']),
> +            'MKFSHFS': os.path.join(dirs['abs_mozilla_dir'], config['mkfshfs_tool'])
> +        }
> +        command = [python, 'mach', '--log-no-times', 'repackage',

It sounds like mshal wants us to run configure first.  That's hopefully just another self.run_command() away.

::: testing/mozharness/scripts/repackage.py:91
(Diff revision 1)
> +            cwd=dirs['abs_mozilla_dir'],
> +            partial_env=env,
> +            halt_on_failure=True,
> +        )
> +
> +    def _run_tooltool(self):

This could live in `setup()` or the action could be named `run-tooltool` to avoid having two functions to do one set of actions, but this works.

Comment 4

[Justin Wood (:Callek)]

Attachment: Bug 1347579 - Address Review nits.

Review commit: https://reviewboard.mozilla.org/r/124270/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/124270/

Comment 5

[Justin Wood (:Callek)]

Comment on attachment 8850053 [details]
Bug 1347579 - Add mozharness script (and docker build shell script) to allow running mach repackage.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122784/diff/1-2/

Comment 6

[Aki Sasaki (not active)]

Comment on attachment 8852027 [details]
Bug 1347579 - Address Review nits.

https://reviewboard.mozilla.org/r/124270/#review126870

Comment 7

[Justin Wood (:Callek)]

Attachment: Bug 1347579 - run configure before doing repack.

Review commit: https://reviewboard.mozilla.org/r/124384/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/124384/

Comment 8

[Aki Sasaki (not active)]

Comment on attachment 8852159 [details]
Bug 1347579 - run configure before doing repack.

https://reviewboard.mozilla.org/r/124384/#review126934

A couple nits, but overall looks good!

::: testing/mozharness/scripts/repackage.py:125
(Diff revision 1)
> +        abs_mozconfig_path = ''
> +
> +        # first determine the mozconfig path
> +        if c.get('src_mozconfig'):
> +            self.info('Using in-tree mozconfig')
> +            abs_mozconfig_path = os.path.join(dirs['abs_mozilla_dir'], c.get('src_mozconfig'))

Nit: this can be `c['src_mozconfig']` since you're inside an `if c.get('src_mozconfig'):` block.

::: testing/mozharness/scripts/repackage.py:133
(Diff revision 1)
> +                       "in order to determine the mozconfig.")
> +
> +        # print its contents
> +        content = self.read_from_file(abs_mozconfig_path, error_level=FATAL)
> +        self.info("mozconfig content:")
> +        self.info(content)

`self.read_from_file` already logs the contents :)

This will duplicate the logs.

https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/base/script.py#975

Comment 9

[Justin Wood (:Callek)]

Attachment: Bug 1347579 - review nits from configure add.

Review commit: https://reviewboard.mozilla.org/r/124406/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/124406/

Comment 10

[Aki Sasaki (not active)]

Comment on attachment 8852194 [details]
Bug 1347579 - review nits from configure add.

https://reviewboard.mozilla.org/r/124406/#review126980

Comment 11

[Justin Wood (:Callek)]

Comment on attachment 8850053 [details]
Bug 1347579 - Add mozharness script (and docker build shell script) to allow running mach repackage.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122784/diff/2-3/

Comment 12

[Justin Wood (:Callek)]

Comment on attachment 8852159 [details]
Bug 1347579 - run configure before doing repack.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/124384/diff/1-2/

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3164718edbf
Add mozharness script (and docker build shell script) to allow running mach repackage. r=aki
https://hg.mozilla.org/integration/autoland/rev/2d17b7839fd8
run configure before doing repack. r=aki

Comment 14

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d3164718edbf
https://hg.mozilla.org/mozilla-central/rev/2d17b7839fd8