now that "Bug 1286075 - Factor builds et al. out of legacy kind" is done, we should switch over to use that :)

Comment on attachment 8791892 [details] Bug 1302590 - remove nightly-fennec kind. use build kind for nightlies, kmoir, callek: first draft is up for new approach using the new build kind and all of the other changes that came with it. Going with something like this should be an improvement from before and signing should be easier for desktop. aki: you mentioned you were interested in what state the graph was in. Once this is finalized and stamped, it will be the new way of generating nightly builds and tasks. Again, let me know if you have questions for testing COT

Comment on attachment 8791892 [details] Bug 1302590 - remove nightly-fennec kind. use build kind for nightlies, https://reviewboard.mozilla.org/r/79178/#review77780 This is fine to land as-is, and I'm happy to re-review with the `nightly` attribute added (or some variation on that theme). ::: taskcluster/ci/build/android.yml:76 (Diff revision 1) > script: "mozharness/scripts/fx_desktop_build.py" > custom-build-variant-cfg: api-15 > tooltool-downloads: internal > > +android-api-15-nightly/opt: > + description: "Android 4.0 API15+ Nightly" If you add `attributes: nightly: true` here that would make the filtering easier. Please also add the attribute to `taskcluster/docs/attributes.rst`. ::: taskcluster/ci/build-signing/android-signing.yml:19 (Diff revision 1) > - maxRunTime: 600 > + maxRunTime: 600 > - metadata: > + metadata: > - name: "Signing Scriptworker Task" > + name: "Signing Scriptworker Task" > - description: "Testing the signing scriptworker" > - owner: "amiyaguchi@mozilla.com" > + description: "Sign Android Build Tasks" > + owner: "jlund@mozilla.com" > - source: "https://tools.taskcluster.net/task-creator/" > + source: "https://tools.taskcluster.net/task-creator/" Ideally this metadata would come from a transform or from the kind implementation (but it's fine to add that later)

Thanks Jordan for your work on this, I'll refactor my desktop patches to reflect these changes.

Comment on attachment 8791892 [details] Bug 1302590 - remove nightly-fennec kind. use build kind for nightlies, https://reviewboard.mozilla.org/r/79178/#review77828 Ah, I missed the commit comments. 1) I don't know if nightly counts as a platform variant or whatever it is we call the things after the slash (opt, debug, asan, tsan, msan, pgo, ccov, jscov, etc.). Probably better to leave the platform stuff alone for now. 2) I'm not sure what you mean by "many of these few lines". That said, the transforms can easily "clone" a task. We do it with e10s [here](https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/tests/desktop_test.py#59) for example. I'll definitely look for an implementation of the latter when the second nightly build stanza lands, but for this patch I'm so delighted to see nightly-fennec go away that I'm happy to look the other way :)

Looks good to me, dustin's points apply to my own mindset as well.

Thanks! I'm mainly going to have to: - set task.payload.features.chainOfTrust to true in the decision task (always), artifact image builder task (always), all build tasks that need signing (probably always, but possibly only for nightly), and l10n tasks that need signing (probably always, but possibly only for nightly). Later on I may have to turn this on in other tasks that need signing, like update mar creation steps. - add the dependency task id to the signing task's dependency list. - if we know the artifact image builder task's task id, add it to the signing task's dependency list. This is likely in a different graph, so figuring this out might be tricky.

I'm not sure what the "artifact image builder task" is -- a docker-image task? If I understand correctly, it's likely to be in a different graph because it's already been built -- that's what optimization does. Basically, you put a new task in the full task graph, and then during the optimization phase you replace that with an existing task (usually from another graph).

Yes, a docker image task. Does that mean the optimization phase will insert the task id of the previous graph's docker image task into the signing task's dependency list? If so, awesome.

(In reply to Aki Sasaki [:aki] from comment #7) > - add the dependency task id to the signing task's dependency list. This should read the 'decision' task id.

Yes, that's exactly what it means :)

Comment on attachment 8791892 [details] Bug 1302590 - remove nightly-fennec kind. use build kind for nightlies, https://reviewboard.mozilla.org/r/79178/#review77828 oh okay. I'll go with the clone approach in follow up bug

Comment on attachment 8791892 [details] Bug 1302590 - remove nightly-fennec kind. use build kind for nightlies, https://reviewboard.mozilla.org/r/79178/#review77780 > Ideally this metadata would come from a transform or from the kind implementation (but it's fine to add that later) I'll fix this in follow up along with the e10s task.clone() style to create nightly tasks based on existing stanzas.

Comment on attachment 8791892 [details] Bug 1302590 - remove nightly-fennec kind. use build kind for nightlies, don't think I can re-r? you in mozreview so setting it here with the inter-diff patch: https://reviewboard.mozilla.org/r/79178/diff/1-2/

Comment on attachment 8791892 [details] Bug 1302590 - remove nightly-fennec kind. use build kind for nightlies, https://reviewboard.mozilla.org/r/79178/#review78250 A bunch of comments here, but none are serious issues. Assuming that you're going to continue to hack on the build-signing kind and its implementation, I think this can land and the comments can be handled in subsequent work. ::: taskcluster/ci/build-signing/android-signing.yml:26 (Diff revision 2) > + nightly: true > + unsigned-task: > + label: "build-android-api-15-nightly/opt" > + artifacts: > + - "public/build/target.apk" > + - "public/build/en-US/target.apk" This seems weird -- the "task" part of this YAML looks like it would be about the same for all signing operations, but then this section is specific to a particular build, named by label. I'm happy to wait to see what happens when you add a second signing thing in here, though. I'd hate to see that "task" but repeated in dozens of files. ::: taskcluster/taskgraph/task/signing.py:34 (Diff revision 2) > - fennec_tasks = [t for t in loaded_tasks if t.attributes.get('kind') == 'nightly-fennec'] > > tasks = [] > - for fennec_task in fennec_tasks: > + for filename in config.get('jobs-from', []): > templates = Templates(root) > - task = templates.load('signing.yml', {}) > + jobs = templates.load(filename, {}) This is a terminology thing, but I think it can mess people up: these are task definitions you're working with here, while "job" in this context suggests a "job description" -- a totally different thing. It would probably help the next person to come along if you name the config `tasks-from`, and use `tasks` as the variable here. ::: taskcluster/taskgraph/task/signing.py:38 (Diff revision 2) > templates = Templates(root) > - task = templates.load('signing.yml', {}) > + jobs = templates.load(filename, {}) > > - artifacts = ['public/build/target.apk', > - 'public/build/en-US/target.apk'] > - for artifact in artifacts: > + for name, job in jobs.iteritems(): > + for artifact in job['unsigned-task']['artifacts']: > + url = ARTIFACT_URL.format('<{}>'.format('unsigned-artifact'), artifact) or just, `ARTIFACT_URL.format('<{unsigned-artifact}>', artifact)` ::: taskcluster/taskgraph/task/signing.py:39 (Diff revision 2) > - task = templates.load('signing.yml', {}) > + jobs = templates.load(filename, {}) > > - artifacts = ['public/build/target.apk', > - 'public/build/en-US/target.apk'] > - for artifact in artifacts: > - url = ARTIFACT_URL.format('<build-nightly-fennec>', artifact) > + for name, job in jobs.iteritems(): > + for artifact in job['unsigned-task']['artifacts']: > + url = ARTIFACT_URL.format('<{}>'.format('unsigned-artifact'), artifact) > + job['task']['payload']['unsignedArtifacts'].append({ This is getting on toward something that could be accomplished with a transform: the YAML would specify a list of artifact names on the upstream task, and the transform would set up the payload like this. This is fine for now, but next time you return to this bit of code and add another tweak, think of transforms :) ::: taskcluster/taskgraph/task/signing.py:49 (Diff revision 2) > - attributes=attributes)) > - > return tasks > > def get_dependencies(self, taskgraph): > - return [('build-nightly-fennec', 'build-nightly-fennec')] > + return [(self.unsigned_artifact_label, 'unsigned-artifact')] Another minor terminology issue here: this is referring to the task, not the artifacts, so the dependency name "unsigned-artifact" feels a bit weird. How about "unsigned-build-task"? That still seems a little clunky but at least it's more descriptive..

https://hg.mozilla.org/integration/mozilla-inbound/rev/1b269fb8a704a1a1fd2fe67543b9a083ce155563 Bug 1302590 - remove nightly-fennec kind. use build kind for nightlies, r=dustin

(In reply to Dustin J. Mitchell [:dustin] from comment #16) > Comment on attachment 8791892 [details] > Bug 1302590 - remove nightly-fennec kind. use build kind for nightlies, > > https://reviewboard.mozilla.org/r/79178/#review78250 > > A bunch of comments here, but none are serious issues. Assuming that you're > going to continue to hack on the build-signing kind and its implementation, > I think this can land and the comments can be handled in subsequent work. > thanks! tracking all follow-ups here: "Bug 1303894 - clean up build-signing kind, create nightly build based on opt equivalent"

Backed out for Gecko Decision Task bustage, e.g., https://treeherder.mozilla.org/logviewer.html#?job_id=36104306&repo=mozilla-inbound#L94

Backout by ihsiao@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d82fb090d690 Backed out changeset 1b269fb8a704 for Gecko Decision Task bustage

patch to fix decision tree breakage

Comment on attachment 8792899 [details] [diff] [review] whitelist.patch Review of attachment 8792899 [details] [diff] [review]: ----------------------------------------------------------------- thanks! I should have known I'd bitrot in 1 day :| guess this whitelist did its job though :) I've combined your patch into mine for a relanding

https://hg.mozilla.org/integration/mozilla-inbound/rev/33d031e3454e1bdd1bb94ada0b918c48d7935067 Bug 1302590 - remove nightly-fennec kind. use build kind for nightlies, r=dustin