Closed Bug 1595808 Opened 4 years ago Closed 3 years ago

taskgraph: `run-task` ignores git tag

Categories

(Release Engineering :: General, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jlorenzo, Assigned: jlorenzo)

References

Details

(Keywords: regression)

Attachments

(7 files)

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

Flags: needinfo?(mozilla)

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.

Flags: needinfo?(mozilla)

Sounds good! I'll put a patch up tomorrow.

Assignee: nobody → jlorenzo
Attachment #9108154 - Attachment description: Bug 1595808 - git_checkout(): Use HEAD_REV first then HEAD_REF r=mhentges → Bug 1595808 - part 1: git_checkout() should use HEAD_REV first then HEAD_REF r=mhentges

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.

Flags: needinfo?(bugspam.Callek)
Flags: needinfo?(aki)

Thanks!

Flags: needinfo?(aki)
Flags: needinfo?(bugspam.Callek)

Closing bug because all known projects using taskgraph are either migrated or not impacted by the problem.

Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED

Any chance this could get pushed to mozillareleases/taskgraph:decision as well?

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

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
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

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

Flags: needinfo?(bugspam.Callek)

don't forget us! :) I'll make the PR though.

oh, wait, "for mobile" is different, never mind!

(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)
Flags: needinfo?(bugspam.Callek)

(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!

Flags: needinfo?(dustin)

I think we're up-to-date for this change, but please keep us on the list for next time :)

Flags: needinfo?(dustin)

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.

[1] https://github.com/mozilla-mobile/fenix/pull/16361/files#diff-a728f7e52d751b98eafa856e45594533339b44f229d7b83f930df335391e7e15R246

Status: REOPENED → RESOLVED
Closed: 4 years ago3 years ago
Resolution: --- → FIXED
See Also: → 1635488
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: