After work on running Spidermonkey on try is done successfully, we should switch from using Buildbot to TaskCluster for these jobs

This might be a little rough around the edges, but feedback would be great nonetheless. Try push with current patch -- https://treeherder.mozilla.org/#/jobs?repo=try&revision=d023d3b08985 Try push with an earlier version (but similar), that succeeded, https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d16f57a8fd3

This is looking really good so far: a couple of comments. Let's use quay.io/mrrrgn/desktop-build -- I know it's my own account, but it's "official" for now until we decide on a official Mozilla docker repo. Then, let's modify the treeherder symbol so that it's not just a 'B' -- The current jobs are: SM(cgc e r) To add that you can use: extra: treeherder: groupSymbol: SM-tc groupName: Spider Monkey, submitted by taskcluster symbol: p collection: debug: false

Comment on attachment 8625999 [details] Initial Draft of patch to enable spidermonkey jobs on try See the comment I just left. Looking good, just needs a few tweaks. :)

Adding updated patch based on mrrrgn's comments. Full suite of all spidermonkey builds incoming soon.

Added all the Spidermonkey builds to try. Working try push here -- https://treeherder.mozilla.org/#/jobs?repo=try&revision=595e62cc0cfa NI'ing sfink for additional input.

RyanVM's already pointed out that the OSX jobs don't belong there at all. This goes back a question in https://bugzilla.mozilla.org/show_bug.cgi?id=1164656#c20 . Other feedback is welcome as well.

Comment on attachment 8627413 [details] [diff] [review] All variants Review of attachment 8627413 [details] [diff] [review]: ----------------------------------------------------------------- Looking good, just had some comments/questions. ::: testing/taskcluster/tasks/builds/sm_base.yml @@ +5,5 @@ > + > + routes: > + - 'ffledgling.test.spidermonkey' > + #- 'index.buildbot.branches.{{project}}.sm-plain' > + #- 'index.buildbot.revisions.{{head_rev}}.{{project}}.sm-plain' My guess would be that these will not be commented out once deployed. Although I'm not sure if there is any risk not having them commented out them while testing. They are just for try right now and the revision would be the one you're pushing. Not sure if the index is used much for try so maybe I'm mistaken. @@ +8,5 @@ > + #- 'index.buildbot.branches.{{project}}.sm-plain' > + #- 'index.buildbot.revisions.{{head_rev}}.{{project}}.sm-plain' > + > + scopes: > + - 'docker-worker:cache:build-linux64-workspace' Is there value in sharing this cache with the other linux64 builds? Since this cache contains not only the sources, but also objdirs, I'm not sure if SM can make use of that. @@ +9,5 @@ > + #- 'index.buildbot.revisions.{{head_rev}}.{{project}}.sm-plain' > + > + scopes: > + - 'docker-worker:cache:build-linux64-workspace' > + # Do I need this? whatever caches listed under payload.cache will require a scope for it otherwise the task will be rejected by the worker. I'm not sure if spider monkey builds need tooltool, but might not and if that's the case you might not need the cache at all (thus removing scope and payload.cache.tooltool-cache). It doesn't hurt having it though. @@ +14,5 @@ > + - 'docker-worker:cache:tooltool-cache' > + > + payload: > + #image: '{{#docker_image}}desktop-build{{/docker_image}}' > + image: 'quay.io/ffledgling/build-desktop-temp' I think I recall you saying that this was changed because the desktop-build image wasn't available. We'll just want to make sure to change this back and make sure that image is freely available to everyone. @@ +33,5 @@ > + treeherderEnv: > + - staging > + - production > + treeherder: > + groupSymbol: SM-tc Are spidermonkey builds currently reported to TH? If not, I'm not sure if we need to include "-tc". From my understanding, including "-tc" was mainly for when we had buildbot and tc jobs living side-by-side. @@ +34,5 @@ > + - staging > + - production > + treeherder: > + groupSymbol: SM-tc > + groupName: Spider Monkey, submitted by taskcluster is the ', submitted by taskcluster' here necessary? @@ +36,5 @@ > + treeherder: > + groupSymbol: SM-tc > + groupName: Spider Monkey, submitted by taskcluster > + collection: > + debug: false If the purpose here is for this to appear ont he 'opt' line in tree herder, it does this by default if the collection key is not specified. https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/mach_commands.py#318

