1397847 - Find toolchains in `./mach artifact toolchain --from-build` using indexes

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Dustin J. Mitchell [:dustin] (he/him)]

In bug 1383880 I came across some code that seems to be trying to fake out parts of the taskgraph generation to determine the taskId of an artifact build.  This is fragile as it relies on implementation details of the task-graph generation code.

A slightly easier option would be to download the most recent decision task's artifacts and use those to find the desired task.  That's what I'll try to implement in bug 1383880.

A *much* easier option would be to just index those tasks at a route that can be reproduced from the given inputs, and look it up at that index path.

Comment 1

[Mike Hommey [:glandium]]

> A *much* easier option would be to just index those tasks at a route that can be reproduced from the given inputs, and look it up at that index path.

That's exactly what the code *currently* does, by "reusing" the taskcluster code. Except the taskcluster API doesn't have an API for that, so there's some abuse going on.

Comment 2

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: Bug 1397847: allow jobs-from to be topsrcdir-relative, move toolchains;

This moves the toolchain definitions out of the taskcluster/ci tree, in
preparation for making them useful outside of task-graph generation.

Review commit: https://reviewboard.mozilla.org/r/180030/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180030/

Comment 3

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: Bug 1397847: rename 'toolchain' transform to 'use_toolchains';

Review commit: https://reviewboard.mozilla.org/r/180032/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180032/

Comment 4

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: Bug 1397847: make toolchain definitions a bit more generic;

This makes the toolchain definitions a little more generic and useful for other
purposes, but includes a transform that turns them back into toolchain jobs.

Review commit: https://reviewboard.mozilla.org/r/180034/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180034/

Comment 5

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: Bug 1397847: build common code for finding and hashing toolchains;

And use this code to hash for optimization.

This indexes tasks under both the primary name and aliases for ease of later
discovery.

Review commit: https://reviewboard.mozilla.org/r/180036/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180036/

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: Bug 1397847: use hashing and the index to download toolchains;

Review commit: https://reviewboard.mozilla.org/r/180038/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180038/

Comment 7

[Dustin J. Mitchell [:dustin] (he/him)]

tl;dr is this:
 1. pull toolchain definitions out of the taskgraph config
 2. standardize hashing toolchains
 3. use that in taskgraph and in ./mach artifact toolchain

Comment 8

[Mike Hommey [:glandium]]

While your approach works for this usecase, it's not sustainable for other similar things. Such as bug 1356529, which would then require to pull definitions out of toolchains/ci/build/ which doesn't seem reasonable. Or a helper command to download docker images.

Comment 9

[Dustin J. Mitchell [:dustin] (he/him)]

Bug 1356529 hasn't seen any action for five months. I'm going to push back on scope-creeping this bug to cover that work. My work here is to maintain current functionality while fixing bustage caused by the "abuse" you referenced in comment 1.

By the way, there is a helper command to download docker images, already.

Comment 10

[Dustin J. Mitchell [:dustin] (he/him)]

Attachment: Bug 1397847: --from-builds is a dev tool, not for automation;

Review commit: https://reviewboard.mozilla.org/r/180272/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180272/

Comment 11

[Mike Hommey [:glandium]]

Comment on attachment 8908409 [details]
Bug 1397847: allow jobs-from to be topsrcdir-relative, move toolchains;

https://reviewboard.mozilla.org/r/180030/#review186354

I don't think this is desirable. TC job definitions shouldn't spread outside the taskcluster/ directory.

Comment 12

[Mike Hommey [:glandium]]

Comment on attachment 8908410 [details]
Bug 1397847: rename 'toolchain' transform to 'use_toolchains';

https://reviewboard.mozilla.org/r/180032/#review186356

This is nice but unrelated. It should go in its own bug.

Comment 13

[Mike Hommey [:glandium]]

Comment on attachment 8908411 [details]
Bug 1397847: make toolchain definitions a bit more generic;

https://reviewboard.mozilla.org/r/180034/#review186358

Nice simplifications, but they should go in their own bug (well, maybe they could share a separate bug with the previous patch).

::: taskcluster/taskgraph/transforms/job/toolchain.py:46
(Diff revision 1)
> -    Optional('toolchain-alias'): basestring,
>  })
>  
>  
>  def add_optimizations(config, run, taskdesc):
> -    files = list(run.get('resources', []))
> +    files = []  # fixed in later commits..

huh

::: taskcluster/taskgraph/transforms/job/toolchain.py:165
(Diff revision 1)
>      attributes = taskdesc.setdefault('attributes', {})
> -    attributes['toolchain-artifact'] = run['toolchain-artifact']
>      if 'toolchain-alias' in run:
>          attributes['toolchain-alias'] = run['toolchain-alias']

Seems like this should all go, like fot docker-worker.

::: taskcluster/taskgraph/transforms/toolchain.py:101
(Diff revision 1)
> +        run_on = tcd['run-on']
> +        if run_on == 'linux':
> +            job['worker-type'] = 'aws-provisioner-v1/gecko-{level}-b-linux'
> +            job['worker'] = {
> +                'docker-image': {'in-tree': 'desktop-build'},
> +                'max-run-time': 36000,

It would be good to be able to override this. Particularly, these values are cargo culted and wrong, and we should change them, possibly to different values depending on the job. Bug 1391427 defines some values that are not 36000 for instance.

Comment 14

[Mike Hommey [:glandium]]

Comment on attachment 8908412 [details]
Bug 1397847: build common code for finding and hashing toolchains;

https://reviewboard.mozilla.org/r/180036/#review186360

I'm not particularly convinced by this. I still think we should be able to get useful partial task graphs through mach taskgraph or some direct API.

::: taskcluster/taskgraph/transforms/job/toolchain.py:63
(Diff revision 1)
> +    if 'toolchain-alias' in toolchain:
> +        subs['name'] = toolchain['toolchain-alias']
> +        routes.append('index.{}'.format(TOOLCHAIN_INDEX.format(**subs)))

I can see why you're adding this, but this is a separate matter, it should be in a separate patch or even bug.

Comment 15

[Mike Hommey [:glandium]]

Comment on attachment 8908413 [details]
Bug 1397847: use hashing and the index to download toolchains;

https://reviewboard.mozilla.org/r/180038/#review186362

::: python/mozbuild/mozbuild/mach_commands.py:1899
(Diff revision 1)
>                               'Could not find a toolchain build named `{build}`')
>                      return 1
> +                toolchain = toolchains[b]
> +                digest = hash_toolchain(toolchains, b)
>  
> -                task_id = optimize_task(task, {})
> +                index_path = 'gecko.cache.level-{level}.toolchains.v1.{name}.{digest}'

This is the kind of thing that I didn't want to have duplicated between mach_commands and the taskcluster code. Plus, this doesn't try all levels above MOZ_SCM_LEVEL like the toolchain optimization code does.

::: taskcluster/taskgraph/util/taskcluster.py:85
(Diff revision 1)
> +    except requests.exceptions.HTTPError as e:
> +        if e.response.status_code == 404:
> +            raise KeyError
> +        raise

Seems like a nice api change, that could be done separately.

Comment 16

[Mike Hommey [:glandium]]

Comment on attachment 8908609 [details]
Bug 1397847: --from-builds is a dev tool, not for automation;

https://reviewboard.mozilla.org/r/180272/#review186364

Fair enough

::: commit-message-19b51:1
(Diff revision 1)
> +Bug 1397847: --from-builds is a dev tool, not for automation; r?glandium

--from-build (no s)

Comment 17

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8908412 [details]
Bug 1397847: build common code for finding and hashing toolchains;

https://reviewboard.mozilla.org/r/180036/#review186360

I think ahal's work on tryselect for `mach try` is the right way to do this -- generate and cache a full taskgraph.  That might need a slight bit of generalization to make it usable here, but it's *very* close to what you need.  Callek was interested in such a thing, too.  You're right that moving task configuration out of taskcluster/ci starts to set a bad precedent, especially if we move build definitions out.

Comment 18

[Tom Prince [:tomprince]]

Attachment: Bug 1397847: Add supported API to get tasks of a given kind from a taskgraph; r?dustin

`mach artifact toolchain` gets task definitions from taskgraph, to get the
index path to find the artifacts at. Now that these index paths depend on the
digests of fetch tasks, those kinds need to be loaded as well. This adds a
supported API to get task definitions for a given kind, which loads all the
kind dependencies.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/393f02fb1ad7
Add supported API to get tasks of a given kind from a taskgraph; r=dustin

Comment 20

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/393f02fb1ad7

Comment 21

[Tom Prince [:tomprince]]

Attachment: Bug 1397847: Pass the taskcluster root to `load_tasks_for_kind` in `mach artifact toolchain`; r?dustin

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/19480c535117
Pass the taskcluster root to `load_tasks_for_kind` in `mach artifact toolchain`; r=dustin a=tomprince

Comment 23

[Sylvestre Ledru [:Sylvestre]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=276cdd9b46895f3a3f695ad348f3b3af1bbc0597

Comment 24

[Tom Prince [:tomprince]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/4cf0b2332bce
https://hg.mozilla.org/releases/mozilla-esr60/rev/ba9634bec13e