Closed Bug 1381577 Opened 3 years ago Closed 3 years ago

Land date changes to support windows nightlies onto central

Categories

(Release Engineering :: General, enhancement)

enhancement
Not set

Tracking

(firefox56 fixed)

RESOLVED FIXED
Tracking Status
firefox56 --- fixed

People

(Reporter: Callek, Assigned: Callek)

References

Details

Attachments

(39 files, 2 obsolete files)

59 bytes, text/x-review-board-request
dustin
: review+
Details
59 bytes, text/x-review-board-request
dustin
: review+
Details
59 bytes, text/x-review-board-request
dustin
: review+
Details
59 bytes, text/x-review-board-request
dustin
: review+
Details
59 bytes, text/x-review-board-request
dustin
: review+
Details
59 bytes, text/x-review-board-request
dustin
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
mshal
: review+
Details
59 bytes, text/x-review-board-request
aki
: review+
Details
59 bytes, text/x-review-board-request
aki
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
dustin
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
grenade
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
jmaher
: review+
Details
59 bytes, text/x-review-board-request
dustin
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
59 bytes, text/x-review-board-request
kmoir
: review+
Details
This patch set may not enable tier 1, that could be a seperate bug (depending mostly on how long it takes for review passes and this to land).
Comment on attachment 8887184 [details]
Bug 1381577 - Part A; Support mozharness actions on windows generic worker

https://reviewboard.mozilla.org/r/157940/#review163116
Attachment #8887184 - Flags: review?(dustin) → review+
Comment on attachment 8887185 [details]
Bug 1381577 - Part B; Rename the nightly win64 hook to just nightly, support win32.

https://reviewboard.mozilla.org/r/157942/#review163118
Attachment #8887185 - Flags: review?(dustin) → review+
Comment on attachment 8887186 [details]
Bug 1381577 - Part C; Add support for not passing MOZ_SIMPLE_PACKAGE_NAME to windows workers.

https://reviewboard.mozilla.org/r/157944/#review163120

::: taskcluster/taskgraph/transforms/job/mozharness.py:213
(Diff revision 1)
>          'MOZ_BUILD_DATE': config.params['moz_build_date'],
>          'MOZ_SCM_LEVEL': config.params['level'],
> -        'MOZ_SIMPLE_PACKAGE_NAME': 'target',
>          'MOZ_AUTOMATION': '1',
>      })
> +    if not env.get('DISABLE_SIMPLE_PACKAGE', False):

This feels a little weird to be passing configuration from transform to transform with environment variables.  I don't see where `DISABLE_SIMPLE_PACKAGE` is set, so I assume that's what's happening here..
Attachment #8887186 - Flags: review?(dustin)
Comment on attachment 8887187 [details]
Bug 1381577 - Part D; Add support for not passing --branch and --skip-buildbot-actions to mozharness tasks on windows workers.

https://reviewboard.mozilla.org/r/157946/#review163122

::: taskcluster/taskgraph/transforms/job/mozharness.py:227
(Diff revision 1)
>      mh_command.append('\\'.join([r'.\build\src\testing', run['script'].replace('/', '\\')]))
>      for cfg in run['config']:
>          mh_command.append('--config ' + cfg.replace('/', '\\'))
> +    if 'NO_MAGIC_MH_BUILD_ARGS' not in env:
> +        # XXXCallek this is a hack to genericize the mozharness run to not
> +        # force the passing of these params

Another one.. these could be job properties, even if they're only temporary..
Attachment #8887187 - Flags: review?(dustin)
Comment on attachment 8887188 [details]
Bug 1381577 - Part E; Add assertion that there is no space in each action or option passed to mozharness tests.

https://reviewboard.mozilla.org/r/157948/#review163124
Attachment #8887188 - Flags: review?(dustin) → review+
Comment on attachment 8887189 [details]
Bug 1381577 - Part F; Make artifact path setting more generic for windows worker.

https://reviewboard.mozilla.org/r/157950/#review163128
Attachment #8887189 - Flags: review?(dustin) → review+
Comment on attachment 8887198 [details]
Bug 1381577 - Part G; Add win32 nightlies as well.

https://reviewboard.mozilla.org/r/157962/#review163508
Attachment #8887198 - Flags: review?(kmoir) → review+
Comment on attachment 8887199 [details]
Bug 1381577 - Part H; Upload Symbols for win32.

https://reviewboard.mozilla.org/r/157964/#review163512
Attachment #8887199 - Flags: review?(kmoir) → review+
Comment on attachment 8887200 [details]
Bug 1381577 - Part I; Win nightlies should generate and upload balrog_props.json.

https://reviewboard.mozilla.org/r/157966/#review163516
Attachment #8887200 - Flags: review?(kmoir) → review+
Comment on attachment 8887602 [details]
Bug 1381577 - Part S; Support Taskcluster win l10n repacks in mozharness scripts.

https://reviewboard.mozilla.org/r/158468/#review163726
Attachment #8887602 - Flags: review?(aki) → review+
Comment on attachment 8887603 [details]
Bug 1381577 - Part T; Add mozharness configs for Taskcluster win l10n repacks.

https://reviewboard.mozilla.org/r/158470/#review163732
Attachment #8887603 - Flags: review?(aki) → review+
Comment on attachment 8887537 [details]
Bug 1381577 - Part O; Run mach repackage for windows nightlies.

https://reviewboard.mozilla.org/r/158406/#review163772
Attachment #8887537 - Flags: review?(kmoir) → review+
Comment on attachment 8887534 [details]
Bug 1381577 - Part L; Support signing windows nightlies.

https://reviewboard.mozilla.org/r/158400/#review163774
Attachment #8887534 - Flags: review?(kmoir) → review+
Comment on attachment 8887599 [details]
Bug 1381577 - Part P; Sign repackaged win artifacts.

https://reviewboard.mozilla.org/r/158462/#review163778
Attachment #8887599 - Flags: review?(kmoir) → review+
Comment on attachment 8887604 [details]
Bug 1381577 - Part U; Add tc win l10n to the taskgraph, and don't try to sign it yet.

https://reviewboard.mozilla.org/r/158472/#review163784

nice job! much more readable
Attachment #8887604 - Flags: review?(kmoir) → review+
Comment on attachment 8887606 [details]
Bug 1381577 - Part W; Sign windows l10n repacks.

https://reviewboard.mozilla.org/r/158476/#review163798
Attachment #8887606 - Flags: review?(kmoir) → review+
Comment on attachment 8887639 [details]
Bug 1381577 - Part Z; Slight refactor to beetmover-repackage to make windows additions clearer.

https://reviewboard.mozilla.org/r/158524/#review164174
Attachment #8887639 - Flags: review?(kmoir) → review+
Comment on attachment 8887640 [details]
Bug 1381577 - Part AA; Enable windows beetmover for en-US and l10n, side affect of enabling balrog for windows nightlies.

https://reviewboard.mozilla.org/r/158526/#review164178
Attachment #8887640 - Flags: review?(kmoir) → review+
Comment on attachment 8887601 [details]
Bug 1381577 - Part R; Build system changes to support taskcluster backed windows l10n repacks.

https://reviewboard.mozilla.org/r/158466/#review164188

LGTM!

::: commit-message-9b8d2:2
(Diff revision 1)
> +Bug 1381577 - Build system changes to support taskcluster backed windows l10n repacks. r=mshal
> +Land date changes to support windows nightlies onto central

I think it would be helpful if the commit message included the rationale for why we need a separate installer URL.

::: toolkit/mozapps/installer/upload-files.mk:415
(Diff revision 1)
>    $(call QUOTED_WILDCARD,$(topobjdir)/browser/installer/windows/instgen/setup-stub.exe) \
>    $(if $(UPLOAD_EXTRA_FILES), $(foreach f, $(UPLOAD_EXTRA_FILES), $(wildcard $(DIST)/$(f))))
>  
> +ifneq ($(AB_CD),x-test)
> +  UPLOAD_FILES += \
> +    $(call QUOTED_WILDCARD,$(topobjdir)/browser/installer/windows/l10ngen/setup.exe) \

Not something to fix for now, but maybe these can be  combined with the instgen/setup.exe & setup-stub.exe lines somehow in the future? I suspect once we have the full installer repackaging toolchain as the only method of generating the installer, we'll want to rewrite how those exes are generated.
Attachment #8887601 - Flags: review?(mshal) → review+
Comment on attachment 8887533 [details]
Bug 1381577 - Part K; Stop using the property 'use-funsize-routes' since we can determine if we want those routes in better ways.

https://reviewboard.mozilla.org/r/158398/#review164202
Attachment #8887533 - Flags: review?(kmoir) → review+
Comment on attachment 8887535 [details]
Bug 1381577 - Part M; Refactor repackage taskgraph code to be more readable and make it easier to add windows.

https://reviewboard.mozilla.org/r/158402/#review164206
Attachment #8887535 - Flags: review?(kmoir) → review+
Comment on attachment 8887536 [details]
Bug 1381577 - Part N; Make repackage task output its artifact to a locale included subdirectory, rather than rely on worker to publish there.

https://reviewboard.mozilla.org/r/158404/#review164212
Attachment #8887536 - Flags: review?(kmoir) → review+
Comment on attachment 8887600 [details]
Bug 1381577 - Part Q; Enable funsize routes for windows nightly.

https://reviewboard.mozilla.org/r/158464/#review164216
Attachment #8887600 - Flags: review?(kmoir) → review+
Comment on attachment 8887638 [details]
Bug 1381577 - Part Y; Don't list host/bin/{mar,mbsdiff} in global list since windows will need the filenames with .exe appended.

https://reviewboard.mozilla.org/r/158522/#review164220
Attachment #8887638 - Flags: review?(kmoir) → review+
Comment on attachment 8887607 [details]
Bug 1381577 - Part X; Run mach repackage for windows l10n, and sign results of windows mach repackage.

https://reviewboard.mozilla.org/r/158478/#review164222
Attachment #8887607 - Flags: review?(kmoir) → review+
Comment on attachment 8887605 [details]
Bug 1381577 - Part V; Remove target.complete.mar from OSX l10n signing (its signed as part of repackage) and error out if an unknown platform attempts to get signed from l10n.

https://reviewboard.mozilla.org/r/158474/#review164226
Attachment #8887605 - Flags: review?(kmoir) → review+
c#60 to c#87 requests are simply from me rebasing the patchset onto latest m-c. And can be ignored (I'm going in to clear the re-added review to dustin, now)
Attachment #8887186 - Flags: review?(dustin)
Attachment #8887187 - Flags: review?(dustin)
(In reply to Justin Wood (:Callek) from comment #89)
> Comment on attachment 8887601 [details]
> Bug 1381577 - Build system changes to support taskcluster backed windows
> l10n repacks.
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/158466/diff/2-3/

:mshal can you give this a once over (due to the followup makefile change and the commit message)
Flags: needinfo?(mshal)
Comment on attachment 8888128 [details]
Bug 1381577 - Part AH; Revert to BBB for talos, until TC's hardware worker is ready for windows.

https://reviewboard.mozilla.org/r/159024/#review164532
Attachment #8888128 - Flags: review?(rthijssen) → review+
Comment on attachment 8888321 [details]
Bug 1381577 - Part AI; Add tests reftest-gpu, talos-quantum-pageload, and talos-xperf.

https://reviewboard.mozilla.org/r/159262/#review164648

we need to add perf-reftest-singletons T(ps) which was turned on last week for win7/10 only.
Attachment #8888321 - Flags: review?(jmaher) → review+
Comment on attachment 8888124 [details]
Bug 1381577 - Part AC; firefox-ui-functional-local should use 'default' tier not explicitly 1 (makes it so we can add this test to not-yet-ready platforms, or non-tier-1 platforms).

https://reviewboard.mozilla.org/r/159016/#review164654

::: taskcluster/ci/test/tests.yml:122
(Diff revision 1)
>  firefox-ui-functional-local:
>      description: "Firefox-ui-tests functional run"
>      suite: "firefox-ui/functional local"
>      treeherder-symbol: tc-Fxfn-l(en-US)
>      max-run-time: 5400
> -    tier: 1
> +    tier: default

why not just remove the line?
Attachment #8888124 - Flags: review?(dustin) → review+
(In reply to Justin Wood (:Callek) from comment #100)
> :mshal can you give this a once over (due to the followup makefile change
> and the commit message)

Looks great - thanks for the detailed message!
Comment on attachment 8888322 [details]
Bug 1381577 - Part AJ; Allow to override worker type in test definitions, supports explicitly buildbot-bridge tests.

https://reviewboard.mozilla.org/r/159264/#review164658

This looks OK, but I'm confused why the need for all of the
```
    worker-type:
        by-test-platform:
            default: null
```
since that should be the default.  Is that preparation for a later landing?
Attachment #8888322 - Flags: review?(dustin) → review+
Attachment #8887641 - Attachment is obsolete: true
Flags: needinfo?(mshal)
Comment on attachment 8887186 [details]
Bug 1381577 - Part C; Add support for not passing MOZ_SIMPLE_PACKAGE_NAME to windows workers.

https://reviewboard.mozilla.org/r/157944/#review163120

> This feels a little weird to be passing configuration from transform to transform with environment variables.  I don't see where `DISABLE_SIMPLE_PACKAGE` is set, so I assume that's what's happening here..

Fixed with a new run feature
Comment on attachment 8887187 [details]
Bug 1381577 - Part D; Add support for not passing --branch and --skip-buildbot-actions to mozharness tasks on windows workers.

https://reviewboard.mozilla.org/r/157946/#review163122

> Another one.. these could be job properties, even if they're only temporary..

Fixed with a new run feature.
Comment on attachment 8887186 [details]
Bug 1381577 - Part C; Add support for not passing MOZ_SIMPLE_PACKAGE_NAME to windows workers.

https://reviewboard.mozilla.org/r/157944/#review164706
Attachment #8887186 - Flags: review?(dustin) → review+
Comment on attachment 8887187 [details]
Bug 1381577 - Part D; Add support for not passing --branch and --skip-buildbot-actions to mozharness tasks on windows workers.

https://reviewboard.mozilla.org/r/157946/#review164708

::: taskcluster/taskgraph/transforms/job/mozharness.py:242
(Diff revision 3)
>      mh_command.append('\\'.join([r'.\build\src\testing', run['script'].replace('/', '\\')]))
>      for cfg in run['config']:
>          mh_command.append('--config ' + cfg.replace('/', '\\'))
> +    if run['use-magic-mh-args']:
> +        # XXXCallek this is a hack to genericize the mozharness run to not
> +        # force the passing of these params

not really a hack anymore..
Attachment #8887187 - Flags: review?(dustin) → review+
Comment on attachment 8888125 [details]
Bug 1381577 - Part AF; Add tests for windows PGO and windows nightly.

https://reviewboard.mozilla.org/r/159018/#review164782
Attachment #8888125 - Flags: review?(kmoir) → review+
Comment on attachment 8888123 [details]
Bug 1381577 - Part AB; Fix typo to make windows PGO get signed.

https://reviewboard.mozilla.org/r/159014/#review164784
Attachment #8888123 - Flags: review?(kmoir) → review+
Comment on attachment 8888127 [details]
Bug 1381577 - Part AG; Rework how test platform variants are calculated for BBB buildernames.

https://reviewboard.mozilla.org/r/159022/#review164786
Attachment #8888127 - Flags: review?(kmoir) → review+
Comment on attachment 8888320 [details]
Bug 1381577 - Part AE; Don't run windows talos until flag day.

https://reviewboard.mozilla.org/r/159260/#review164788

I think I remember seeing the extra spacve after "common-tests" are removed in another patch
Attachment #8888320 - Flags: review?(kmoir) → review+
Comment on attachment 8888323 [details]
Bug 1381577 - Part AK; Add support for win8 testing, will be via BBB, leave disabled until flag day.

https://reviewboard.mozilla.org/r/159266/#review164798
Attachment #8888323 - Flags: review?(kmoir) → review+
Comment on attachment 8888324 [details]
Bug 1381577 - Part AL; Filter tasks based on run-on-projects for windows nightlies too.

https://reviewboard.mozilla.org/r/159268/#review164806
Attachment #8888324 - Flags: review?(kmoir) → review+
Comment on attachment 8888126 [details]
Bug 1381577 - Part AD; cleanup run on projects, to ensure things are run everywhere they are supposed to.

https://reviewboard.mozilla.org/r/159020/#review164810
Attachment #8888126 - Flags: review?(kmoir) → review+
Comment on attachment 8888321 [details]
Bug 1381577 - Part AI; Add tests reftest-gpu, talos-quantum-pageload, and talos-xperf.

https://reviewboard.mozilla.org/r/159262/#review164648

As discussed on IRC this test was backed out of the tree a day or two ago, I'm going to revisit it early next week after this patch set lands.
Comment on attachment 8888322 [details]
Bug 1381577 - Part AJ; Allow to override worker type in test definitions, supports explicitly buildbot-bridge tests.

https://reviewboard.mozilla.org/r/159264/#review164658

Yea, its to make the addition of buildbot-bridge to these entires clearer.
Comment on attachment 8887187 [details]
Bug 1381577 - Part D; Add support for not passing --branch and --skip-buildbot-actions to mozharness tasks on windows workers.

https://reviewboard.mozilla.org/r/157946/#review165242

::: taskcluster/taskgraph/transforms/job/mozharness.py:242
(Diff revision 3)
>      mh_command.append('\\'.join([r'.\build\src\testing', run['script'].replace('/', '\\')]))
>      for cfg in run['config']:
>          mh_command.append('--config ' + cfg.replace('/', '\\'))
> +    if run['use-magic-mh-args']:
> +        # XXXCallek this is a hack to genericize the mozharness run to not
> +        # force the passing of these params

Removed this comment
Comment on attachment 8888876 [details]
Bug 1381577 - Part J; Sign setup.exe on windows and error if we try to sign an unknown platform.

https://reviewboard.mozilla.org/r/159894/#review165258
Attachment #8888876 - Flags: review?(kmoir) → review+
Comment on attachment 8888877 [details]
Bug 1381577 - Part AM; Make sure to properly utilize MOZ_BUILD_DATE in taskcluster windows.

https://reviewboard.mozilla.org/r/159896/#review165260
Attachment #8888877 - Flags: review?(kmoir) → review+
Duplicate of this bug: 1375851