Comment on attachment 8627413 [details] [diff] [review] All variants This is super close. Great work so far!

(In reply to Greg Arndt [:garndt] from comment #7) > Comment on attachment 8627413 [details] [diff] [review] > All variants > > Review of attachment 8627413 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looking good, just had some comments/questions. > > ::: testing/taskcluster/tasks/builds/sm_base.yml > @@ +5,5 @@ > > + > > + routes: > > + - 'ffledgling.test.spidermonkey' > > + #- 'index.buildbot.branches.{{project}}.sm-plain' > > + #- 'index.buildbot.revisions.{{head_rev}}.{{project}}.sm-plain' > > My guess would be that these will not be commented out once deployed. > Although I'm not sure if there is any risk not having them commented out > them while testing. They are just for try right now and the revision would > be the one you're pushing. Not sure if the index is used much for try so > maybe I'm mistaken. > > @@ +8,5 @@ > > + #- 'index.buildbot.branches.{{project}}.sm-plain' > > + #- 'index.buildbot.revisions.{{head_rev}}.{{project}}.sm-plain' > > + > > + scopes: > > + - 'docker-worker:cache:build-linux64-workspace' > > Is there value in sharing this cache with the other linux64 builds? Since > this cache contains not only the sources, but also objdirs, I'm not sure if > SM can make use of that. > > @@ +9,5 @@ > > + #- 'index.buildbot.revisions.{{head_rev}}.{{project}}.sm-plain' > > + > > + scopes: > > + - 'docker-worker:cache:build-linux64-workspace' > > + # Do I need this? > > whatever caches listed under payload.cache will require a scope for it > otherwise the task will be rejected by the worker. I'm not sure if spider > monkey builds need tooltool, but might not and if that's the case you might > not need the cache at all (thus removing scope and > payload.cache.tooltool-cache). It doesn't hurt having it though. > > @@ +14,5 @@ > > + - 'docker-worker:cache:tooltool-cache' > > + > > + payload: > > + #image: '{{#docker_image}}desktop-build{{/docker_image}}' > > + image: 'quay.io/ffledgling/build-desktop-temp' > > I think I recall you saying that this was changed because the desktop-build > image wasn't available. We'll just want to make sure to change this back > and make sure that image is freely available to everyone. Yes, as soon as the patch in Bug 1164656 lands, this can be changed to the 'official' version. > > @@ +33,5 @@ > > + treeherderEnv: > > + - staging > > + - production > > + treeherder: > > + groupSymbol: SM-tc > > Are spidermonkey builds currently reported to TH? If not, I'm not sure if we > need to include "-tc". From my understanding, including "-tc" was mainly > for when we had buildbot and tc jobs living side-by-side. They are. See https://treeherder.mozilla.org/help.html for the SM builds. > @@ +34,5 @@ > > + - staging > > + - production > > + treeherder: > > + groupSymbol: SM-tc > > + groupName: Spider Monkey, submitted by taskcluster > > is the ', submitted by taskcluster' here necessary? I'm not entirely sure, but I'm following the convention based on what I saw for the linux builds, doesn't really seem to hurt, since Spidermonkey builds exist on Treeherder outside of TaskCluster as well. > @@ +36,5 @@ > > + treeherder: > > + groupSymbol: SM-tc > > + groupName: Spider Monkey, submitted by taskcluster > > + collection: > > + debug: false > > If the purpose here is for this to appear ont he 'opt' line in tree herder, > it does this by default if the collection key is not specified. > https://dxr.mozilla.org/mozilla-central/source/testing/taskcluster/ > mach_commands.py#318 Everything else, I've addressed in my updated patch.

hI've incorporated most of gardnt's suggestions in this patch, I've also cleaned up some other stuff like proper separation b/w debug and opt. The only fiddly bits left here are the artifact upload bits, which should be sorted out soon. Note: This patch needs to land after the patch for Bug 11646656 lands.

Final one, with all the issues sorted out. This builds on-top of the patch from Bug 1164656, so in case you're testing locally, you might want to apply that one first. Successful try push with these exact changes -- https://treeherder.mozilla.org/#/jobs?repo=try&revision=8a316a5d4314

mrrrgn, This has the changes, and uses your repo -- https://treeherder.mozilla.org/#/jobs?repo=try&revision=2ace7278cff0 + a bunch of other comments removed that I forgot to kill.