Closed Bug 1347579 Opened 4 years ago Closed 4 years ago

Integrate and fully support OSX Signing in taskcluster

Categories

(Infrastructure & Operations Graveyard :: CIDuty, task)

task
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
Tracking Status
firefox55 --- fixed

People

(Reporter: Callek, Assigned: Callek)

References

(Depends on 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

The work being done by mshal, aki, and kmoir needs to be pulled together and verified, this bug tracks that.
Depends on: 1347580
Depends on: 1346015
Blocks: 1267425
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.
Attachment #8850053 - Flags: review?(aki) → review+
Comment on attachment 8852027 [details]
Bug 1347579 - Address Review nits.

https://reviewboard.mozilla.org/r/124270/#review126870
Attachment #8852027 - Flags: review?(aki) → review+
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
Attachment #8852159 - Flags: review?(aki) → review+
Comment on attachment 8852194 [details]
Bug 1347579 - review nits from configure add.

https://reviewboard.mozilla.org/r/124406/#review126980
Attachment #8852194 - Flags: review?(aki) → review+
Depends on: 1351474
Attachment #8852027 - Attachment is obsolete: true
Attachment #8852194 - Attachment is obsolete: true
Pushed by Callek@gmail.com:
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
Component: Platform Support → Buildduty
Product: Release Engineering → Infrastructure & Operations
Product: Infrastructure & Operations → Infrastructure & Operations Graveyard
You need to log in before you can comment on or make changes to this bug.