taskgraph: code_review transform injects dependencies task in the wrong tasks_for
Categories
(Firefox Build System :: Task Configuration, defect)
Tracking
(Not tracked)
People
(Reporter: jlorenzo, Assigned: jlorenzo)
References
Details
Attachments
(3 files, 4 obsolete files)
In android-components, we have 2 complete tasks[1]: one for pull request, the other one for regular pushes. The latter is used by bors[2] (the autoland bot) when a trusted user thinks a PR is ready to be merged. This way, we run tests that require some privilege just before the patch lands on master.
Speaking of tests, :sebastian realized complete-push doesn't wait for these tests[3]. That's expected because the complete kind doesn't depend on the test kind[4]. So I first added it, but it had an unwanted side-effect: the privileged tests are brought to pull requests.
I think we should make this transform[5] smarter.
[1] https://github.com/mozilla-mobile/android-components/blob/f33bbcb2255fa196c7c7c245e8608c6aa9dd4e56/taskcluster/ci/complete/kind.yml
[2] https://github.com/mozilla-mobile/android-components/blob/f33bbcb2255fa196c7c7c245e8608c6aa9dd4e56/bors.toml#L2
[3] https://github.com/mozilla-mobile/android-components/blob/master/taskcluster/ci/test/kind.yml#L33
[4] https://github.com/mozilla-mobile/android-components/blob/f33bbcb2255fa196c7c7c245e8608c6aa9dd4e56/taskcluster/ci/complete/kind.yml#L7
[5] https://hg.mozilla.org/ci/taskgraph/file/ec220e56cf107ea279d7c08706fc370374869550/src/taskgraph/transforms/code_review.py#l22
|
Assignee | |
Comment 1•5 years ago
|
||
|
|
Assignee | |
Comment 2•5 years ago
|
||
I'm unassigning myself to this bug. I believe it will likely bite someone else later on but I don't have the bandwidth to handle it. Keeping it open for the sake of tracking.
|
||
Updated•4 years ago
|
|
|
Assignee | |
Comment 3•3 years ago
|
||
I had another look at this current bug because of bug 1803141. Looking back at the current state of things, I agree with :tomprince's comment[1] about making this a morph rather than a transforms. The morph looks like this one[2]. The good news is :ahal made morphs registrable from a random project in taskcluster/taskgraph#91. Thus, we can make this morph in firefox-android first then maybe see if it's generic-enough to be part of core taskgraph.
For the record, this index[3] doesn't exist anymore which means this code_review tasks hasn't been scheduled in the past year, which means it's dead code.
[1] https://phabricator.services.mozilla.com/D78043#2624412
[2] https://github.com/taskcluster/taskgraph/blob/1013921db0badd2e35bb975a6247523dcd75cbf1/src/taskgraph/morph.py#L206-L262
[3] https://github.com/taskcluster/taskgraph/blob/c8c057193d028a4ab4bbf673ed28299f2a2f06d4/src/taskgraph/morph.py#L220
|
||
Updated•3 years ago
|
|
||
Comment 4•3 years ago
|
||
|
|
||
Comment 5•3 years ago
|
||
|
|
||
Comment 6•3 years ago
|
||
|
|
Assignee | |
Updated•3 years ago
|
|
|
||
Comment 7•3 years ago
|
||
Comment on attachment 9306239 [details] [review]
[mozilla-mobile/firefox-android] Bug 1642977 - Let complete task depend on tasks are targeted and not optimized away (#242)
GitHub pull request attachment was moved to bug 1804506. Setting attachment 9306239 [details] [review] to obsolete.
|
|
||
Comment 8•3 years ago
|
||
Comment on attachment 9306254 [details] [review]
[mozilla-mobile/firefox-android] Bug 1642977 - Let complete task depend on tasks which are targeted and not optimized away (#243)
GitHub pull request attachment was moved to bug 1805265. Setting attachment 9306254 [details] [review] to obsolete.
|
|
||
Comment 9•3 years ago
|
||
|
|
||
Comment 10•3 years ago
|
||
|
|
Assignee | |
Comment 11•3 years ago
|
||
|
|
Assignee | |
Comment 12•3 years ago
|
||
Status update: :jcristau, :ahal, and I agreed the the morph task wasn't the right approach after all. Most of the task definition should be handled by transforms[1] and we don't have access to that anymore during the morph phase. In the end, the bug was simpler to fix: the chunk transforms were actually making the soft dependencies hard ones[2]. We just had to make them comply.
The changes were landed a couple of weeks ago and the generated graphs looks much better. I even forgot to uplift these patch set to the 108 branch and I started seeing unwanted tasks. A backport fixed it, proving the patch was the right one.
Closing bug 🙂
[1] https://github.com/mozilla-mobile/firefox-android/pull/242/files#r1037201333
[2] https://github.com/mozilla-mobile/firefox-android/pull/360/files#diff-37e61e69120f91f8bcdbd5afbf690705acaf462fb3faac9395909cf7f13461bfR39
|
|
||
Comment 13•3 years ago
|
||
Comment on attachment 9306439 [details] [review]
[mozilla-mobile/firefox-android] Bug 1642977 - part 4: Final complete tasks should only depend on ch… (#254)
GitHub pull request attachment was moved to bug 1807954. Setting attachment 9306439 [details] [review] to obsolete.
| Comment hidden (collapsed) |
Description
•