Closed Bug 1385055 Opened 2 years ago Closed 2 years ago

Allow `mach taskgraph decision` to create graphs that checkout comm-central.

Categories

(Firefox Build System :: Task Configuration, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla58

People

(Reporter: tomprince, Assigned: tomprince)

References

Details

Attachments

(2 files, 1 obsolete file)

Add support to taskgraph for passing --comm-checkout to taskcluster/docker/recipes/run-task added in Bug 1384210.
Comment on attachment 8891026 [details]
Bug 1385055 - Add some taskgraph parameters for dealing with comm-central and related repositories.

https://reviewboard.mozilla.org/r/162214/#review175140
Attachment #8891026 - Flags: review?(dustin) → review+
Comment on attachment 8891027 [details]
Bug 1385055 - Add taskgraph support for checking out comm-central and related branches.

https://reviewboard.mozilla.org/r/162216/#review175146

::: taskcluster/taskgraph/transforms/job/common.py:82
(Diff revision 2)
> +        taskdesc['worker']['env'].update({
> +            'COMM_BASE_REPOSITORY': config.params['comm_base_repository'],
> +            'COMM_HEAD_REF': config.params['comm_head_rev'],
> +            'COMM_HEAD_REPOSITORY': config.params['comm_head_repository'],
> +            'COMM_HEAD_REV': config.params['comm_head_rev'],
> +        })

For both this and the one below, I'd like to see an exception raised if either the params are defined and not run.comm-checkout *or* params are not defined and run.comm-checkout.

::: taskcluster/taskgraph/transforms/job/common.py:122
(Diff revision 2)
> +    if 'comm_base_repository' in config.params:
> +        taskdesc['worker']['env'].update({
> +            'COMM_BASE_REPOSITORY': config.params['comm_base_repository'],
> +            'COMM_HEAD_REPOSITORY': config.params['comm_head_repository'],
> +            'COMM_HEAD_REV': config.params['comm_head_rev'],
> +        })

no COMM_HEAD_REF here?
Attachment #8891027 - Flags: review?(dustin) → review+
Comment on attachment 8891027 [details]
Bug 1385055 - Add taskgraph support for checking out comm-central and related branches.

https://reviewboard.mozilla.org/r/162216/#review175146

> For both this and the one below, I'd like to see an exception raised if either the params are defined and not run.comm-checkout *or* params are not defined and run.comm-checkout.

We can't raise if the params are definied but not `run.comm-checkout`, as that will happen in the case that this is a test task that isn't checking out gecko or comm. I'll file a bug for raising an exception in the other case; I'd rather not add it yet, since I'm not sure that `run.comm-checkout` is the only thing that should trigger that exception.

> no COMM_HEAD_REF here?

I'm just copying the code for `GECKO_*` above. From [here](http://gecko.readthedocs.io/en/latest/taskcluster/taskcluster/parameters.html#push-information]), `*_REF` is only relevant to git, which `run-task` doesn't support, so it isn't passed here.
Keywords: checkin-needed
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 99bef9c12f0a -d 2f78aad3c907: rebasing 414551:99bef9c12f0a "Bug 1385055 - Add some taskgraph parameters for dealing with comm-central and related repositories. r=dustin"
merging taskcluster/docs/parameters.rst
merging taskcluster/mach_commands.py
merging taskcluster/taskgraph/decision.py
merging taskcluster/taskgraph/parameters.py
warning: conflicts while merging taskcluster/docs/parameters.rst! (edit, then use 'hg resolve --mark')
warning: conflicts while merging taskcluster/taskgraph/parameters.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
Attachment #8891026 - Attachment is obsolete: true
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/314ffe5d7d40
Add taskgraph support for checking out comm-central and related branches. r=dustin
Keywords: checkin-needed
Comment on attachment 8891027 [details]
Bug 1385055 - Add taskgraph support for checking out comm-central and related branches.

https://reviewboard.mozilla.org/r/162212/#review191962

::: taskcluster/taskgraph/transforms/job/common.py:121
(Diff revisions 3 - 4)
>          taskdesc['worker']['env'].update({
>              'COMM_BASE_REPOSITORY': config.params['comm_base_repository'],
>              'COMM_HEAD_REPOSITORY': config.params['comm_head_repository'],
>              'COMM_HEAD_REV': config.params['comm_head_rev'],
>          })
> -    elif job['run']['comm-checkout']:
> +    elif job['run'].get('comm-checkout', False):

I made this check (but not the one for `run-task`) conditional, since this appears to be called from many places.

::: taskcluster/taskgraph/transforms/job/mach.py:31
(Diff revision 4)
>      # defer to the run_task implementation
>      run['command'] = 'cd /builds/worker/checkouts/gecko && ./mach ' + run['mach']
> -    run['checkout'] = True
> +    run['using'] = 'run-task'
> -    run['sparse-profile'] = None
>      del run['mach']
> -    if job['worker']['implementation'] == 'docker-worker':
> +    configure_taskdesc_for_run(config, job, taskdesc, job['worker']['implementation'])

I changed this to call the dispatcher so that it will verify the schema aganst what `run-task` expects and, in particular, apply default values from the schema.
Flags: needinfo?(mozilla)
Attachment #8891027 - Flags: review+ → review?
Attachment #8891027 - Flags: review? → review?(dustin)
Comment on attachment 8891027 [details]
Bug 1385055 - Add taskgraph support for checking out comm-central and related branches.

https://reviewboard.mozilla.org/r/162216/#review191964
Attachment #8891027 - Flags: review?(mozilla)
Attachment #8915732 - Flags: review?(dustin)
Attachment #8891027 - Flags: review?
Attachment #8891027 - Flags: review? → review?(dustin)
Comment on attachment 8891027 [details]
Bug 1385055 - Add taskgraph support for checking out comm-central and related branches.

https://reviewboard.mozilla.org/r/162216/#review192794
Attachment #8891027 - Flags: review?(dustin) → review+
Comment on attachment 8915732 [details]
Bug 1385055 - Add some taskgraph parameters for dealing with comm-central and related repositories.

https://reviewboard.mozilla.org/r/186930/#review192796

::: taskcluster/docs/parameters.rst:131
(Diff revision 2)
>     which will print to the console by default.
> +
> +Comm Push Information
> +---------------------
> +
> +These parameters correspond to the repository and revision of the comm

Is this typically referred to as just "comm" or "comm-central" or "communicator"?
Attachment #8915732 - Flags: review?(dustin) → review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/044f0f7d3af5
Add some taskgraph parameters for dealing with comm-central and related repositories. r=dustin
https://hg.mozilla.org/integration/autoland/rev/5bde30d8db94
Add taskgraph support for checking out comm-central and related branches. r=dustin
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/044f0f7d3af5
https://hg.mozilla.org/mozilla-central/rev/5bde30d8db94
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Product: TaskCluster → Firefox Build System
You need to log in before you can comment on or make changes to this bug.