Closed
Bug 1423081
Opened 7 years ago
Closed 7 years ago
land firefox+devedition relpro patches from maple to m-c
Categories
(Release Engineering :: Release Automation: Other, enhancement)
Release Engineering
Release Automation: Other
Tracking
(firefox59 fixed)
RESOLVED
FIXED
| Tracking | Status | |
|---|---|---|
| firefox59 | --- | fixed |
People
(Reporter: mozilla, Assigned: mozilla)
References
Details
Attachments
(34 files, 3 obsolete files)
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
|
59 bytes,
text/x-review-board-request
|
Callek
:
review+
|
Details |
We have a lot of unmerged patches, largely due to both the change freeze and not wanting to have patches ride the trains before they're ready. It's probably too late at this time to have bisectable, logically split patches. At the moment I'm thinking we'll split these up by file; I'll try to get the base files early in the review, and add the relatively simple-to-review kind yml files at the end.
Starting these reviews early, even though we're expecting more changes, so we have more time to address review comments. Thanks for the suggestion, Callek!
The patches in this bug are dependent on bug 1423014. I'm also excluding Johan's google play work in bug 1385401.
|
Assignee | |
Comment 1•7 years ago
|
||
I currently have https://github.com/mozilla/gecko-dev/compare/master...escapewindow:maple-staging?expand=1 . I'm hoping to clean up the commit comments and possibly rebase a bit more for sanity before submitting to mozreview.
| 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 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 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 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 hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 37•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934790 [details]
bug 1423081 - add release partials support.
https://reviewboard.mozilla.org/r/205668/#review211640
This one looks pretty good, I'm not marking r+ yet until I get a better feel for the whole patchset and these questions though.
::: taskcluster/ci/partials-signing/kind.yml:17
(Diff revision 1)
> kind-dependencies:
> - partials
> +
> +job-template:
> + shipping-phase: promote
> + shipping-product: firefox
this should support devedition, for shipping-product, right?
(I'm assuming that is upcoming, leaving this as an issue to confirm... I won't be marking other cases of this potential for now)
::: taskcluster/taskgraph/transforms/partials.py:109
(Diff revision 1)
> + if 'product' in builds[build]:
> + partial_info['product'] = builds[build]['product']
> + if 'previousVersion' in builds[build]:
> + partial_info['previousVersion'] = builds[build]['previousVersion']
> + if 'previousBuildNumber' in builds[build]:
> + partial_info['previousBuildNumber'] = builds[build]['previousBuildNumber']
I'm not sure I understand what these previous* things are used for. I could use some help there... (since they are passed as data into the generation of partials for funsize)
::: taskcluster/taskgraph/util/partials.py:165
(Diff revision 1)
> + if "-localtest" in channel:
> + return channel
> +
> +
> +def populate_release_history(product, branch, maxbuilds=4, maxsearch=10,
> + partial_updates=None):
I don't see partial_updates being utilized from the decision graph itself.
Is it intentional that we don't seem to have any way to fill that in other than via action tasks?
|
|
||
Comment 38•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934802 [details]
bug 1423081 - buildbot release worker doesn't need force=True.
https://reviewboard.mozilla.org/r/205692/#review211650
::: commit-message-47d48:1
(Diff revision 1)
> +bug 1423081 - buildbot release worker doesn't need force=True. r=callek
Technically, per current maple anyway, get_release_config doesn't even take a kwarg of force anymore ;-)
Attachment #8934802 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 39•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934801 [details]
bug 1423081 - add devedition+source to the gecko_v2_whitelist.
https://reviewboard.mozilla.org/r/205690/#review211652
::: commit-message-88265:1
(Diff revision 1)
> +bug 1423081 - add devedition+source to the gecko_v2_whitelist. r=callek
This patch makes me wonder if "{}-devedition" can be removed in favor of the "{}-devedition-opt" entries. (we'll probably just drop the whitelist once bb is dead, so not a requirement)
Attachment #8934801 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 40•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934812 [details]
bug 1423081 - add version.txt to decision sparse checkout.
https://reviewboard.mozilla.org/r/205712/#review211654
Attachment #8934812 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 41•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934823 [details]
bug 1423081 - desktop tc relpro taskcluster docs.
https://reviewboard.mozilla.org/r/205734/#review211656
::: taskcluster/docs/attributes.rst:196
(Diff revision 1)
> ================
> For release promotion jobs, this is the product we are shipping.
>
> shipping_phase
> ==============
> -For release promotion jobs, this is the shipping phase (promote, publish, ship).
> +For release promotion jobs, this is the shipping phase (build, promote, push, ship).
nit: would love a longer description of what each phase is, so that future work has a definition of what belongs in each phase.
We can postpone that for "cleanup" though.
::: taskcluster/docs/kinds.rst:206
(Diff revision 1)
> Beetmover-cdns publishes promoted releases to CDNs. This is part of release promotion.
>
> +beetmover-source
> +-------------------
> +
> +Beetmover-source publishes release source This is part of release promotion.
s/release source This/release source. This/
::: taskcluster/docs/kinds.rst:234
(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.
>
> -release-notify-publish
> +release-binary-transparency
> +---------------------------
> +Binary transparency to issue a certificate
I think many of these release-* kind descriptions are a bit under-verbose, e.g. "What is binary transparency, what sort of certificate..." and "What is a snap" --> "Task that generates an installer using Ubuntu's Snap format"
This is more cleanup fodder though
::: taskcluster/docs/kinds.rst:337
(Diff revision 1)
> +--------------------
> +Dummy tasks to handle balrog tasks as dependencies without hitting max-dependencies.
> +
> +post-beetmover-dummy
> +--------------------
> +Dummy tasks to handle beetmover tasks as dependencies without hitting max-dependencies.
nit (x2) "Dummy tasks to consolidate .... dependencies used in order to avoid taskcluster limits on number of dependencies per task" or some such.
Attachment #8934823 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 42•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934811 [details]
bug 1423081 - desktop release support in transforms/task.py
https://reviewboard.mozilla.org/r/205710/#review211848
::: taskcluster/taskgraph/transforms/task.py:362
(Diff revision 1)
> Optional('command'): [taskref_or_string],
>
> # the maximum time to run, in seconds
> Required('max-run-time'): int,
>
> # the exit status code that indicates the task should be retried
nit: update the comment to say that an array of exit status codes is ok.
::: taskcluster/taskgraph/transforms/task.py:525
(Diff revision 1)
> + Required('contact', default=None): optionally_keyed_by(
> + 'project', 'product', Any(None, basestring)
> + ),
> +
> + # the maximum time to run, in seconds
> + Required('max-run-time', default=600): int,
None of these other entries in the worker are utilized from any passed down tasks in maple.
Are they required to be configurable or should we hardcode in the payload builder below to cut out complexity for now?
::: taskcluster/taskgraph/transforms/task.py:736
(Diff revision 1)
> key=key
> )
>
>
> def verify_index_job_name(index):
> - job_name = index['job-name']
> + job_name = index.get('job-name')
what cases will we not have a job-name and still be valid?
::: taskcluster/taskgraph/transforms/task.py:999
(Diff revision 1)
> + worker = task['worker']
> + release_config = get_release_config(config)
> +
> + contact = resolve_keyed_by(
> + worker, 'contact', 'binary-transparency',
> + project=config.params['project'],
As mentioned above this doesn't actually seem used by entries here.
Of note of course, is that the dict of the job is passed by reference (not copy) [unless something changed outside of my memory] and that resolve_keyed_by replaces values.
So if you're not doing a .deepcopy() you run the risk of having the return of this resolve not being valid against any other possible returns from the same taskgraph. (as in, if you resolve by product, whichever product gets run first will be sticky and you won't get the resolve for next product).
Though as I said, nothing in the current maple branch uses this param, so you get metadata.owner anyway instead ;-)
::: taskcluster/taskgraph/transforms/task.py
(Diff revision 1)
> def add_release_index_routes(config, task):
> index = task.get('index')
> routes = []
> - release_config = get_release_config(config, force=True)
> + release_config = get_release_config(config)
> -
> - verify_index_job_name(index)
why don't we want to verify here anymore?
::: taskcluster/taskgraph/transforms/task.py:1349
(Diff revision 1)
> worker_type = task['worker-type'].format(level=level)
> provisioner_id, worker_type = worker_type.split('/', 1)
> + project = config.params['project']
>
> routes = task.get('routes', [])
> - scopes = [s.format(level=level) for s in task.get('scopes', [])]
> + scopes = [s.format(level=level, project=project) for s in task.get('scopes', [])]
Dustin has once scolded me for doing this ({project}) replacement in transforms. It is why I have a big warning in l10n.py transforms [1]
Can we do some sort of comment here that expanding scope on project is only for buildbot-bridge or something. (or drop it entirely if we don't actually need it)
[1] - https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/l10n.py#344
::: taskcluster/taskgraph/transforms/task.py:1461
(Diff revision 1)
> - attributes['shipping_phase'] = task['shipping-phase']
> + attributes['shipping_phase'] = task['shipping-phase']
> - attributes['shipping_product'] = task['shipping-product']
> + else:
> + attributes.setdefault('shipping_phase', None)
> + # shipping_product will always match the upstream task's
> + # shipping_product, so a pre-set existing attributes['shipping_product']
> + # takes precedence over task['shipping-product'].
I'd rather we have a sanity check/exception here if task['shipping-product'] !== attributes['shipping_product'] when set.
Since doing otherwise would cause confusion in the job-defaults and such. (It may have caused me some confusion in some already-looked at reviews).
::: taskcluster/taskgraph/transforms/task.py:1490
(Diff revision 1)
> - if v is None:
> - continue
> - elif isinstance(v, list):
> - v = {'ids': v}
> - if 'completed' == k:
> - v.update({
> + resolve_keyed_by(
> + notification,
> + notification_option,
> + 'notifications',
> + project=config.params['project'],
> + )
same comment as above re: resolve_keyed_by
::: taskcluster/taskgraph/transforms/task.py:1513
(Diff revision 1)
> + # change event to correct event
> + if notification_event != 'artifact-created':
> + notification_event = 'task-' + notification_event
> +
> + # update notifications
> + task_def['extra']['notifications'][notification_event] = notification
Notification stuff is a bit hard to reason about here, and increases the size of this function and with it cognitive load...
I'd love if we either made a seperate transform in this file, or a seperate transform file itself to support notification stuff.
This can be relegated to followup.
|
||
Comment 43•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934792 [details]
bug 1423081 - source readme.
https://reviewboard.mozilla.org/r/205672/#review211942
::: taskcluster/taskgraph/transforms/job/mozharness.py:146
(Diff revision 1)
>
> if 'actions' in run:
> env['MOZHARNESS_ACTIONS'] = ' '.join(run['actions'])
>
> if 'options' in run:
> - env['MOZHARNESS_OPTIONS'] = ' '.join(run['options'])
> + env['MOZHARNESS_OPTIONS'] = ' '.join(run['options']).format(**config.params)
This is pretty general, and applies to a lot of jobs. I'm a little worried it will get used more widely and become something of a footgun. My original comment on a similar topic was in https://bugzilla.mozilla.org/show_bug.cgi?id=1306598#c3.
Options:
1. Only substitute `head_repository` and `head_rev` (and document that in the schema in this file). That at least avoids the temptation to substitute any parameter.
2. Do this in a separate transform, specific to this kind and applied before the build, build_attrs, and build_lints transforms. Then other users of the mozharness job transforms don't have this functionality available.
3. Get this information from env vars in mozharness, as it's [already provided](https://tools.taskcluster.net/groups/LcldJhU0T-mAD0MiSq44rg/tasks/AYX58EBRRze1xk_0LvAKvw/details)
|
|
||
Comment 44•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934791 [details]
bug 1423081 - add firefox + devedition relpro.
https://reviewboard.mozilla.org/r/205670/#review211928
::: taskcluster/taskgraph/actions/release_promotion.py:47
(Diff revision 1)
> + 'push_devedition': {
> + 'target_tasks_method': 'push_devedition',
> + },
> + 'ship_devedition': {
> + 'target_tasks_method': 'ship_devedition',
> },
This whole CONFIG looks pretty formulaic at this point. Is it worth keeping around?
|
|
||
Comment 45•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934807 [details]
bug 1423081 - add release updates.
https://reviewboard.mozilla.org/r/205702/#review211960
::: taskcluster/taskgraph/transforms/release_updates.py:5
(Diff revision 1)
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +"""
> +Transform the beetmover task into an actual task description.
needs updating?
|
|
||
Comment 46•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934811 [details]
bug 1423081 - desktop release support in transforms/task.py
https://reviewboard.mozilla.org/r/205710/#review211964
::: taskcluster/taskgraph/transforms/task.py:1349
(Diff revision 1)
> worker_type = task['worker-type'].format(level=level)
> provisioner_id, worker_type = worker_type.split('/', 1)
> + project = config.params['project']
>
> routes = task.get('routes', [])
> - scopes = [s.format(level=level) for s in task.get('scopes', [])]
> + scopes = [s.format(level=level, project=project) for s in task.get('scopes', [])]
This might be OK (we already substitute level here, and for releases project is pretty important). But it should be documented in the schema above (around line 111).
|
|
||
Comment 47•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934813 [details]
bug 1423081 - add shipping-{phase,product} to builds.
https://reviewboard.mozilla.org/r/205714/#review211970
::: taskcluster/ci/build/linux.yml:189
(Diff revision 1)
> script: "mozharness/scripts/fx_desktop_build.py"
> secrets: true
> tooltool-downloads: public
> need-xvfb: true
> custom-build-variant-cfg: devedition
> - run-on-projects: ['mozilla-beta']
> + run-on-projects: ['mozilla-beta', 'maple']
Intentional to uplift this, or was this just to make it run on maple? (I don't mind either way, just calling it out in case it wasn't intended)
|
|
||
Comment 48•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934823 [details]
bug 1423081 - desktop tc relpro taskcluster docs.
https://reviewboard.mozilla.org/r/205734/#review211972
::: taskcluster/docs/attributes.rst:196
(Diff revision 1)
> ================
> For release promotion jobs, this is the product we are shipping.
>
> shipping_phase
> ==============
> -For release promotion jobs, this is the shipping phase (promote, publish, ship).
> +For release promotion jobs, this is the shipping phase (build, promote, push, ship).
Some longer-form, narrative documentation of the release mechanics would be cool. I admit I don't understand most of the release-related kinds because, I think, I lack a high-level overview.
Maybe that would make sense as a few files in taskcluster/docs/releases, e.g., nightlies.rst, signing.rst, beets-and-balrogs.rst, relpromo.rst.
Definitely not something to do in this bug, just something to think about.
::: taskcluster/docs/kinds.rst:337
(Diff revision 1)
> +--------------------
> +Dummy tasks to handle balrog tasks as dependencies without hitting max-dependencies.
> +
> +post-beetmover-dummy
> +--------------------
> +Dummy tasks to handle beetmover tasks as dependencies without hitting max-dependencies.
Agree that a lot of these are not very helpful (they remind me of documentation that says things like "To create a pivot table, click 'Create Pivot Table', enter the pivot table options, and click 'Save'" -- yeah, I can figure out how to click buttons on my own, but what's a pivot table??).
In particular, even after reading the code I'm not sure how these dummies are used -- if I wanted to depend on some subset of beetmover tasks, how would I do that?
Again, good cleanup-fodder: explain this stuff to the reader as if they were a n00b :)
|
|
||
Comment 49•7 years ago
|
||
I didn't mark r+ for these since I don't have the release/relpromo context that Callek does, but what I understand of it (the taskgraph-related bits) looks good, modulo the minor comments above. It's exciting to see all of this falling into place!
|
|
||
Comment 50•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934795 [details]
bug 1423081 - add generate_bz2_blob to schema identifiers.
https://reviewboard.mozilla.org/r/205678/#review212036
Attachment #8934795 -
Flags: review?(bugspam.Callek) → 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 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 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 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 hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Updated•7 years ago
|
Attachment #8934801 -
Attachment is obsolete: true
|
|
Assignee | |
Updated•7 years ago
|
Attachment #8934817 -
Attachment is obsolete: true
Attachment #8934817 -
Flags: review?(bugspam.Callek)
|
|
Assignee | |
Comment 85•7 years ago
|
||
(In reply to Dustin J. Mitchell [:dustin] from comment #49)
> I didn't mark r+ for these since I don't have the release/relpromo context
> that Callek does, but what I understand of it (the taskgraph-related bits)
> looks good, modulo the minor comments above. It's exciting to see all of
> this falling into place!
Woo, thank you!
Latest patches pushed, I hope I didn't screw anything up.
|
|
Assignee | |
Comment 86•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934790 [details]
bug 1423081 - add release partials support.
https://reviewboard.mozilla.org/r/205668/#review212124
::: taskcluster/ci/partials-signing/kind.yml:17
(Diff revision 1)
> kind-dependencies:
> - partials
> +
> +job-template:
> + shipping-phase: promote
> + shipping-product: firefox
Correct. Fixed on maple, should be fixed in the coming patch updates.
::: taskcluster/taskgraph/transforms/partials.py:109
(Diff revision 1)
> + if 'product' in builds[build]:
> + partial_info['product'] = builds[build]['product']
> + if 'previousVersion' in builds[build]:
> + partial_info['previousVersion'] = builds[build]['previousVersion']
> + if 'previousBuildNumber' in builds[build]:
> + partial_info['previousBuildNumber'] = builds[build]['previousBuildNumber']
I think that's for generating partials. For more I might have to send you to Simon, Rail, Ben, Nick? Maybe Mihai.
::: taskcluster/taskgraph/util/partials.py:165
(Diff revision 1)
> + if "-localtest" in channel:
> + return channel
> +
> +
> +def populate_release_history(product, branch, maxbuilds=4, maxsearch=10,
> + partial_updates=None):
Correct, release history isn't used outside of action tasks currently. Nightlies use partials, but I'm thinking they, too, will become shippable builds that are promoted via action tasks, on some cadence.
|
|
Assignee | |
Comment 87•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934791 [details]
bug 1423081 - add firefox + devedition relpro.
https://reviewboard.mozilla.org/r/205670/#review212128
::: taskcluster/taskgraph/actions/release_promotion.py:47
(Diff revision 1)
> + 'push_devedition': {
> + 'target_tasks_method': 'push_devedition',
> + },
> + 'ship_devedition': {
> + 'target_tasks_method': 'ship_devedition',
> },
Good point :) We still allow for defining `do_not_optimize` and `rebuild_kinds` in the config, but haven't needed to yet. I'm not 100% sure we won't need those as we hit real world issues. I'm leaning towards keeping this until we prove we don't need those two config items, and then we can tear it out. Does that work?
|
|
Assignee | |
Comment 88•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934791 [details]
bug 1423081 - add firefox + devedition relpro.
https://reviewboard.mozilla.org/r/205670/#review212148
|
|
Assignee | |
Comment 89•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934792 [details]
bug 1423081 - source readme.
https://reviewboard.mozilla.org/r/205672/#review212132
::: taskcluster/taskgraph/transforms/job/mozharness.py:146
(Diff revision 1)
>
> if 'actions' in run:
> env['MOZHARNESS_ACTIONS'] = ' '.join(run['actions'])
>
> if 'options' in run:
> - env['MOZHARNESS_OPTIONS'] = ' '.join(run['options'])
> + env['MOZHARNESS_OPTIONS'] = ' '.join(run['options']).format(**config.params)
I'm leaning towards 1 or 3. Keeping this open for now.
|
|
Assignee | |
Comment 90•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934811 [details]
bug 1423081 - desktop release support in transforms/task.py
https://reviewboard.mozilla.org/r/205710/#review212136
::: taskcluster/taskgraph/transforms/task.py:1490
(Diff revision 1)
> - if v is None:
> - continue
> - elif isinstance(v, list):
> - v = {'ids': v}
> - if 'completed' == k:
> - v.update({
> + resolve_keyed_by(
> + notification,
> + notification_option,
> + 'notifications',
> + project=config.params['project'],
> + )
A number of these issues are dealt with in the next round of patch updates.
|
|
Assignee | |
Comment 91•7 years ago
|
||
(In reply to Justin Wood (:Callek) from comment #39)
> Comment on attachment 8934801 [details]
> bug 1423081 - add devedition+source to the gecko_v2_whitelist.
>
> https://reviewboard.mozilla.org/r/205690/#review211652
>
> ::: commit-message-88265:1
> (Diff revision 1)
> > +bug 1423081 - add devedition+source to the gecko_v2_whitelist. r=callek
>
> This patch makes me wonder if "{}-devedition" can be removed in favor of the
> "{}-devedition-opt" entries. (we'll probably just drop the whitelist once bb
> is dead, so not a requirement)
Yeah, I don't understand the whitelist, so I just added entries til it stopped complaining. Dropping it once bb is dead sgtm.
|
|
Assignee | |
Comment 92•7 years ago
|
||
(In reply to Dustin J. Mitchell [:dustin] from comment #47)
> Comment on attachment 8934813 [details]
> bug 1423081 - add shipping-{phase,product} to builds.
>
> https://reviewboard.mozilla.org/r/205714/#review211970
>
> ::: taskcluster/ci/build/linux.yml:189
> (Diff revision 1)
> > script: "mozharness/scripts/fx_desktop_build.py"
> > secrets: true
> > tooltool-downloads: public
> > need-xvfb: true
> > custom-build-variant-cfg: devedition
> > - run-on-projects: ['mozilla-beta']
> > + run-on-projects: ['mozilla-beta', 'maple']
>
> Intentional to uplift this, or was this just to make it run on maple? (I
> don't mind either way, just calling it out in case it wasn't intended)
The latter. I don't have strong feelings either way, but if we land maple support we should land all of it intentionally.
Removed from my patch queue; I'll submit that change the next round.
|
|
Assignee | |
Comment 93•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934811 [details]
bug 1423081 - desktop release support in transforms/task.py
https://reviewboard.mozilla.org/r/205710/#review212184
|
|
Assignee | |
Comment 94•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934807 [details]
bug 1423081 - add release updates.
https://reviewboard.mozilla.org/r/205702/#review211960
> needs updating?
Updated on maple, will update review queue next round.
|
|
Assignee | |
Comment 95•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934811 [details]
bug 1423081 - desktop release support in transforms/task.py
https://reviewboard.mozilla.org/r/205710/#review211848
> Dustin has once scolded me for doing this ({project}) replacement in transforms. It is why I have a big warning in l10n.py transforms [1]
>
> Can we do some sort of comment here that expanding scope on project is only for buildbot-bridge or something. (or drop it entirely if we don't actually need it)
>
> [1] - https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/l10n.py#344
Addressed by updating the schema as Dustin suggested.
> Notification stuff is a bit hard to reason about here, and increases the size of this function and with it cognitive load...
>
> I'd love if we either made a seperate transform in this file, or a seperate transform file itself to support notification stuff.
>
> This can be relegated to followup.
This was cleaned up significantly in the latest push; let me know if you still want this in a separate file.
|
|
Assignee | |
Comment 96•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934823 [details]
bug 1423081 - desktop tc relpro taskcluster docs.
https://reviewboard.mozilla.org/r/205734/#review211656
> nit: would love a longer description of what each phase is, so that future work has a definition of what belongs in each phase.
>
> We can postpone that for "cleanup" though.
Addressed on maple; will update the review queue...
> s/release source This/release source. This/
Fixed on maple..
> I think many of these release-* kind descriptions are a bit under-verbose, e.g. "What is binary transparency, what sort of certificate..." and "What is a snap" --> "Task that generates an installer using Ubuntu's Snap format"
>
> This is more cleanup fodder though
Added a Trello card.
> nit (x2) "Dummy tasks to consolidate .... dependencies used in order to avoid taskcluster limits on number of dependencies per task" or some such.
Fixed on maple...
|
||
Comment 97•7 years ago
|
||
Re: comment 43 patch on maple[1] I pushed a tiny follow-up fix[2]
[1]: https://hg.mozilla.org/projects/maple/rev/425fa61ab335#l3.2
[2]: https://hg.mozilla.org/projects/maple/rev/d21b201d06e66af79a103c89bf51f8ffc5b1c15d
|
||
Comment 98•7 years ago
|
||
(In reply to Justin Wood (:Callek) from comment #37)
> ::: taskcluster/taskgraph/transforms/partials.py:109
> (Diff revision 1)
> > + if 'product' in builds[build]:
> > + partial_info['product'] = builds[build]['product']
> > + if 'previousVersion' in builds[build]:
> > + partial_info['previousVersion'] = builds[build]['previousVersion']
> > + if 'previousBuildNumber' in builds[build]:
> > + partial_info['previousBuildNumber'] = builds[build]['previousBuildNumber']
>
> I'm not sure I understand what these previous* things are used for. I could
> use some help there... (since they are passed as data into the generation
> of partials for funsize)
These are the "from" versions. Maybe this pseudo code would explain it better:
to_version = '60.0'
to_build_number = 1
partials = [
{'previousVersion': '59.0', previousBuildNumber: 1},
{'previousVersion': '58.0', previousBuildNumber: 1},
]
for previousVersion, previousBuildNumber in partials:
print "will generate ${previousVersion}-${previousBuildNumber} to ${to_version}-${to_build_number} partials"
Should output
will generate 59.0-1 to 60.0.1 partials
will generate 58.0-1 to 60.0.1 partials
> ::: taskcluster/taskgraph/util/partials.py:165
> (Diff revision 1)
> > + if "-localtest" in channel:
> > + return channel
> > +
> > +
> > +def populate_release_history(product, branch, maxbuilds=4, maxsearch=10,
> > + partial_updates=None):
>
> I don't see partial_updates being utilized from the decision graph itself.
>
> Is it intentional that we don't seem to have any way to fill that in other
> than via action tasks?
partials are set in ship-it, so they are not the part of the decision task; they are passed via input to the first action task.
|
|
||
Comment 99•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934791 [details]
bug 1423081 - add firefox + devedition relpro.
https://reviewboard.mozilla.org/r/205670/#review212370
::: taskcluster/taskgraph/actions/release_promotion.py:292
(Diff revisions 1 - 2)
> + # that didn't exist in the first full_task_graph, so combining them is
> + # important.
> + combined_full_task_graph = {}
> + for graph_id in previous_graph_ids:
> + full_task_graph = get_artifact(graph_id, "public/full-task-graph.json")
> + combined_full_task_graph.update(full_task_graph)
I have a conceptual wonder about what happens if there is a conflict in this .update() (I know it replaces it, but what conceptually do we want to happen if the contents don't ===)
I don't have a strong concern here, so feel free to say that its not really a worry and resolve.
Attachment #8934791 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 100•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934790 [details]
bug 1423081 - add release partials support.
https://reviewboard.mozilla.org/r/205668/#review212376
Attachment #8934790 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 101•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934792 [details]
bug 1423081 - source readme.
https://reviewboard.mozilla.org/r/205672/#review212426
::: taskcluster/ci/release-source/kind.yml:45
(Diff revision 2)
> + emails:
> + by-project:
> + mozilla-beta: ["release-automation-notifications@mozilla.com"]
> + mozilla-release: ["release-automation-notifications@mozilla.com"]
> + try: ["{task_def[metadata][owner]}"]
> + maple: ["release+tcstaging@mozilla.com"]
possibly don't need maple here, but no big deal if it stays. (x2)
::: taskcluster/ci/release-source/source.yml:17
(Diff revision 2)
> + run:
> + using: mozharness
> + actions: [build]
> + config:
> + - builds/releng_sub_linux_configs/64_source.py
> + options: ["repo={head_repository}", "revision={head_rev}"]
[x3] We could pull this from the environment rather than transform the values... (I think this was suggested elsewhere)
::: taskcluster/taskgraph/transforms/beetmover.py:198
(Diff revision 2)
> 'win32-nightly-l10n': _DESKTOP_UPSTREAM_ARTIFACTS_UNSIGNED_L10N,
> 'win64-nightly-l10n': _DESKTOP_UPSTREAM_ARTIFACTS_UNSIGNED_L10N,
> }
> # Until bug 1331141 is fixed, 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 a beetmover patch in https://github.com/mozilla-releng/beetmoverscript/.
In this case, I'm assuming beetmover is patched
::: taskcluster/taskgraph/transforms/beetmover_source.py:45
(Diff revision 2)
> + )
> + )
> + upstream_artifacts = job['worker']['upstream-artifacts']
> + for artifact in upstream_artifacts:
> + if artifact['taskType'] == 'build':
> + artifact['paths'].append(u'public/build/balrog_props.json')
this might be safer as `artifact['paths'] = [...]` since we're changing the build target above. And in the beetmover transform itself an '# XXX Unused, see comment in `beetmover_source` transform or some such'
Merely a nit, skippable per author discretion
::: taskcluster/taskgraph/transforms/job/mozharness.py:146
(Diff revision 2)
>
> if 'actions' in run:
> env['MOZHARNESS_ACTIONS'] = ' '.join(run['actions'])
>
> if 'options' in run:
> - env['MOZHARNESS_OPTIONS'] = ' '.join(run['options'])
> + env['MOZHARNESS_OPTIONS'] = ' '.join(run['options']).format(**config.params)
if we don't change to pulling by environ, can we at least just pass the specific params we know we care about here (or use a seperate, earlier, transform)
::: taskcluster/taskgraph/util/signed_artifacts.py:14
(Diff revision 2)
>
>
> def generate_specifications_of_artifacts_to_sign(
> - build_platform, is_nightly=False, keep_locale_template=True
> + build_platform, is_nightly=False, keep_locale_template=True, kind=None
> ):
> - if 'android' in build_platform:
> + if kind == 'release-source-signing':
I'm not really a fan of having to switch this on kind name here, *but* I don't have a better suggestion offhand, so unless you do, we'll keep it.
Attachment #8934792 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 102•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934793 [details]
bug 1423081 - target_tasks_methods updates.
https://reviewboard.mozilla.org/r/205674/#review212454
::: taskcluster/taskgraph/target_tasks.py:309
(Diff revision 2)
> -@_target_task('maple_desktop_promotion')
> -@_target_task('mozilla-beta_desktop_promotion')
> +@_target_task('promote_firefox')
> +def target_tasks_promote_firefox(full_task_graph, parameters, graph_config):
> -@_target_task('mozilla-release_desktop_promotion')
> -def target_tasks_mozilla_beta_desktop_promotion(full_task_graph, parameters, graph_config):
> """Select the superset of tasks required to promote a beta or release build
> - of desktop. This should include all non-android mozilla_beta tasks, plus
> + of firefox. This should include all non-android mozilla_beta tasks, plus
nit "mozilla_beta" in doc...
::: taskcluster/taskgraph/target_tasks.py:312
(Diff revision 2)
> -def target_tasks_mozilla_beta_desktop_promotion(full_task_graph, parameters, graph_config):
> """Select the superset of tasks required to promote a beta or release build
> - of desktop. This should include all non-android mozilla_beta tasks, plus
> + of firefox. This should include all non-android mozilla_beta tasks, plus
> l10n, beetmover, balrog, etc."""
>
> beta_tasks = [l for l, t in full_task_graph.tasks.iteritems() if
lets genericize the naming here, there is the phrase "beta" and "beta_tasks" type comments/variables a bunch here.
::: taskcluster/taskgraph/target_tasks.py:350
(Diff revision 2)
> - # TODO: tc update verify
> - # TODO: beetmover push-to-candidates
> - # TODO: binary transparency
> # TODO: bouncer sub
> - # TODO: snap
> # TODO: recompression tasks
Nomenclature question, what is a "recompression task"
::: taskcluster/taskgraph/target_tasks.py:409
(Diff revision 2)
> +def target_tasks_promote_devedition(full_task_graph, parameters, graph_config):
> + """Select the superset of tasks required to promote a beta or release build
> + of devedition. This should include all non-android mozilla_beta tasks, plus
> + l10n, beetmover, balrog, etc."""
> +
> + beta_tasks = [l for l, t in full_task_graph.tasks.iteritems() if
same comment about "beta"
Attachment #8934793 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 103•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934794 [details]
bug 1423081 - inherit shipping-product from upstream.
https://reviewboard.mozilla.org/r/205676/#review212458
::: commit-message-17b68:1
(Diff revision 2)
> +bug 1423081 - inherit shipping-{phase,product} from upstream. r=callek
nit: I don't see anything inheriting shipping-phase from upstream here, just product.
Attachment #8934794 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 104•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934796 [details]
bug 1423081 - add firefox and devedition relpro support to scriptworker.py
https://reviewboard.mozilla.org/r/205680/#review212460
::: taskcluster/taskgraph/util/scriptworker.py:457
(Diff revision 2)
> + partial_updates = json.loads(partial_updates)
> + release_config['partial_versions'] = ', '.join([
> + '{}build{}'.format(v, info['buildNumber'])
> + for v, info in partial_updates.items()
> + ])
> + if release_config['partial_versions'] == "{}":
where would we get "{}" from for this?
Attachment #8934796 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 105•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934797 [details]
bug 1423081 - add release balrog support.
https://reviewboard.mozilla.org/r/205682/#review212466
::: taskcluster/ci/balrog/kind.yml:22
(Diff revision 2)
> only-for-attributes:
> - nightly
> - signed
> +
> +job-template:
> + notifications:
I'm a bit worried about someone accidentally causing balrog notifications on central, but it won't as written so no need to redesign right now
Attachment #8934797 -
Flags: review?(bugspam.Callek) → review+
|
|
Assignee | |
Comment 106•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934791 [details]
bug 1423081 - add firefox + devedition relpro.
https://reviewboard.mozilla.org/r/205670/#review212370
> I have a conceptual wonder about what happens if there is a conflict in this .update() (I know it replaces it, but what conceptually do we want to happen if the contents don't ===)
>
> I don't have a strong concern here, so feel free to say that its not really a worry and resolve.
We explicitly want the latest graph (rightmost) to take precedence in the case of dup/conflicting keys. Would a comment help here?
|
|
||
Comment 107•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934798 [details]
bug 1423081 - add desktop release beetmover support.
https://reviewboard.mozilla.org/r/205684/#review212506
::: taskcluster/taskgraph/transforms/beetmover_repackage.py:432
(Diff revision 2)
> extra = list()
>
> artifact_map = get_partials_artifact_map(
> config.params.get('release_history'), balrog_platform, locale)
> for artifact in artifact_map:
> - extra.append({
> + _ = {
nit: var name should be something other than `_` (I'm not a fan of _ for anything expected to be used...)
Attachment #8934798 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 108•7 years ago
|
||
(In reply to Aki Sasaki [:aki] from comment #106)
> Comment on attachment 8934791 [details]
> bug 1423081 - add firefox + devedition relpro.
>
> https://reviewboard.mozilla.org/r/205670/#review212370
>
> > I have a conceptual wonder about what happens if there is a conflict in this .update() (I know it replaces it, but what conceptually do we want to happen if the contents don't ===)
> >
> > I don't have a strong concern here, so feel free to say that its not really a worry and resolve.
>
> We explicitly want the latest graph (rightmost) to take precedence in the
> case of dup/conflicting keys. Would a comment help here?
I do think it would for future reading of this, yea.
|
|
||
Comment 109•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934799 [details]
bug 1423081 - desktop release update verify support.
https://reviewboard.mozilla.org/r/205686/#review212526
::: taskcluster/ci/release-buildbot-update-verify/kind.yml:9
(Diff revision 2)
> +
> +loader: taskgraph.loader.transform:loader
> +
> +kind-dependencies:
> + # TODO: this makes us depend on beetmover/balrog for all platforms
> + # does it matter?
make sure this question in the TODO is figured out before landing ;-)
::: taskcluster/ci/release-buildbot-update-verify/kind.yml:128
(Diff revision 2)
> + jamun: "beta-firefox-win32.cfg"
> + maple: "beta-firefox-win32.cfg"
> + mozilla-beta: "beta-firefox-win32.cfg"
> + mozilla-release: "release-firefox-win32.cfg"
> + mozilla-esr52: "esr-firefox-win32.cfg"
> + default: "none"
can use: null | Null | NULL | ~
This will eval out into python `None` as a possible improvement in code/readability here
::: taskcluster/ci/release-final-verify/kind.yml:9
(Diff revision 2)
> +
> +loader: taskgraph.loader.transform:loader
> +
> +kind-dependencies:
> + # release-uptake-monitoring will get removed as a dep
> + # for secondary channel final verify
since final_verify has a seperate kind for secondary, drop this comment ;-)
::: taskcluster/ci/release-final-verify/kind.yml:24
(Diff revision 2)
> + run-on-projects: [] # to make sure this never runs as part of CI
> + worker-type: aws-provisioner-v1/gecko-{level}-b-linux
> + worker:
> + implementation: docker-worker
> + os: linux
> + docker-image: mozillareleases/python-test-runner@sha256:0729c2e6e7bc0d6a4cbccb2e66a78e1d8e8cbb5e44105d56e3c9c610230ebd69
cleanup: lets get this docker image in tree ;-)
::: taskcluster/ci/release-secondary-final-verify/kind.yml:9
(Diff revision 2)
> +
> +loader: taskgraph.loader.transform:loader
> +
> +kind-dependencies:
> + # TODO: this makes us depend on beetmover/balrog for all platforms
> + # does it matter?
solve this todo before landing too.
::: taskcluster/ci/release-update-verify/kind.yml:3
(Diff revision 2)
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
is there a reason we can't use `jobs-from:` and set `job-defaults` appropriately for release-update-verify and the buildbot variant.
To avoid an extra kind here, or do they need drastically different transforms/implementations that it doesn't work out like I think?
::: taskcluster/taskgraph/transforms/buildbot_update_verify.py:7
(Diff revision 2)
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +"""
> +Transform the beetmover task into an actual task description.
> +"""
> +
The majority of this transform could be done as a job/* (as in run: using: buildbot_update_verify) if its easier/clearer. (and if it eases merging both UV kinds together)
::: taskcluster/taskgraph/transforms/buildbot_update_verify.py:38
(Diff revision 2)
> + for this_chunk in range(1, total_chunks+1):
> + chunked = deepcopy(task)
> + chunked["scopes"] = [
> + "project:releng:buildbot-bridge:builder-name:{}".format(
> + # In scopes, "branch" is called "project"
> + buildername.replace("branch", "project")
I'd probably suggest buildername.format(branch="{project}") merely because it prevents something like a job named `branching_sanity` becoming `projecting_sanity`. But buildbot is dying so no big deal
::: taskcluster/taskgraph/transforms/update_verify.py:40
(Diff revision 2)
> + if not chunked["worker"].get("env"):
> + chunked["worker"]["env"] = {}
> + chunked["worker"]["command"] = [
> + "/bin/bash",
> + "-c",
> + "hg clone $BUILD_TOOLS_REPO tools && cd tools && " +
I'd love if we had a better cloning story here (retries/robustcheckout/etc) but I think that should all be followup fodder
Attachment #8934799 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 110•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934808 [details]
bug 1423081 - add shipping-{phase,product} to repackage.
https://reviewboard.mozilla.org/r/205704/#review212660
Attachment #8934808 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 111•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934810 [details]
bug 1423081 - shipping-{phase,product} in signing.
https://reviewboard.mozilla.org/r/205708/#review212662
Attachment #8934810 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 112•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934813 [details]
bug 1423081 - add shipping-{phase,product} to builds.
https://reviewboard.mozilla.org/r/205714/#review212664
A few designations of maple in here, but no worry if they land.
Attachment #8934813 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 113•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934800 [details]
bug 1423081 - desktop release update/final verify support.
https://reviewboard.mozilla.org/r/205688/#review212696
::: taskcluster/ci/release-secondary-final-verify/kind.yml:9
(Diff revision 2)
>
> loader: taskgraph.loader.transform:loader
>
> kind-dependencies:
> - # TODO: this makes us depend on beetmover/balrog for all platforms
> + # TODO: this makes us depend on beetmover/balrog for all platforms
> - # does it matter?
> + # does it matter?
Same comment about figuring out this todo
Attachment #8934800 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 114•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934803 [details]
bug 1423081 - devedition l10n support.
https://reviewboard.mozilla.org/r/205694/#review212698
::: taskcluster/ci/nightly-l10n/kind.yml:89
(Diff revision 2)
> default: firefox
> + linux32-devedition-nightly: devedition
> + linux64-devedition-nightly: devedition
> + macos64-devedition-nightly: devedition
> + win32-devedition-nightly: devedition
> + win64-devedition-nightly: devedition
This is fine, but you could do .*-devedition-.* to specify more concisely
Attachment #8934803 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 115•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934805 [details]
bug 1423081 - add dynamic dep_tasks based on shipping-phase.
https://reviewboard.mozilla.org/r/205698/#review212700
::: taskcluster/taskgraph/transforms/release_deps.py:58
(Diff revision 2)
> # Skip old-id
> if 'old-id' in dep_task.label:
> continue
> + # We can only depend on tasks within the same phase
> + if dep_task.attributes.get('shipping_phase') != phase:
> + continue
I didn't know this was a limitation, does that mean we can't schedule next phase if previous phase isn't competely done?
::: taskcluster/taskgraph/transforms/release_deps.py:65
(Diff revision 2)
> if _get_product(dep_task.task) == product:
> dependencies[dep_task.label] = dep_task.label
> + # this is dumb, but i don't know how to get beetmover to set
> + # shipping-product in a way that _get_product recognizes
> + if dep_task.attributes.get('shipping_product') == product:
> + dependencies[dep_task.label] = dep_task.label
I bet this can be fixed, or at least setup to only be in beetmover transforms. At the least can someone explain the reasoning better?
Attachment #8934805 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 116•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934809 [details]
bug 1423081 - reverse chunk deps.
https://reviewboard.mozilla.org/r/205706/#review212702
::: commit-message-969c1:5
(Diff revision 2)
> +bug 1423081 - reverse chunk deps. r=callek
> +
> +This allows us to funnel large numbers of tasks down to avoid hitting
> +MAX_DEPENDENCIES. I avoided using a morph here because we might break
> +certain cot assumptions.
This is certainly the type of task well suited for a morph. We should as a followup figure out how to get cot to let us do it that way.
::: taskcluster/ci/post-balrog-dummy/kind.yml:32
(Diff revision 2)
> + command:
> + - /bin/bash
> + - -c
> + - echo "Dummy task"
> + treeherder:
> + symbol: Rel(BD)
I'd be ok with the tasks in this commit not even reporting to treeherder.
I suspect that we will end up using https://github.com/taskcluster/taskcluster-rfcs/issues/75 here at some point
::: taskcluster/taskgraph/transforms/reverse_chunk_deps.py:37
(Diff revision 2)
> + return job
> +
> +
> +@transforms.add
> +def add_dependencies(config, jobs):
> + for job in release_deps.add_dependencies(config, jobs):
This is an odd construct, iterating through another transform to handle our transform.
This might need some explanatory text
Attachment #8934809 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 117•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934814 [details]
bug 1423081 - add new docker image tasks.
https://reviewboard.mozilla.org/r/205716/#review212704
Attachment #8934814 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 118•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934807 [details]
bug 1423081 - add release updates.
https://reviewboard.mozilla.org/r/205702/#review212706
::: taskcluster/taskgraph/transforms/release_updates.py:6
(Diff revision 2)
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +"""
> +Transform the beetmover task into an actual task description.
> +"""
Beetmover?
Attachment #8934807 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 119•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934820 [details]
bug 1423081 - release updates builder support.
https://reviewboard.mozilla.org/r/205728/#review212708
::: taskcluster/ci/release-updates-builder/kind.yml:84
(Diff revision 2)
> + worker:
> + properties:
> + # fixme if this is actually used, kill otherwise
> + # revision: ?
> + # fixme if this is actually used - kill me otherwise
> + # script_repo_revision: ?
X2 places, resolve fixmes before landing (it's ok to reference a follow-up bug to resolve)
Attachment #8934820 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 120•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934804 [details]
bug 1423081 - partner_repack support.
https://reviewboard.mozilla.org/r/205696/#review212828
::: taskcluster/taskgraph/transforms/partner_repack.py:18
(Diff revision 2)
> +
> +transforms = TransformSequence()
> +
> +
> +@transforms.add
> +def add_command(config, tasks):
This transform name doesn't seem to match what its doing. (It's not adding any command)
Attachment #8934804 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 121•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934806 [details]
bug 1423081 - snap support.
https://reviewboard.mozilla.org/r/205700/#review212830
Attachment #8934806 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 122•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934815 [details]
bug 1423081 - binary transparency kind.
https://reviewboard.mozilla.org/r/205718/#review212832
Clearing review, since it feels like there is more to do on this one.
::: taskcluster/ci/release-binary-transparency/kind.yml:13
(Diff revision 2)
> + # - taskgraph.transforms.release_deps:transforms
> + - taskgraph.transforms.task:transforms
> +
> +# TODO: once release checksums is done
> +# kind-dependencies:
> +# - release-checksums
This probably shouldn't land until the commented-out bits above this are solved (the transform and the kind-dependency stuff)
Attachment #8934815 -
Flags: review?(bugspam.Callek)
|
|
||
Comment 123•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934816 [details]
bug 1423081 - desktop bouncer sub+aliases.
https://reviewboard.mozilla.org/r/205720/#review212834
Attachment #8934816 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 124•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934818 [details]
bug 1423081 - desktop release-mark-as-shipped support.
https://reviewboard.mozilla.org/r/205724/#review212836
Attachment #8934818 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 125•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934819 [details]
bug 1423081 - release-notify email updates.
https://reviewboard.mozilla.org/r/205726/#review212838
Attachment #8934819 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 126•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934821 [details]
bug 1423081 - desktop uptake monitoring support.
https://reviewboard.mozilla.org/r/205730/#review212840
::: taskcluster/ci/release-uptake-monitoring/kind.yml:84
(Diff revision 2)
> + run:
> + product: fennec
> + buildername: release-{branch}-fennec_uptake_monitoring
> + worker:
> + properties:
> + # TODO: Calculate "platforms" dynamically
can we point # TODO at a bug # instead?
Attachment #8934821 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 127•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934822 [details]
bug 1423081 - desktop release-version-bump support.
https://reviewboard.mozilla.org/r/205732/#review212842
Attachment #8934822 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 128•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934824 [details]
bug 1423081 - desktop tc relpro dev mozharness configs.
https://reviewboard.mozilla.org/r/205736/#review212844
I'm just rubberstamping this one, its only dev configs. (let me know seperately if you need a thorough review here)
Attachment #8934824 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 129•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8935585 [details]
bug 1423081 - add checksums builder.
https://reviewboard.mozilla.org/r/206466/#review212846
Attachment #8935585 -
Flags: review?(bugspam.Callek) → 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 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 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 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 hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 164•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934792 [details]
bug 1423081 - source readme.
https://reviewboard.mozilla.org/r/205672/#review211942
> This is pretty general, and applies to a lot of jobs. I'm a little worried it will get used more widely and become something of a footgun. My original comment on a similar topic was in https://bugzilla.mozilla.org/show_bug.cgi?id=1306598#c3.
>
> Options:
>
> 1. Only substitute `head_repository` and `head_rev` (and document that in the schema in this file). That at least avoids the temptation to substitute any parameter.
> 2. Do this in a separate transform, specific to this kind and applied before the build, build_attrs, and build_lints transforms. Then other users of the mozharness job transforms don't have this functionality available.
> 3. Get this information from env vars in mozharness, as it's [already provided](https://tools.taskcluster.net/groups/LcldJhU0T-mAD0MiSq44rg/tasks/AYX58EBRRze1xk_0LvAKvw/details)
Fixed on maple with (3); will update the review queue during the next round.
|
|
Assignee | |
Comment 165•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934791 [details]
bug 1423081 - add firefox + devedition relpro.
https://reviewboard.mozilla.org/r/205670/#review212370
> We explicitly want the latest graph (rightmost) to take precedence in the case of dup/conflicting keys. Would a comment help here?
Added another comment on maple.
|
|
Assignee | |
Comment 166•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934792 [details]
bug 1423081 - source readme.
https://reviewboard.mozilla.org/r/205672/#review212426
> [x3] We could pull this from the environment rather than transform the values... (I think this was suggested elsewhere)
Yeah, fixed in subsequent pushes to maple.
> this might be safer as `artifact['paths'] = [...]` since we're changing the build target above. And in the beetmover transform itself an '# XXX Unused, see comment in `beetmover_source` transform or some such'
>
> Merely a nit, skippable per author discretion
We're tracking this in both https://trello.com/c/eImDIb4X/188-solve-the-beetmover-source-hack-and-address-it-via-mozharness-instead-of-altering-the-deps-upstreamartifacts and https://trello.com/c/fwJruFlS/186-rewrite-the-beetmover-dependency-system-its-dirty-and-not-flexible-enough-to-meet-our-needs . Both are in cleanup, so I'll drop this for now.
> if we don't change to pulling by environ, can we at least just pass the specific params we know we care about here (or use a seperate, earlier, transform)
Already addressed in environ.
> I'm not really a fan of having to switch this on kind name here, *but* I don't have a better suggestion offhand, so unless you do, we'll keep it.
Yeah, we want to make artifacts and beetmover all data/yaml driven. In cleanup.
|
|
Assignee | |
Comment 167•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934793 [details]
bug 1423081 - target_tasks_methods updates.
https://reviewboard.mozilla.org/r/205674/#review212454
> nit "mozilla_beta" in doc...
That's actually accurate - mozilla_beta_tasks is pretty much shared with mozilla_release. Amended to `mozilla_{beta,release}` for clarity.
> lets genericize the naming here, there is the phrase "beta" and "beta_tasks" type comments/variables a bunch here.
Changed to beta_release_tasks.
> Nomenclature question, what is a "recompression task"
bz2/lzma
|
|
Assignee | |
Comment 168•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934794 [details]
bug 1423081 - inherit shipping-product from upstream.
https://reviewboard.mozilla.org/r/205676/#review212458
> nit: I don't see anything inheriting shipping-phase from upstream here, just product.
Amended in my local review queue; pushing that with the other changes in the next day or two.
|
|
Assignee | |
Comment 169•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934796 [details]
bug 1423081 - add firefox and devedition relpro support to scriptworker.py
https://reviewboard.mozilla.org/r/205680/#review212460
> where would we get "{}" from for this?
The line above: I imagine an empty PARTIAL_UPDATES env var. We check for "" but not "{}" or "[]" above, probably to avoid a json ValueError, but we don't check for an otherwise empty value.
|
|
Assignee | |
Comment 170•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934798 [details]
bug 1423081 - add desktop release beetmover support.
https://reviewboard.mozilla.org/r/205684/#review212506
> nit: var name should be something other than `_` (I'm not a fan of _ for anything expected to be used...)
+1. Renamed to `artifact_extra`
|
|
||
Comment 171•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934811 [details]
bug 1423081 - desktop release support in transforms/task.py
https://reviewboard.mozilla.org/r/205710/#review212870
Attachment #8934811 -
Flags: review?(bugspam.Callek) → review+
|
|
Assignee | |
Comment 172•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934804 [details]
bug 1423081 - partner_repack support.
https://reviewboard.mozilla.org/r/205696/#review212828
> This transform name doesn't seem to match what its doing. (It's not adding any command)
renamed to `resolve_properties`.
|
|
Assignee | |
Comment 173•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934805 [details]
bug 1423081 - add dynamic dep_tasks based on shipping-phase.
https://reviewboard.mozilla.org/r/205698/#review212700
> I didn't know this was a limitation, does that mean we can't schedule next phase if previous phase isn't competely done?
This isn't a limitation; fixed on maple.
> I bet this can be fixed, or at least setup to only be in beetmover transforms. At the least can someone explain the reasoning better?
This is an old comment, but this highlighted another bug. Both addressed on maple.
|
|
Assignee | |
Comment 174•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934807 [details]
bug 1423081 - add release updates.
https://reviewboard.mozilla.org/r/205702/#review212706
> Beetmover?
Addressed on maple.
|
|
Assignee | |
Comment 175•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934815 [details]
bug 1423081 - binary transparency kind.
https://reviewboard.mozilla.org/r/205718/#review212832
> This probably shouldn't land until the commented-out bits above this are solved (the transform and the kind-dependency stuff)
Just some dep issues. Fixed on maple.
|
|
Assignee | |
Comment 176•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934821 [details]
bug 1423081 - desktop uptake monitoring support.
https://reviewboard.mozilla.org/r/205730/#review212840
> can we point # TODO at a bug # instead?
Already a trello card; I can remove the line. https://trello.com/c/tcP7IaoT/199-compute-uptake-monitoring-platforms-dynamically
|
|
Assignee | |
Comment 177•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934811 [details]
bug 1423081 - desktop release support in transforms/task.py
https://reviewboard.mozilla.org/r/205710/#review211848
> nit: update the comment to say that an array of exit status codes is ok.
Fixed on maple.
|
|
Assignee | |
Comment 178•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934811 [details]
bug 1423081 - desktop release support in transforms/task.py
https://reviewboard.mozilla.org/r/205710/#review211848
|
|
Assignee | |
Comment 179•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934811 [details]
bug 1423081 - desktop release support in transforms/task.py
https://reviewboard.mozilla.org/r/205710/#review211848
> what cases will we not have a job-name and still be valid?
Removed the .get() on maple.
|
|
Assignee | |
Comment 180•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934811 [details]
bug 1423081 - desktop release support in transforms/task.py
https://reviewboard.mozilla.org/r/205710/#review211848
> why don't we want to verify here anymore?
Release indexes don't use job-name at all, so verifying it seems redundant / overkill. Once we removed the requirement for release indexes to have a job-name, we added a bunch of tasks without a job-name, so if we want to verify (even though the index doesn't use it) that'll require a non-trivial amount of cleanup.
|
|
Assignee | |
Comment 181•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934811 [details]
bug 1423081 - desktop release support in transforms/task.py
https://reviewboard.mozilla.org/r/205710/#review211848
> I'd rather we have a sanity check/exception here if task['shipping-product'] !== attributes['shipping_product'] when set.
>
> Since doing otherwise would cause confusion in the job-defaults and such. (It may have caused me some confusion in some already-looked at reviews).
Added a sanity check on maple. I'd rather this be nicer, but this should help for now.
|
||
Comment 182•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934799 [details]
bug 1423081 - desktop release update verify support.
https://reviewboard.mozilla.org/r/205686/#review212526
> can use: null | Null | NULL | ~
>
> This will eval out into python `None` as a possible improvement in code/readability here
Probably doesn't matter (either should cause update verify to fail), but that seems more correct.
> since final_verify has a seperate kind for secondary, drop this comment ;-)
+1
> cleanup: lets get this docker image in tree ;-)
Yeah, this is for future. We might just be able to use (and rename) the update-verify docker image as-is, or tweak it slightly.
> solve this todo before landing too.
I don't think we should block on solving this, but maybe we can decide if this less-than-ideal dependency is worth adding to the cleanup list.
> is there a reason we can't use `jobs-from:` and set `job-defaults` appropriately for release-update-verify and the buildbot variant.
>
> To avoid an extra kind here, or do they need drastically different transforms/implementations that it doesn't work out like I think?
I think they're pretty drastically different, because of all the buildbot bridge parts. If the buildbot one was going to be longer lived it might be worth trying to unify them, but I think it's probably going to die in January anyways. I don't think the cost/benefit of overhauling this is worthwhile at this point.
> The majority of this transform could be done as a job/* (as in run: using: buildbot_update_verify) if its easier/clearer. (and if it eases merging both UV kinds together)
This sounds correct (I'll defer to you on that), but like the above I'm not sure it's worth doing at this point. The current thing works, and is going away in the very near term.
> I'd love if we had a better cloning story here (retries/robustcheckout/etc) but I think that should all be followup fodder
Yep, that's for cleanup. It might go away completely when we kill patcher configs, too.
|
|
||
Comment 183•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934820 [details]
bug 1423081 - release updates builder support.
https://reviewboard.mozilla.org/r/205728/#review212708
> X2 places, resolve fixmes before landing (it's ok to reference a follow-up bug to resolve)
These can go, the updates builder is working without them!
|
|
||
Comment 184•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934799 [details]
bug 1423081 - desktop release update verify support.
https://reviewboard.mozilla.org/r/205686/#review212526
> make sure this question in the TODO is figured out before landing ;-)
This is probably something for cleanup. Or maybe we can decide now if we want to bother addressing it.
|
|
||
Comment 185•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934800 [details]
bug 1423081 - desktop release update/final verify support.
https://reviewboard.mozilla.org/r/205688/#review212696
> Same comment about figuring out this todo
This is probably something for cleanup. Or maybe we can decide now if we want to bother addressing it.
|
|
||
Comment 186•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934799 [details]
bug 1423081 - desktop release update verify support.
https://reviewboard.mozilla.org/r/205686/#review212526
> I don't think we should block on solving this, but maybe we can decide if this less-than-ideal dependency is worth adding to the cleanup list.
I was mostly meaning solve the "does it matter" question in some way. If its a "not really" then meh, if its a "it would be better in cleanup" then cleanup it goes, if it matters stronger than that then we block. I just don't want the question itself. lingering
|
|
Assignee | |
Comment 187•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934799 [details]
bug 1423081 - desktop release update verify support.
https://reviewboard.mozilla.org/r/205686/#review212526
> This is probably something for cleanup. Or maybe we can decide now if we want to bother addressing it.
Added https://trello.com/c/C4jHhhqG/231-revisit-buildbot-update-verify-deps ; going to kill the TODO comment.
> Probably doesn't matter (either should cause update verify to fail), but that seems more correct.
Using null breaks graph generation, since it requires a string. Going to wontfix this.
> +1
Done on maple.
> I was mostly meaning solve the "does it matter" question in some way. If its a "not really" then meh, if its a "it would be better in cleanup" then cleanup it goes, if it matters stronger than that then we block. I just don't want the question itself. lingering
Added to https://trello.com/c/C4jHhhqG/231-revisit-buildbot-update-verify-release-final-verify-deps .
> I think they're pretty drastically different, because of all the buildbot bridge parts. If the buildbot one was going to be longer lived it might be worth trying to unify them, but I think it's probably going to die in January anyways. I don't think the cost/benefit of overhauling this is worthwhile at this point.
Dropping due to this comment.
|
|
Assignee | |
Comment 188•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934800 [details]
bug 1423081 - desktop release update/final verify support.
https://reviewboard.mozilla.org/r/205688/#review212696
> This is probably something for cleanup. Or maybe we can decide now if we want to bother addressing it.
Added to https://trello.com/c/C4jHhhqG/231-revisit-buildbot-update-verify-release-final-verify-release-secondary-final-verify-deps .
|
|
Assignee | |
Comment 189•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934820 [details]
bug 1423081 - release updates builder support.
https://reviewboard.mozilla.org/r/205728/#review212708
> These can go, the updates builder is working without them!
Fixed on maple.
|
|
Assignee | |
Comment 190•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934811 [details]
bug 1423081 - desktop release support in transforms/task.py
https://reviewboard.mozilla.org/r/205710/#review211848
> None of these other entries in the worker are utilized from any passed down tasks in maple.
>
> Are they required to be configurable or should we hardcode in the payload builder below to cut out complexity for now?
Removed complexity for now.
> As mentioned above this doesn't actually seem used by entries here.
>
> Of note of course, is that the dict of the job is passed by reference (not copy) [unless something changed outside of my memory] and that resolve_keyed_by replaces values.
>
> So if you're not doing a .deepcopy() you run the risk of having the return of this resolve not being valid against any other possible returns from the same taskgraph. (as in, if you resolve by product, whichever product gets run first will be sticky and you won't get the resolve for next product).
>
> Though as I said, nothing in the current maple branch uses this param, so you get metadata.owner anyway instead ;-)
Resolved on maple with hardcodes, esp since binary-transparency is a dummy worker task atm.
|
|
Assignee | |
Updated•7 years ago
|
Attachment #8934815 -
Flags: review?(bugspam.Callek)
| 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 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 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 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 hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Updated•7 years ago
|
Attachment #8934799 -
Attachment is obsolete: true
|
|
||
Comment 225•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8936668 [details]
bug 1423081 - Add balrog publishing task.
https://reviewboard.mozilla.org/r/207408/#review213286
::: taskcluster/ci/release-balrog-publishing/kind.yml:17
(Diff revision 1)
> +kind-dependencies:
> + - release-uptake-monitoring
> +
> +job-defaults:
> + description: Schedule publishing in balrog
> + worker-type: buildbot-bridge/buildbot-bridge
O, we're not using balrogscriptworker at this stage?
Attachment #8936668 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 226•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934790 [details]
bug 1423081 - add release partials support.
https://reviewboard.mozilla.org/r/205668/#review213288
::: taskcluster/taskgraph/transforms/partials.py:125
(Diff revisions 3 - 4)
> + else:
> + mar_channel_id = 'firefox-mozilla-beta'
> + elif config.params['project'] == 'mozilla-release':
> + mar_channel_id = 'firefox-mozilla-release'
> + elif 'esr' in config.params['project']:
> + mar_channel_id = 'firefox-mozilla-esr'
This feels like the type of thing that should be in the yaml, otherwise we could hit confusion.... but I think this is ok if we have a followup on file.
|
|
||
Comment 227•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934798 [details]
bug 1423081 - add desktop release beetmover support.
https://reviewboard.mozilla.org/r/205684/#review213292
::: taskcluster/taskgraph/transforms/beetmover_repackage.py:361
(Diff revisions 3 - 4)
> for job in jobs:
> if not is_valid_beetmover_job(job):
> - raise NotImplementedError("Beetmover_repackage must have five dependencies.")
> + print(job['dependencies'])
> + raise NotImplementedError(
> + "{}: Beetmover_repackage must have five dependencies.".format(job['label'])
> + )
nit: debugging info in a print...
|
|
||
Comment 228•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934800 [details]
bug 1423081 - desktop release update/final verify support.
https://reviewboard.mozilla.org/r/205688/#review213294
::: commit-message-59eaf:3
(Diff revisions 3 - 4)
> -bug 1423081 - desktop release final verify support. r=callek
> +bug 1423081 - desktop release update/final verify support. r=callek
>
> -MozReview-Commit-ID: 53TNzBXIUYh
> +MozReview-Commit-ID: AJsterLOwYg
This interdiff is harder because https://reviewboard.mozilla.org/r/205686/diff/3#index_header got folded into it....
Let me know if there are any specific changes here you want me to peek at, otherwise I'm just high-level skimming
|
|
||
Comment 229•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934806 [details]
bug 1423081 - snap support.
https://reviewboard.mozilla.org/r/205700/#review213296
::: taskcluster/docker/firefox-snap/runme.sh:82
(Diff revision 4)
> # Upload Beta snaps to Ubuntu Snap Store (No channel)
> # TODO: Add a release channel once ready for broader audience
> # TODO: Don't filter out non-beta releases
> # TODO: Parametrize channel depending on beta vs release
> # TODO: Make this part an independent task
> -if [[ $VERSION =~ ^[0-9]+\.0b[0-9]+$ ]]; then
> +if [ "$PUSH_TO_CHANNEL" != "" ]; then
Would probably be better to detect "edge" here rather than != blank being "edge". So an invalid channel doesn't push (or attempt to push)
|
|
Assignee | |
Comment 230•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934806 [details]
bug 1423081 - snap support.
https://reviewboard.mozilla.org/r/205700/#review213296
> Would probably be better to detect "edge" here rather than != blank being "edge". So an invalid channel doesn't push (or attempt to push)
As mentioned in channel,
a) this is a secret URL. If a non-edge PUSH_TO_CHANNEL is specified, the secret won't exist.
b) if someone adds the non-edge secret to the secrets service and updates the PUSH_TO_CHANNEL to that same non-edge string, likely they'll want to ship. If they don't, they've (accidentally?) taken 2 steps towards shipping but will fail on missing the secrets scope.
c) if they add the non-edge secret to the secrets service, update the PUSH_TO_CHANNEL to that same non-edge string, add the non-edge secret scope to the task definition, they've made 3 ['accidental'?] steps towards shipping, but the decision task will fail for missing the scope.
d) if they do all the above and add the scope to the decision task, I would be very hard pressed to say that person should have been protected from themselves.
Dropping this issue.
|
|
||
Comment 231•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8934815 [details]
bug 1423081 - binary transparency kind.
https://reviewboard.mozilla.org/r/205718/#review213302
Attachment #8934815 -
Flags: review?(bugspam.Callek) → review+
|
|
||
Comment 232•7 years ago
|
||
I've looked through this whole patch queue, and am happy with its outcome. feel free to land once we resolve the last few comments I've had
| 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 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 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 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 hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 267•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934798 [details]
bug 1423081 - add desktop release beetmover support.
https://reviewboard.mozilla.org/r/205684/#review213292
> nit: debugging info in a print...
fixed on maple.
|
|
||
Comment 268•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934790 [details]
bug 1423081 - add release partials support.
https://reviewboard.mozilla.org/r/205668/#review213288
> This feels like the type of thing that should be in the yaml, otherwise we could hit confusion.... but I think this is ok if we have a followup on file.
I looked into this and I don't think it's reasonable to block on it. Note that it's two levels of by- statements that we'd need (one for project, one for devedition vs. firefox on Beta), and I couldn't find a reasonable way of doing this in partials/kind.yml. Adding them to the job-template block does nothing, because that block isn't processed until the task.py transform, which happens after partials.py.
If you have any ideas as to how to improve this, I'm happy to hear them - I'm a bit out of my depth.
| 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 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 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 hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 295•7 years ago
|
||
| mozreview-review-reply | ||
Comment on attachment 8934790 [details]
bug 1423081 - add release partials support.
https://reviewboard.mozilla.org/r/205668/#review213288
> I looked into this and I don't think it's reasonable to block on it. Note that it's two levels of by- statements that we'd need (one for project, one for devedition vs. firefox on Beta), and I couldn't find a reasonable way of doing this in partials/kind.yml. Adding them to the job-template block does nothing, because that block isn't processed until the task.py transform, which happens after partials.py.
>
> If you have any ideas as to how to improve this, I'm happy to hear them - I'm a bit out of my depth.
I was hoping we could be more explicit with our kind.yml files, rather than assume getting all info from upstream will be a good model. I'm thinking we'll drop this until we can address the larger problem.
|
||
Comment 296•7 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.
hg error in cmd: hg rebase -s afecfc0b9704 -d cc95b6d1680b: rebasing 439122:afecfc0b9704 "bug 1423081 - add release partials support. r=Callek"
merging taskcluster/taskgraph/transforms/partials.py
warning: conflicts while merging taskcluster/taskgraph/transforms/partials.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
|
||
Comment 297•7 years ago
|
||
Pushed by asasaki@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/89cf53d0d832
add release partials support. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/77de103de179
add firefox + devedition relpro. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/35d738241395
source readme. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/960aa6859716
target_tasks_methods updates. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/13d6dfaa9bce
inherit shipping-product from upstream. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f06fb0261ce
add generate_bz2_blob to schema identifiers. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/493b05782992
add firefox and devedition relpro support to scriptworker.py r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/72768f79e796
add release balrog support. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0d07b79b766
add desktop release beetmover support. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/58903ea7d352
desktop release update/final verify support. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca91e2d46629
buildbot release worker doesn't need force=True. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/9cc70d4ac5ba
devedition l10n support. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/66991f25b97f
partner_repack support. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/ccf665c924ec
add dynamic dep_tasks based on shipping-phase. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/67616e1ec271
snap support. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/b368b9518bd4
add release updates. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/d30adb030f53
add shipping-{phase,product} to repackage. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/aaf1311fa0e0
reverse chunk deps. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/bb6231dddbbd
shipping-{phase,product} in signing. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/9bee53aca413
desktop release support in transforms/task.py r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/2ea1f5bca8ef
add version.txt to decision sparse checkout. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/54e60b17ae8f
add shipping-{phase,product} to builds. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/0447bd19ce12
add new docker image tasks. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/fe1a244286b9
binary transparency kind. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/1bcb353a5918
desktop bouncer sub+aliases. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed8773e3afff
desktop release-mark-as-shipped support. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/a55797cddea1
release-notify email updates. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/f477a33ac463
release updates builder support. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/64bc707ca4ba
desktop uptake monitoring support. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/bab4575b91e4
desktop release-version-bump support. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f8a8c49a2df
desktop tc relpro taskcluster docs. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f58139ea687
desktop tc relpro dev mozharness configs. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac273544f2a1
add checksums builder. r=callek
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5894798a3a0
Add balrog publishing task. r=callek
|
|
Assignee | |
Comment 298•7 years ago
|
||
Landed as of maple a223695098dd : https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=c5894798a3a00122fa93c15c044116947861f4f4
|
||
Comment 299•7 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/89cf53d0d832
https://hg.mozilla.org/mozilla-central/rev/77de103de179
https://hg.mozilla.org/mozilla-central/rev/35d738241395
https://hg.mozilla.org/mozilla-central/rev/960aa6859716
https://hg.mozilla.org/mozilla-central/rev/13d6dfaa9bce
https://hg.mozilla.org/mozilla-central/rev/3f06fb0261ce
https://hg.mozilla.org/mozilla-central/rev/493b05782992
https://hg.mozilla.org/mozilla-central/rev/72768f79e796
https://hg.mozilla.org/mozilla-central/rev/e0d07b79b766
https://hg.mozilla.org/mozilla-central/rev/58903ea7d352
https://hg.mozilla.org/mozilla-central/rev/ca91e2d46629
https://hg.mozilla.org/mozilla-central/rev/9cc70d4ac5ba
https://hg.mozilla.org/mozilla-central/rev/66991f25b97f
https://hg.mozilla.org/mozilla-central/rev/ccf665c924ec
https://hg.mozilla.org/mozilla-central/rev/67616e1ec271
https://hg.mozilla.org/mozilla-central/rev/b368b9518bd4
https://hg.mozilla.org/mozilla-central/rev/d30adb030f53
https://hg.mozilla.org/mozilla-central/rev/aaf1311fa0e0
https://hg.mozilla.org/mozilla-central/rev/bb6231dddbbd
https://hg.mozilla.org/mozilla-central/rev/9bee53aca413
https://hg.mozilla.org/mozilla-central/rev/2ea1f5bca8ef
https://hg.mozilla.org/mozilla-central/rev/54e60b17ae8f
https://hg.mozilla.org/mozilla-central/rev/0447bd19ce12
https://hg.mozilla.org/mozilla-central/rev/fe1a244286b9
https://hg.mozilla.org/mozilla-central/rev/1bcb353a5918
https://hg.mozilla.org/mozilla-central/rev/ed8773e3afff
https://hg.mozilla.org/mozilla-central/rev/a55797cddea1
https://hg.mozilla.org/mozilla-central/rev/f477a33ac463
https://hg.mozilla.org/mozilla-central/rev/64bc707ca4ba
https://hg.mozilla.org/mozilla-central/rev/bab4575b91e4
https://hg.mozilla.org/mozilla-central/rev/6f8a8c49a2df
https://hg.mozilla.org/mozilla-central/rev/2f58139ea687
https://hg.mozilla.org/mozilla-central/rev/ac273544f2a1
https://hg.mozilla.org/mozilla-central/rev/c5894798a3a0
You need to log in
before you can comment on or make changes to this bug.
Description
•