When the Servo vcs sync service lands Servo's GitHub commits to autoland, Treeherder currently runs all tests on all platforms. For now, Servo code changes can only break Stylo, so we can limit tests to the linux64-stylo platform on Servo commits to save testing time and resources. Checking the push user (servo-vcs-sync@mozilla.com) might be the easiest way to identify commits from the Servo vcs sync service.

So I talked to my releng colleagues about this, and I'm not sure how we would implement this given the existing state of the tools at our disposal It seems like we would need something like buildbot's enable_perproduct_builds but for taskcluster but only when a commit is landed from a certain user I'm going to ni dustin and see if he has any suggestions on how to implement this or if there is work planned to address this type of scenario

You don't actually need "only a certain user" - servo-vcs-sync only touches '^servo/', so to save on Mac and Windows tests all you need is to add that to the _product_excludes for firefox. Doesn't help with talos-linux64-ix, but then, that's just the longstanding problem that Taskcluster doesn't have a perproduct, so every single mobile/android/-only push is triggering all Linux builds and tests including Linux64 talos.

I want to minimize the impact of the stuff we do to minimize cost for stylo on the design of the in-tree work is kept as low as possible, since when stylo is fully merged it will all be superfluous. So, I can think of a few ways to implement this, but I think the best, and easiest to delete later, is to add a new filter in taskcluster/taskgraph/filter_tasks.py which, when the `owner` parameter is `servo-vcs-sync@mozilla.com`, filters out tasks *not* requiring servo, that is, those that are not in SERVO_PLATFORMS.

I wasn't sure why taskcluster/taskgraph/filter_tasks.py should be modified vs taskcluster/taskgraph/target_tasks.py so this patch is a guess

Comment on attachment 8838246 [details] [diff] [review] bu1339542.patch Review of attachment 8838246 [details] [diff] [review]: ----------------------------------------------------------------- The difference is that only one target_tasks_method can be selected for a run of the taskgraph generation, whereas multiple filters can run. So in this situation, I think we want to run the "normal" target_task_method for each projct, so basic behavior doesn't change, and then add a new filter which further subtracts stylo jobs when they do not need to be run (based on owner and project). Filters were a little clearer when there were two of them, but as of the 12th there's only one (https://hg.mozilla.org/mozilla-central/rev/4db2b9c21878).

So I tried a few approaches to this problem but it doesn't seem to be working In the previous patch to vendor servo, the filter looked like this. Basically it is checking for the existence of a file to ensure and then filtering on associated jobs. specifically if b'servo/ports/geckolib' in cargo: return graph.tasks.keys() I'm not sure how what how to filter out the tasks servo tasks without looking for a file. I have the bits to filter out on the email address in def fltr. Do you have any suggestions? def filter_servo(graph, parameters): """Filters out tasks requiring Servo if Servo isn't present.""" # This filter is temporary until Servo's dependencies are vendored. cargo = os.path.join(GECKO, 'toolkit', 'library', 'rust', 'shared', 'Cargo.toml') with open(cargo, 'rb') as fh: cargo = fh.read() if b'servo/ports/geckolib' in cargo: return graph.tasks.keys() logger.info('real servo geckolib not used; removing tasks requiring it') SERVO_PLATFORMS = { 'linux64-stylo', } def fltr(task): if task.attributes.get('build_platform') in SERVO_PLATFORMS: return False return True return [l for l, t in graph.tasks.iteritems() if fltr(t)]

So breaking down that exisitng function: set up the condition: > # This filter is temporary until Servo's dependencies are vendored. > cargo = os.path.join(GECKO, 'toolkit', 'library', 'rust', 'shared', > 'Cargo.toml') > with open(cargo, 'rb') as fh: > cargo = fh.read() > > if b'servo/ports/geckolib' in cargo: servo exists, so run everything > return graph.tasks.keys() else servo does not exist, so run everything but linux64-servo > logger.info('real servo geckolib not used; removing tasks requiring it') > > SERVO_PLATFORMS = { > 'linux64-stylo', > } > > def fltr(task): > if task.attributes.get('build_platform') in SERVO_PLATFORMS: > return False > > return True > > return [l for l, t in graph.tasks.iteritems() if fltr(t)] From what I understand, in this case, the logic should be: Set up the condition, by looking at the owner. If the owner is the vcs sync service run everything else run everything but linux64 servo So basically just a different condition. I'm not totally sure that answers your question, though.. let me know if not :)

(In reply to Dustin J. Mitchell [:dustin] from comment #8) > From what I understand, in this case, the logic should be: > > Set up the condition, by looking at the owner. > If the owner is the vcs sync service > run everything > else > run everything but linux64 servo I may be misunderstanding what "run everything" means in this context, but I believe the logic we want is: if the owner is the vcs sync service (or whatever check for servo commits): run only the linux64-stylo tests (because Servo changes can't break Gecko) else run all tests, including linux64-stylo Stylo code changes can land directly in autoland, not just upstream in the Servo GitHub repo. Also, other Gecko code changes might break the Stylo code, so we always want to run the linux64-stylo tests.

"run everything" = "run all tests, including linux64-stylo", but yes, I did misunderstand what "only" in the bug summary modifies. Thanks!

Oops. I see how my original bug summary was ambiguous, so I will reword it.

I have new patches but I'm not sure how to test a new filter via try or if there is a different mechanism I should use

kmoir: maybe don't test on try, but test locally with `./mach taskgraph optimized` from a parameters from a stylo merge? 10:34 AM <kmoir> Kim Moir okay will try that, thanks! 10:34 AM <Callek> (maybe optimized --json if you want to see the tasks not just taskids, and/or mach taskgraph target)

Comment on attachment 8854072 [details] [diff] [review] bug1339542-2.patch Review of attachment 8854072 [details] [diff] [review]: ----------------------------------------------------------------- This is the right idea, just needs a tweak to the logic :) You can test this in different scenarios as Callek suggested. Just change the parameters in `parameters.yml` to simulate different conditions (e.g., set owner to the sync user, and to someone else). ::: taskcluster/taskgraph/filter_tasks.py @@ +34,5 @@ > + > + > +@filter_task('check_servo') > +def filter_servo(graph, parameters): > + """Filters out tasks requiring Servo if Servo isn't present.""" This docstring needs a tweak, I think. @@ +47,5 @@ > + if parameters.get('owner') == "servo-vcs-sync@mozilla.com" and \ > + task.attributes.get('build_platform') in SERVO_PLATFORMS: > + return True > + > + return False I think this will run nothing if owner != servo-vcs-sync? I suspect you want if owner != servo-vcs-sync: return true return build_platform in SERVO_PLATFORMS

I tested this and got the expected results

Pushed by kmoir@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/37162353a3a2 When Servo vcs sync service lands Servo commits in autoland repo, run linux64-stylo tests but skip other platforms (to reduce test load) r=dustin DONTBUILD

Looks like this is working after the merge from m-c https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=ff65f545908426cd3d420a19592b0c3730ce5b5d