taskgraph: `run-task` ignores git tag
Categories
(Release Engineering :: General, defect, P1)
Tracking
(Not tracked)
People
(Reporter: jlorenzo, Assigned: jlorenzo)
References
Details
(Keywords: regression)
Attachments
(7 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
62 bytes,
text/x-github-pull-request
|
Details | Review | |
49 bytes,
text/x-github-pull-request
|
Details | Review | |
60 bytes,
text/x-github-pull-request
|
Details | Review | |
54 bytes,
text/x-github-pull-request
|
Details | Review | |
42 bytes,
text/x-github-pull-request
|
Details | Review |
Sebastian just noticed Android-Components releases are built off the master branch[1] even though the decision task specifies the following payload:
"env": {
"MOBILE_HEAD_REF": "master",
"MOBILE_HEAD_REPOSITORY": "https://github.com/mozilla-mobile/android-components",
"MOBILE_HEAD_REV": "v21.0.0",
"MOBILE_HEAD_TAG": "v21.0.0",
}
I initially thought it's a fallout of the Taskgraph migration, but the first release that has taskgraph enabled was AC 16.0.0, and it didn't have this flaw[3]. After looking more closely, the first busted AC is 18.0.0[4] because the taskgraph image was bumped in [5]. This bump introduced that patch[6], which exposes HEAD_REF
to any run-task
task (including AC build tasks). It turns out run-task
prefers HEAD_REF
whenever it's specified[7].
Tom, I'm not sure what's the right fix, here. I see several ways, but none of them seems ideal. Do you have an idea?
Impact if we don't fix it: Android-Components cannot ship any dot releases.
[1] https://firefox-ci-tc.services.mozilla.com/tasks/c4wJXJsMT_2Oj2Eg8CMVUg/runs/0/logs/https%3A%2F%2Ffirefox-ci-tc.services.mozilla.com%2Fapi%2Fqueue%2Fv1%2Ftask%2Fc4wJXJsMT_2Oj2Eg8CMVUg%2Fruns%2F0%2Fartifacts%2Fpublic%2Flogs%2Flive.log#L23
[2] https://firefox-ci-tc.services.mozilla.com/tasks/e8Sgva6iSM-8VOEnwEsc-A
[3] https://tools.taskcluster.net/groups/ewYBlP_eQ9euNKnMmbKfIQ/tasks/LL2FrM7ESBeK2c0jpsDSoA/runs/0/logs/public%2Flogs%2Flive_backing.log#L31
[4] https://tools.taskcluster.net/groups/bA5FnaiLQjePvat8593PWw/tasks/bA5FnaiLQjePvat8593PWw/runs/0/logs/public%2Flogs%2Flive_backing.log#L18
[5] https://github.com/mozilla-mobile/android-components/pull/4745/files#diff-ac0229d1171c0983b27003af4c7441a5R10
[6] https://hg.mozilla.org/ci/taskgraph/rev/faeb33c29200827783e3244ea7c5b8bafa9b9f72
[7] https://hg.mozilla.org/ci/taskgraph/file/bbd657c72f8a5fb113f14ca0a8bd02d9cbd06189/src/taskgraph/run-task/run-task#l446
Comment 1•4 years ago
|
||
It looks like the logic in run-task
for git is backwards. It should always prefer HEAD_REV
to HEAD_REF
, if the former is specified.
Assignee | ||
Comment 2•4 years ago
|
||
Sounds good! I'll put a patch up tomorrow.
Assignee | ||
Comment 3•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Comment 4•4 years ago
|
||
Assignee | ||
Comment 5•4 years ago
|
||
Assignee | ||
Comment 6•4 years ago
|
||
Assignee | ||
Comment 7•4 years ago
|
||
Assignee | ||
Comment 8•4 years ago
|
||
Assignee | ||
Comment 9•4 years ago
|
||
All the above patches have landed. Aki, Callek, I think the XPI and the Guardian repos will likely need this bump too. Otherwise, Github releases don't ship the right code and tasks against master will pick whatever is at the top of the branch, making the jobs non-idempotent.
Updated•4 years ago
|
Assignee | ||
Comment 11•4 years ago
|
||
Closing bug because all known projects using taskgraph are either migrated or not impacted by the problem.
Comment 12•4 years ago
|
||
Any chance this could get pushed to mozillareleases/taskgraph:decision
as well?
Assignee | ||
Comment 13•4 years ago
|
||
This is done. I just pushed[1] the latest decision image built by our infrastructure[2].
[1] https://hub.docker.com/layers/mozillareleases/taskgraph/decision-d9fab4448ee5e00b0a29825c3f0609af957279daf102547d715414782710ef06/images/sha256-79eb469838621168a6364476b96850fc9f2d353686195c010ad078fdcf29568e?context=explore
[2] https://treeherder.mozilla.org/#/jobs?repo=taskgraph&revision=7dde9ce7740068d7b73218976343a28fc99be847&selectedTaskRun=LejeLYQ7QC-yuUfX9YgKhw.0
Comment 14•4 years ago
|
||
Thanks Johan!
Comment 15•4 years ago
|
||
The decision task for mobile still wasn't using the new run-task. I've pushed an updated decision-mobile image:
mozillareleases/taskgraph:decision-mobile-6607973bc60e32323a541861cc5856cd6a0f51ea9fd664ef7d43bca8df53db47@sha256:8c471aacc469ea8e7bb4846c16efe086f7350a5cc1df570cc6c86b22895a2456
Comment 16•4 years ago
|
||
I got through some of my local repos so far, I still have the following, which I'll do in the AM
./firefox-ios/.taskcluster.yml
./guardian-vpn-windows/.taskcluster.yml
./reference-browser/.taskcluster.yml
./scriptworker-scripts/.taskcluster.yml
I created PR's for:
adhoc-signing
android-l10n-tooling
fenix
android-components
Comment 17•4 years ago
|
||
don't forget us! :) I'll make the PR though.
Comment 18•4 years ago
|
||
oh, wait, "for mobile" is different, never mind!
Comment 19•4 years ago
|
||
Comment 20•4 years ago
|
||
(In reply to Justin Wood (:Callek) from comment #16)
I got through some of my local repos so far, I still have the following, which I'll do in the AM
./firefox-ios/.taskcluster.yml
./guardian-vpn-windows/.taskcluster.yml
./reference-browser/.taskcluster.yml
./scriptworker-scripts/.taskcluster.yml
These are now created as are:
- app-services
- glean
- firefox-tv
- mozilla-extensions/** (xpi-manifest, xpi-template, and others)
Comment 21•4 years ago
|
||
(In reply to Dustin J. Mitchell [:dustin] (he/him) from comment #17)
don't forget us! :) I'll make the PR though.
There are non-mobile decision updates in my set here, for example: https://github.com/mozilla-services/guardian-vpn-windows/pull/123
So if there are things you need to change by all means do so!
Comment 22•4 years ago
|
||
I think we're up-to-date for this change, but please keep us on the list for next time :)
Assignee | ||
Comment 23•3 years ago
|
||
I published a new mobile image and bumped it in Fenix[1], as part of bug 1635488. I don't remember anybody hitting this issue over the past 4 months. Reclosing this bug. Feel free to reopen if needed.
Comment hidden (collapsed) |
Description
•