Closed Bug 1534283 Opened 5 years ago Closed 5 years ago

Handle DONTBUILD in commit messages

Categories

(Firefox Build System :: Task Configuration, task)

Product:
Firefox Build System
Component:
Task Configuration
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(firefox67 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla67
Tracking Flags:
Tracking Status
firefox67 --- fixed

People

(Reporter: dustin, Assigned: dustin)

References

Details

Attachments

(4 files)

Bug 1534283 - filter out tasks when DONT-BUILD is in the message r=tomprince
47 bytes, text/x-phabricator-request
Details | Review
Bug 1534283 - use default parameters for mach try fuzzy r=tomprince
47 bytes, text/x-phabricator-request
Details | Review
Bug 1534283 - add a 'tasks_for' property; r=tomprince
47 bytes, text/x-phabricator-request
Details | Review
[build/braindump] taskgraph-diff: Add `tasks_for` property r=tomprince
65.47 KB, patch
tomprince
: review-
Details | Diff | Splinter Review

I thought this functionality was in the decision task, but it's not!

https://github.com/taskcluster/mozilla-taskcluster/blob/master/src/jobs/treeherder_resultset.js#L68

    // Common idom is to include "DONTBUILD" in changes to ammend something in a                                                                                                                                                                                                            
    // previous commit like code comments or modify something that is not part
    // of CI.
    if (lastRev.comment.indexOf("DONTBUILD") !== -1) {
      console.log(`Commit for project '${repo.alias}' contains DONTBUILD, skipping`);
      return;
    }

It probably should be, though.

+1. I was thinking DONTBUILD should result in a green decision task with an empty graph. That would allow for add-task type actions, while still saving us most of the compute time.

I like that solution better, but it does mean that we'll have a small window of DONTBUILD not working on branches where that change hadn't been uplifted. Survivable, I suppose!

Well, I tried to make a try job but try is still using mozilla-central so it just didn't build :)

You could probably test on try by adding a 2nd magic string that mozilla-taskcluster doesn't know about? :)

The last try push has an empty target-tasks.json, yet ran a bunch of tasks.

[task 2019-03-11T23:12:37.901Z] Full task graph contains 14886 tasks and 44757 dependencies
[task 2019-03-11T23:12:41.375Z] writing artifact file `full-task-graph.json`
[task 2019-03-11T23:12:46.316Z] writing artifact file `runnable-jobs.json`
[task 2019-03-11T23:12:46.703Z] Generating empty target task set because dontbuild parameter is set
[task 2019-03-11T23:12:46.703Z] writing artifact file `target-tasks.json`
[task 2019-03-11T23:12:46.703Z] Generating target task graph
[task 2019-03-11T23:12:46.721Z] Adding 35 tasks with `always_target` attribute
[task 2019-03-11T23:12:46.751Z] Generating optimized task graph
[task 2019-03-11T23:12:46.752Z] Querying version control for metadata: https://hg.mozilla.org/try/json-automationrelevance/f2a15e0023c368711362fee7e53964f2d9f1ee4f
[task 2019-03-11T23:12:46.752Z] attempt 1/10
[task 2019-03-11T23:12:46.752Z] retry: calling get_automationrelevance, attempt #1
[task 2019-03-11T23:12:48.836Z] 2 commits influencing task scheduling:
[task 2019-03-11T23:12:48.836Z]  bacaa8143a97 Bug 1534283 - don't build when told not to
[task 2019-03-11T23:12:48.836Z]  f2a15e0023c3 Bug 1534283 - try: -j somejob DONUTBUILD

so I guess we need to override always target. Echoes of Buildbot..

(In reply to Dustin J. Mitchell [:dustin] pronoun: he from comment #12)

The last try push has an empty target-tasks.json, yet ran a bunch of tasks.
so I guess we need to override always target. Echoes of Buildbot..

Or not. All these tasks are cheap lints or tests that only run when the corresponding code changes, so I think it would be reasonable to run them anyway.

(Which isn't to say that we shouldn't have a better story around always-target, but I don't think we need to worry about it here)

It was easy enough to move the always-target support into the conditional block, and it kind of fits there better anyway.

Attachment #9050071 - Attachment description: Bug 1534283 - don't build when told not to → Bug 1534283 - don't build when told not to r=tomprince
Attachment #9050071 - Attachment description: Bug 1534283 - don't build when told not to r=tomprince → Bug 1534283 - filter out tasks when DONT-BUILD is in the message r=tomprince

From that latest try push:

[task 2019-03-12T23:18:05.595Z] Filter filter_no_tasks pruned 14895 tasks (0 remain)
[task 2019-03-12T23:18:05.595Z] writing artifact file target-tasks.json
[task 2019-03-12T23:18:05.595Z] Generating target task graph
[task 2019-03-12T23:18:05.613Z] Adding 35 tasks with always_target attribute
...

so it seems to work.

Pushed by dmitchell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/eb2a987c6ba2
add a 'tasks_for' property; r=tomprince
https://hg.mozilla.org/integration/autoland/rev/e44be7f40c6d
filter out tasks when DONT-BUILD is in the message r=aki,tomprince
https://hg.mozilla.org/integration/autoland/rev/6e05f81a2f8f
use default parameters for mach try fuzzy r=tomprince

https://hg.mozilla.org/mozilla-central/rev/eb2a987c6ba2
https://hg.mozilla.org/mozilla-central/rev/e44be7f40c6d
https://hg.mozilla.org/mozilla-central/rev/6e05f81a2f8f

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox67: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Looks like that got ported in https://hg.mozilla.org/comm-central/rev/f2b728425eaf, thanks.

(note that both of those brought along https://lando.services.mozilla.com/D18748/ as a prereq)

Comment on attachment 9052304 [details] [diff] [review]
[build/braindump] taskgraph-diff: Add `tasks_for` property r=tomprince

Review of attachment 9052304 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay in reviewing this. A bunch of these should have `tasks_for` be either `cron` or `action`. I had a patch locally that did that I hadn't pushed, but have done that now.
Attachment #9052304 - Flags: review?(mozilla) → review-

Okay! No problem, I'm glad the fix is pushed :)

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: