Support OSX Signed nightlies (in the complete.mar too)

RESOLVED FIXED

Status

task
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: Callek, Assigned: Callek)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(8 attachments)

Assignee

Description

2 years ago
This patch set, as it stands now, will cause actual OSX L10n bustage, but getting en-US ready is worth the early patch set, while I investigate the L10n fix(es).

I'm finishing crafting those patches based on `date` now, but for anyone who wants to validate for themselves you can set your update channel to `nightly-date` and update to a signed Nightly.

:jlund helpfully did this for me.  The benefit here is this also does update unsigned builds *to* signed builds, so we have no need to make our nightly audience reinstall.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 3

2 years ago
mozreview-review
Comment on attachment 8881514 [details]
Bug 1376550 - Support tar.gz (mac) regeneration of complete.mar

https://reviewboard.mozilla.org/r/152662/#review157806

Looks reasonable! Just a few nits.

::: python/mozbuild/mozbuild/repackaging/mar.py:19
(Diff revision 1)
>  from mozbuild.util import ensureParentDir
>  
> +
>  def repackage_mar(topsrcdir, package, mar, output):
> -    if not zipfile.is_zipfile(package):
> +    if not zipfile.is_zipfile(package) and not tarfile.is_tarfile(package):
>          raise Exception("Package file %s is not a valid .zip file." % package)

This could be "valid .zip or .tar file" to match the if statement.

::: python/mozbuild/mozbuild/repackaging/mar.py:37
(Diff revision 1)
> +            filelist = z.getnames()
> +            z.close()
>  
> +        toplevel_dirs = set([mozpath.split(f)[0] for f in filelist])
> +        if not zipfile.is_zipfile(package):
> +            excluded_stuff = set([' ', '.background', '.DS_Store', '.VolumeIcon.icns'])

I think we could just always exclude this stuff rather than exclude it based on the input file type, in case the file type changes in the future. IOW, there's no case where we'd want eg: the .background file to be picked up as a package name.

::: python/mozbuild/mozbuild/repackaging/mar.py:52
(Diff revision 1)
>  
>          env = os.environ.copy()
>          env['MOZ_FULL_PRODUCT_VERSION'] = get_application_ini_value(tmpdir, 'App', 'Version')
>          env['MAR'] = mozpath.normpath(mar)
> +        # Ensure mar is executable
> +        os.chmod(env['MAR'], 0755)

It's a little weird to have this side effect on a file referenced in an environment variable. If we can move it to where Taskcluster downloads it, or at least mozharness, I think that would be preferable.
Attachment #8881514 - Flags: review?(mshal) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 6

2 years ago
mozreview-review
Comment on attachment 8881530 [details]
Bug 1376550 - Make OSX complete.mar's generated via ./mach repackage mozharness configs

https://reviewboard.mozilla.org/r/152688/#review157830

lgtm!
Attachment #8881530 - Flags: review?(aki) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 13

2 years ago
mozreview-review
Comment on attachment 8881533 [details]
Bug 1376550 - Stop signing the complete.mar (generated from the unsigned build) on OSX

https://reviewboard.mozilla.org/r/152700/#review158024
Attachment #8881533 - Flags: review?(dustin) → review+

Comment 14

2 years ago
mozreview-review
Comment on attachment 8881585 [details]
Bug 1376550 - Add complete.mar to repackage tasks.

https://reviewboard.mozilla.org/r/152740/#review158028

::: taskcluster/taskgraph/transforms/repackage.py:100
(Diff revision 1)
>          signing_task = None
>          for dependency in dependencies.keys():
>              if 'signing' in dependency:
>                  signing_task = dependency
> +            else:
> +                build_task = dependency

yucky, but we talked about that last week
Attachment #8881585 - Flags: review?(dustin) → review+

Comment 15

2 years ago
mozreview-review
Comment on attachment 8881544 [details]
Bug 1376550 - Add repackage signing kind.

https://reviewboard.mozilla.org/r/152714/#review158034

::: taskcluster/taskgraph/transforms/repackage_signing.py:22
(Diff revision 2)
> +taskref_or_string = Any(
> +    basestring,
> +    {Required('task-reference'): basestring})

this is unsed (nit)
Attachment #8881544 - Flags: review?(dustin) → review+

Comment 16

2 years ago
mozreview-review
Comment on attachment 8881586 [details]
Bug 1376550 - remove beetmover-repackage-l10n kind.

https://reviewboard.mozilla.org/r/152742/#review158038

::: taskcluster/taskgraph/transforms/beetmover_repackage_l10n.py:23
(Diff revision 1)
>      for job in jobs:
>          dep_job = job['dependent-task']
>  
>          locale = dep_job.attributes.get('locale')
>          if not locale:
> -            return
> +            yield job

Eep, `return` wasn't right here!!
Attachment #8881586 - Flags: review?(dustin) → review+

Comment 17

2 years ago
mozreview-review
Comment on attachment 8881592 [details]
Bug 1376550 - Support beetmoving target.complete.mar (from repackage), does not yet support l10n repacks.

https://reviewboard.mozilla.org/r/152748/#review158040

::: taskcluster/taskgraph/transforms/beetmover_repackage.py:246
(Diff revision 2)
> -            if 'repackage' in dependency:
> +            if 'repackage-signing' in dependency:
> +                repackage_signing_task = dependency
> +            elif 'repackage' in dependency:
>                  repackage_task = dependency
>              elif 'signing' in dependency:
>                  pass

That's a pretty carefully ordered list of tests :)
Attachment #8881592 - Flags: review?(dustin) → review+

Comment 18

2 years ago
mozreview-review
Comment on attachment 8881812 [details]
Bug 1376550 - Wire the unsigned 'build' task as a dependency on the repackage job for l10n repackage jobs, to support the complete.mar generation.

https://reviewboard.mozilla.org/r/152888/#review158046

::: taskcluster/taskgraph/transforms/repackage.py:106
(Diff revision 1)
> +            # XXXCallek: todo: rewrite dependency finding
> +            # Use string splice to strip out 'nightly-l10n-' .. '-<chunk>/opt'
> +            # We need this additional dependency to support finding the mar binary
> +            # Which is needed in order to generate a new complete.mar
> +            dependencies['build'] = "build-{}/opt".format(
> +                dependencies[build_task][13:dependencies[build_task].rfind('-')])

I know this is temporary, but maybe a regexp would be clearer (!) here.
Attachment #8881812 - Flags: review?(dustin) → review+
Assignee

Comment 19

2 years ago
mozreview-review-reply
Comment on attachment 8881812 [details]
Bug 1376550 - Wire the unsigned 'build' task as a dependency on the repackage job for l10n repackage jobs, to support the complete.mar generation.

https://reviewboard.mozilla.org/r/152888/#review158046

> I know this is temporary, but maybe a regexp would be clearer (!) here.

Will do that as a followup, around windows time :-)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 28

2 years ago
Pushed by Callek@gmail.com:
https://hg.mozilla.org/mozilla-central/rev/f98b5c33cecc
Support tar.gz (mac) regeneration of complete.mar r=mshal
https://hg.mozilla.org/mozilla-central/rev/19f13a4e5272
Make OSX complete.mar's generated via ./mach repackage mozharness configs r=aki
https://hg.mozilla.org/mozilla-central/rev/3e2a17000eec
Stop signing the complete.mar (generated from the unsigned build) on OSX r=dustin
https://hg.mozilla.org/mozilla-central/rev/3c0ec95bd6cc
Add complete.mar to repackage tasks. r=dustin
https://hg.mozilla.org/mozilla-central/rev/c4406fc42f85
Add repackage signing kind. r=dustin
https://hg.mozilla.org/mozilla-central/rev/a6fd8e071d84
remove beetmover-repackage-l10n kind. r=dustin
https://hg.mozilla.org/mozilla-central/rev/35a899f16242
Support beetmoving target.complete.mar (from repackage), does not yet support l10n repacks. r=dustin
https://hg.mozilla.org/mozilla-central/rev/d858e9a1e217
Wire the unsigned 'build' task as a dependency on the repackage job for l10n repackage jobs, to support the complete.mar generation. r=dustin a=releng
Assignee

Updated

2 years ago
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED

Updated

a year ago
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.