Closed Bug 1357867 Opened 7 years ago Closed 7 years ago

Land completed OSX cross compile Nightly code to mozilla-central

Categories

(Firefox Build System :: Task Configuration, task)

Product:
Firefox Build System
Component:
Task Configuration
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla55

People

(Reporter: Callek, Assigned: Callek)

Details

Attachments

(8 files, 1 obsolete file)

Bug 1357867 - Land in progress OSX cross compile nightly support.
59 bytes, text/x-review-board-request
dustin
: review+
Details
Bug 1357867 - add support for OSX Nightly (cross compile) Builds.
59 bytes, text/x-review-board-request
dustin
: review+
Details
Bug 1357867 - add support for 'ignore-locales' in tc l10n jobs.
59 bytes, text/x-review-board-request
dustin
: review+
Details
Bug 1357867 - Run l10n tasks to completion for OSX cross compile. And report on correct treeherder line.
59 bytes, text/x-review-board-request
dustin
: review+
Details
Bug 1357867 - Add support for signing OSX cross compile, output is a tarball.
59 bytes, text/x-review-board-request
dustin
: review+
Details
Bug 1357867 - Add repackage tasks suited for generating a .dmg from a signed tarball of OSX binaries.
59 bytes, text/x-review-board-request
dustin
: review+
Details
Bug 1357867 - Add balrog and beetmover support to cross compiled OSX, allow dependencies on repackage jobs.
59 bytes, text/x-review-board-request
dustin
: review+
Details
[Date Only]
15.77 KB, patch
jlund
: feedback+
Details | Diff | Splinter Review 
There is a bunch of landed work on the `date` project branch in support of OSX Nightlies.

With the project getting put on the back burner in favor of a "Plan B" for OSX, and prioritizing Windows, we want to land the in progress work to mozilla-central to avoid bitrot and ease merging to date for Windows work.
Attached patch [Date Only] — — Splinter Review 
Patch to land on date, of stuff on date and would not be on m-c after the patches that I am having dustin land.
Assignee: nobody → bugspam.Callek
Attachment #8859750 - Flags: feedback?(mshal)
Attachment #8859750 - Flags: feedback?(jlund)
Comment on attachment 8859742 [details]
Bug 1357867 - Land in progress OSX cross compile nightly support.

https://reviewboard.mozilla.org/r/131736/#review134870

::: .cron.yml:31
(Diff revision 1)
> +          type: decision-task
> +          treeherder-symbol: Nd-OSX
> +          triggered-by: nightly
> +          target-tasks-method: nightly_macosx
> +      run-on-projects:
> +          - date

I expect this is intentionally still `date`?
Attachment #8859742 - Flags: review?(dustin) → review+
Comment on attachment 8859743 [details]
Bug 1357867 - add support for OSX Nightly (cross compile) Builds.

https://reviewboard.mozilla.org/r/131738/#review134872
Attachment #8859743 - Flags: review?(dustin) → review+
Comment on attachment 8859744 [details]
Bug 1357867 - add support for 'ignore-locales' in tc l10n jobs.

https://reviewboard.mozilla.org/r/131740/#review134874
Attachment #8859744 - Flags: review?(dustin) → review+
Comment on attachment 8859745 [details]
Bug 1357867 - Run l10n tasks to completion for OSX cross compile. And report on correct treeherder line.

https://reviewboard.mozilla.org/r/131742/#review134876
Attachment #8859745 - Flags: review?(dustin) → review+
Comment on attachment 8859746 [details]
Bug 1357867 - Add support for signing OSX cross compile, output is a tarball.

https://reviewboard.mozilla.org/r/131744/#review134878
Attachment #8859746 - Flags: review?(dustin) → review+
Comment on attachment 8859747 [details]
Bug 1357867 - Add repackage tasks suited for generating a .dmg from a signed tarball of OSX binaries.

https://reviewboard.mozilla.org/r/131746/#review134880

::: taskcluster/docs/kinds.rst:213
(Diff revision 1)
>  all the signed multi-locales (aka "multi") APKs for a given release and upload them
>  all at once. They also depend on the breakpoint.
> +
> +repackage
> +---------
> +Repackage tasks take a signed output and package them up into something suiteable

"suitable"

::: taskcluster/taskgraph/transforms/repackage.py:28
(Diff revision 1)
> +taskref_or_string = Any(
> +    basestring,
> +    {Required('task-reference'): basestring})
> +
> +packaging_description_schema = Schema({
> +    # the dependant task (object) for this  job, used to inform signing.

used to inform repackaging?

::: taskcluster/taskgraph/transforms/repackage.py:34
(Diff revision 1)
> +    Required('dependent-task'): object,
> +
> +    # depname is used in taskref's to identify the taskID of the unsigned things
> +    Required('depname', default='build'): basestring,
> +
> +    # unique label to describe this signing task, defaults to {dep.label}-signing

I think this comment is wrong too.. and "signing" is mentioned a few more times in the comments.

::: taskcluster/taskgraph/transforms/repackage.py:54
(Diff revision 1)
> +    Optional('extra'): task_description_schema['extra'],
> +
> +})
> +
> +
> +# comment out adding routes until talking to mshal

Do we want outcommented code in m-c?
Attachment #8859747 - Flags: review?(dustin) → review+
Comment on attachment 8859748 [details]
Bug 1357867 - Add balrog and beetmover support to cross compiled OSX, allow dependencies on repackage jobs.

https://reviewboard.mozilla.org/r/131748/#review134892

::: taskcluster/docs/kinds.rst:189
(Diff revision 1)
>  This separate kind uses logic specific to localized artifacts, such as including
>  the language in the final artifact names.
>  
> +beetmover-repackage
> +---------
> +

... I feel like something is missing here :)

::: taskcluster/taskgraph/loader/single_dep.py:15
(Diff revision 1)
>  def loader(kind, path, config, params, loaded_tasks):
>      """
>      Load tasks based on the jobs dependant kinds.
>  
>      The `only-for-build-platforms` kind configuration, if specified, will limit
>      the build platforms for which a job will be created.

This needs a comment about `not-for-build-platforms`, too.  It would be good to describe how the two interact.  Is it possible to redefine that as

    only-for-build-platforms;
        all-except:
            - plat1
            - plat2

or something that makes the distinction a little clearer?  Just an idea -- adding `not-for-build-platforms` to the docstring would be enough for me.

::: taskcluster/taskgraph/transforms/beetmover_repackage.py:20
(Diff revision 1)
> +# For developers: if you are adding any new artifacts here that need to be
> +# transfered to S3, please be aware you also need to follow-up with patch in
> +# the actual beetmoverscript logic that lies under
> +# https://github.com/mozilla-releng/beetmoverscript/. See example in bug
> +# 1348286
> +_DESKTOP_UPSTREAM_ARTIFACTS_UNSIGNED_EN_US = [
> +    "balrog_props.json",
> +    "target.common.tests.zip",
> +    "target.cppunittest.tests.zip",
> +    "target.crashreporter-symbols.zip",
> +    "target.json",
> +    "target.mochitest.tests.zip",
> +    "target.mozinfo.json",
> +    "target.reftest.tests.zip",
> +    "target.talos.tests.zip",
> +    "target.awsy.tests.zip",
> +    "target.test_packages.json",
> +    "target.txt",
> +    "target.web-platform.tests.zip",
> +    "target.xpcshell.tests.zip",
> +    "target_info.txt",
> +    "target.jsshell.zip",
> +    "mozharness.zip",
> +    "target.langpack.xpi",
> +    "host/bin/mar",
> +    "host/bin/mbsdiff",
> +]
> +_DESKTOP_UPSTREAM_ARTIFACTS_SIGNED_EN_US = [
> +    "update/target.complete.mar",
> +]
> +_DESKTOP_UPSTREAM_ARTIFACTS_UNSIGNED_L10N = [
> +    "target.langpack.xpi",
> +    "balrog_props.json",
> +]
> +_DESKTOP_UPSTREAM_ARTIFACTS_SIGNED_L10N = [
> +    "target.complete.mar",
> +]
> +
> +UPSTREAM_ARTIFACT_UNSIGNED_PATHS = {
> +    'macosx64-nightly': _DESKTOP_UPSTREAM_ARTIFACTS_UNSIGNED_EN_US,
> +    'macosx64-nightly-l10n': _DESKTOP_UPSTREAM_ARTIFACTS_UNSIGNED_L10N,
> +}
> +UPSTREAM_ARTIFACT_SIGNED_PATHS = {
> +    'macosx64-nightly': _DESKTOP_UPSTREAM_ARTIFACTS_SIGNED_EN_US,
> +    'macosx64-nightly-l10n': _DESKTOP_UPSTREAM_ARTIFACTS_SIGNED_L10N,
> +}
> +UPSTREAM_ARTIFACT_REPACKAGE_PATHS = {
> +    'macosx64-nightly': ["target.dmg"],
> +    'macosx64-nightly-l10n': ["target.dmg"],
> +}

Does this have to remain consistent with the similar config in `beetmover.py`?  I don't really understand what it all does, to be honest!  I know there's been some discussion of moving this information to a single location, maybe `taskcluster/ci/beetmover-foo.yml`.
Attachment #8859748 - Flags: review?(dustin) → review+
Comment on attachment 8859750 [details] [diff] [review]
[Date Only]

Review of attachment 8859750 [details] [diff] [review]:
-----------------------------------------------------------------

disclaimer: I'm probably not the best f? for this. :)

I have no idea what 0002-hfslib-set-permissions-when-extracting.patch and 0001-hfslib-create-copies-instead-of-symlinks-on-Windows.patch do. this patch seems to be largely about ripping out osx dev stuff from date but those two files are about windows.

some more thoughts in line.

::: taskcluster/ci/beetmover-repackage/kind.yml
@@ +9,4 @@
>     - taskgraph.transforms.task:transforms
>  
>  kind-dependencies:
> +  - repackage

we are removing a bunch of repackage refs, do we still want this here?

@@ -13,3 @@
>  
>  only-for-build-platforms:
>    - macosx64-nightly/opt

what happens here when we remove macosx64-nightly/opt stanza from macosx.yml?
Attachment #8859750 - Flags: feedback?(jlund) → feedback+
Comment on attachment 8859742 [details]
Bug 1357867 - Land in progress OSX cross compile nightly support.

https://reviewboard.mozilla.org/r/131736/#review134982

::: .cron.yml:31
(Diff revision 1)
> +          type: decision-task
> +          treeherder-symbol: Nd-OSX
> +          triggered-by: nightly
> +          target-tasks-method: nightly_macosx
> +      run-on-projects:
> +          - date

Correct, is intentionally still date
Comment on attachment 8859747 [details]
Bug 1357867 - Add repackage tasks suited for generating a .dmg from a signed tarball of OSX binaries.

https://reviewboard.mozilla.org/r/131746/#review134986

::: taskcluster/taskgraph/transforms/repackage.py:34
(Diff revision 1)
> +    Required('dependent-task'): object,
> +
> +    # depname is used in taskref's to identify the taskID of the unsigned things
> +    Required('depname', default='build'): basestring,
> +
> +    # unique label to describe this signing task, defaults to {dep.label}-signing

indeed, looks like much of this was curbed directly from the signing transforms when landed on date

::: taskcluster/taskgraph/transforms/repackage.py:54
(Diff revision 1)
> +    Optional('extra'): task_description_schema['extra'],
> +
> +})
> +
> +
> +# comment out adding routes until talking to mshal

Probably not, removed.
Comment on attachment 8859748 [details]
Bug 1357867 - Add balrog and beetmover support to cross compiled OSX, allow dependencies on repackage jobs.

https://reviewboard.mozilla.org/r/131748/#review134988

::: taskcluster/docs/kinds.rst:189
(Diff revision 1)
>  This separate kind uses logic specific to localized artifacts, such as including
>  the language in the final artifact names.
>  
> +beetmover-repackage
> +---------
> +

:-P -- Added

::: taskcluster/taskgraph/transforms/beetmover_repackage.py:20
(Diff revision 1)
> +# For developers: if you are adding any new artifacts here that need to be
> +# transfered to S3, please be aware you also need to follow-up with patch in
> +# the actual beetmoverscript logic that lies under
> +# https://github.com/mozilla-releng/beetmoverscript/. See example in bug
> +# 1348286
> +_DESKTOP_UPSTREAM_ARTIFACTS_UNSIGNED_EN_US = [
> +    "balrog_props.json",
> +    "target.common.tests.zip",
> +    "target.cppunittest.tests.zip",
> +    "target.crashreporter-symbols.zip",
> +    "target.json",
> +    "target.mochitest.tests.zip",
> +    "target.mozinfo.json",
> +    "target.reftest.tests.zip",
> +    "target.talos.tests.zip",
> +    "target.awsy.tests.zip",
> +    "target.test_packages.json",
> +    "target.txt",
> +    "target.web-platform.tests.zip",
> +    "target.xpcshell.tests.zip",
> +    "target_info.txt",
> +    "target.jsshell.zip",
> +    "mozharness.zip",
> +    "target.langpack.xpi",
> +    "host/bin/mar",
> +    "host/bin/mbsdiff",
> +]
> +_DESKTOP_UPSTREAM_ARTIFACTS_SIGNED_EN_US = [
> +    "update/target.complete.mar",
> +]
> +_DESKTOP_UPSTREAM_ARTIFACTS_UNSIGNED_L10N = [
> +    "target.langpack.xpi",
> +    "balrog_props.json",
> +]
> +_DESKTOP_UPSTREAM_ARTIFACTS_SIGNED_L10N = [
> +    "target.complete.mar",
> +]
> +
> +UPSTREAM_ARTIFACT_UNSIGNED_PATHS = {
> +    'macosx64-nightly': _DESKTOP_UPSTREAM_ARTIFACTS_UNSIGNED_EN_US,
> +    'macosx64-nightly-l10n': _DESKTOP_UPSTREAM_ARTIFACTS_UNSIGNED_L10N,
> +}
> +UPSTREAM_ARTIFACT_SIGNED_PATHS = {
> +    'macosx64-nightly': _DESKTOP_UPSTREAM_ARTIFACTS_SIGNED_EN_US,
> +    'macosx64-nightly-l10n': _DESKTOP_UPSTREAM_ARTIFACTS_SIGNED_L10N,
> +}
> +UPSTREAM_ARTIFACT_REPACKAGE_PATHS = {
> +    'macosx64-nightly': ["target.dmg"],
> +    'macosx64-nightly-l10n': ["target.dmg"],
> +}

I expect it should remain consistent, I'm not a huge fan of how this sits/is organized, but long term we hope to have a better manifest in tree for beetmover, and medium term I hope to refactor some of this a bit better when we have to modify things for windows.
Comment on attachment 8859748 [details]
Bug 1357867 - Add balrog and beetmover support to cross compiled OSX, allow dependencies on repackage jobs.

https://reviewboard.mozilla.org/r/131748/#review134892

> This needs a comment about `not-for-build-platforms`, too.  It would be good to describe how the two interact.  Is it possible to redefine that as
> 
>     only-for-build-platforms;
>         all-except:
>             - plat1
>             - plat2
> 
> or something that makes the distinction a little clearer?  Just an idea -- adding `not-for-build-platforms` to the docstring would be enough for me.

I'd ideally be able to figure out how to get rid of not-for-build-platform entirely, or come up with a better way to matrix this. In the meantime I'll add some data to the docstring...
Attachment #8859741 - Attachment is obsolete: true
Pushed by Callek@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/e9c6a251a807
Land in progress OSX cross compile nightly support. r=dustin
https://hg.mozilla.org/integration/autoland/rev/4fb4020b8c8b
add support for OSX Nightly (cross compile) Builds. r=dustin
https://hg.mozilla.org/integration/autoland/rev/cc380e839a30
add support for 'ignore-locales' in tc l10n jobs. r=dustin
https://hg.mozilla.org/integration/autoland/rev/e873713a2a81
Run l10n tasks to completion for OSX cross compile. And report on correct treeherder line. r=dustin
https://hg.mozilla.org/integration/autoland/rev/83c08163f311
Add support for signing OSX cross compile, output is a tarball. r=dustin
https://hg.mozilla.org/integration/autoland/rev/598dd73ca7f3
Add repackage tasks suited for generating a .dmg from a signed tarball of OSX binaries. r=dustin
https://hg.mozilla.org/integration/autoland/rev/feb371151541
Add balrog and beetmover support to cross compiled OSX, allow dependencies on repackage jobs. r=dustin
Sorry for the delay - do you still need feedback on the date patch? And what is it supposed to do exactly? It's not clear to me why we'd need to change hfs or turn off cross-compiled nightlies, for example.
Flags: needinfo?(bugspam.Callek)
(In reply to Pulsebot from comment #30)
> Pushed by kwierso@gmail.com:
> https://hg.mozilla.org/integration/autoland/rev/e9b061189ad1
> Lint fixes a=bustage

Thanks for this! (I'm sad I missed it, to be honest)


(In reply to Jordan Lund (:jlund) from comment #17)
> Comment on attachment 8859750 [details] [diff] [review]
> [Date Only]
> 
> Review of attachment 8859750 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> disclaimer: I'm probably not the best f? for this. :)
> 
> I have no idea what 0002-hfslib-set-permissions-when-extracting.patch and
> 0001-hfslib-create-copies-instead-of-symlinks-on-Windows.patch do. this
> patch seems to be largely about ripping out osx dev stuff from date but
> those two files are about windows.

This is a rip-out of stuff that landed on date and had to be morphed to land on central from review passes (:mshal took care of the landing in both cases, and is why I flagged him here).

> some more thoughts in line.
> 
> ::: taskcluster/ci/beetmover-repackage/kind.yml
> @@ +9,4 @@
> >     - taskgraph.transforms.task:transforms
> >  
> >  kind-dependencies:
> > +  - repackage
> 
> we are removing a bunch of repackage refs, do we still want this here?

This line is whitespace fixing (`repackage ` vs `repackage`)

> @@ -13,3 @@
> >  
> >  only-for-build-platforms:
> >    - macosx64-nightly/opt
> 
> what happens here when we remove macosx64-nightly/opt stanza from macosx.yml?

In particular the stanza I'm removing is a duplicate (unused, due to redefine) macosx64-nightly, I probably should have explained that up front

(In reply to Michael Shal [:mshal] from comment #32)
> Sorry for the delay - do you still need feedback on the date patch? And what
> is it supposed to do exactly? It's not clear to me why we'd need to change
> hfs or turn off cross-compiled nightlies, for example.

I don't think I need review explicitly, its primary purpose was to match central to date closer, including removing files that are not needed in central (after your review comments) and some stale files/changes related to the divergence we hit above.
Flags: needinfo?(bugspam.Callek)
Attachment #8859750 - Flags: feedback?(mshal)
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: