Chris AtLee [:catlee] Reporter
	Description • 9 years ago

      No description provided.

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 1 • 9 years ago

Taking this bug, to get my hands dirty with taskcluster.

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 2 • 9 years ago

Update:

Intial build using the existing scripts (https://github.com/mozilla/build-tools/blob/master/scripts/spidermonkey_builds/spidermonkey.sh) worked on TaskCluster, you can see that build here -- https://tools.taskcluster.net/task-inspector/#MdNFt-dnRkSygo5w-OWrGg/0

An image with everything baked in can be found at https://quay.io/repository/ffledgling/spidermonkey-build , it's based on the docker image called desktop-build

I'm currently working on moving spidermonkey to mozharness so that it works similar to the rest of the builds.

After that's done, I'll probably need to write a decision task for so that it works with Try, need input from dustin or mrrrgn about that bit.

Currently latching onto rail as temp advisor for this bit.

TL;DR:

(1) Initial Build on TC for SM build is successful with fixed params.
(2) Working on moving away from spidermonkey.sh to mozharness.

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 3 • 9 years ago

Patch of currently working setup with spidermonkey.sh
Patch is currently not using quay.io/mozilla/desktop-build because I can't actually pull that from upstream (either private or does not exist, filling a bug for that). But I built the same locally from the dockerfile in-tree, so shouldn't be a problem otherwise.

Will also change the repo to mozilla/spidermonkey-build whenever I get access/someone else wants to push.

	Morgan Phillips [:mrrrgn]
	Comment 4 • 9 years ago

Comment on attachment 8622747 [details] [diff] [review]
Bug-1164656.patch

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

I'm going to refactor the existing container to save you from needing any of these changes. I'll run a demonstration job after and attach the result to the task. That will just leave hooking the job up to try.

::: testing/docker/spidermonkey-build/Dockerfile
@@ +1,1 @@
> +#  Replace with quay.io/mozilla/desktop-build later

Instead of creating a new container, it would be preferable to use the existing one.

::: testing/docker/spidermonkey-build/bin/build.sh
@@ +1,4 @@
> +#!/bin/bash
> +
> +# Clone build-tools
> +tc-vcs checkout build-tools https://github.com/mozilla/build-tools.git

So, we need the ability to specify which repo/branch to checkout. I'm refactoring the container tonight so that you can use the logic we already have.

	Morgan Phillips [:mrrrgn]
	Comment 5 • 9 years ago

(In reply to Anhad Jai Singh (:ffledgling) from comment #3)
> Created attachment 8622747 [details] [diff] [review]
> Bug-1164656.patch
> 
> Patch of currently working setup with spidermonkey.sh
> Patch is currently not using quay.io/mozilla/desktop-build because I can't
> actually pull that from upstream (either private or does not exist, filling
> a bug for that). But I built the same locally from the dockerfile in-tree,
> so shouldn't be a problem otherwise.
> 
> Will also change the repo to mozilla/spidermonkey-build whenever I get
> access/someone else wants to push.

For now you can find the latest versions at quay.io/mrrrgn/desktop-build

	Rail Aliiev [:rail]
	Comment 6 • 9 years ago

Comment on attachment 8622747 [details] [diff] [review]
Bug-1164656.patch

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

In overall it looks plausible. With Morgan's changes the diff will be even smaller! :)

::: testing/docker/spidermonkey-build/bin/build.sh
@@ +3,5 @@
> +# Clone build-tools
> +tc-vcs checkout build-tools https://github.com/mozilla/build-tools.git
> +
> +# Clone M-C using tc-vcs, then spidermonkey can clone locally, otherwise the spidermonkey script timesout
> +tc-vcs checkout mozilla-central http://hg.mozilla.org/mozilla-central/

It would be great to avoid hardcoding URLs, or at least branch names.

	Steve Fink [:sfink] [:s:]
	Comment 7 • 9 years ago

For the record: there is nothing in spidermonkey.sh that I was hoping to preserve. It is supposed to be an adapter between buildbot and the actual job. autospider.sh is intended to be runnable from either automation or a developer's desktop, and spidermonkey.sh does the glue (checking out the code, purging, etc.) necessary in automation. So if this gets moved to mozharness and/or onto mrrrgn's setup, I would be quite happy if you shot spidermonkey.sh in the head.

	Steve Fink [:sfink] [:s:]
	Comment 8 • 9 years ago

The current SM(...) builds do not produce any build artifacts at all. Eventually, I would like to get the whole obj-spider/dist/ subdirectory. Though if that's too big (looks like it's about 1GB), the highest priority are:

  dist/bin/js
  dist/bin/jsapi-tests
  dist/bin/js-gdb.py

	Steve Fink [:sfink] [:s:]
	Comment 9 • 9 years ago

ffledgling: note that in bug 1172901, people have requested that these jobs run on the release branches too. If that patch sticks, it will update the buildbot scheduling to include them, so you'll probably want to match that.

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 10 • 9 years ago

I made a few changes to checkout-sources to optionally disable checking out mozharness and tools since they're not required.

Added build-sm.sh to kickoff the build based on whatever variant of the SM job is required.

	Morgan Phillips [:mrrrgn]
	Comment 11 • 9 years ago

Comment on attachment 8624200 [details] [diff] [review]
Bug-1164656.2.patch

Perfect! 

My only nit, is that you change the MOZHARNESS_DISABLE and TOOLS_DISABLE to something like

: MOZHARNESS_DISABLE                     ${MOZHARNESS_DISABLE:=false}
: TOOLS_DISABLE                          ${TOOLS_DISABLE:=false}

To fit with the style of the other arguments.

Thanks for working on this!

	Morgan Phillips [:mrrrgn]
	Comment 12 • 9 years ago

Can you also attach some working builds using a container built with this patch?

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 13 • 9 years ago

Thanks for the review, Morgan.

The problem with setting a default on the MOZHARNESS_DISABLE, instead of not setting it at all will cause the -v test to pass (which checks if a variable is set). If you still want a default, I can change the test condition to test for the truth of the variable or similar instead.

I started a few builds with the container, they were killed at the hour mark when I checked back. Although the build actually starts and runs without hicups, so that's fairly encouraging.

Here's a new one that has more time -- https://tools.taskcluster.net/task-inspector/#G8OgOJlMTAu_e_710GxPWA/0 , It's still running, I'll update with results, when it's done.

	Morgan Phillips [:mrrrgn]
	Comment 14 • 9 years ago

(In reply to Anhad Jai Singh (:ffledgling) from comment #13)
> Thanks for the review, Morgan.
> 
> The problem with setting a default on the MOZHARNESS_DISABLE, instead of not
> setting it at all will cause the -v test to pass (which checks if a variable
> is set). If you still want a default, I can change the test condition to
> test for the truth of the variable or similar instead.
> 
> I started a few builds with the container, they were killed at the hour mark
> when I checked back. Although the build actually starts and runs without
> hicups, so that's fairly encouraging.
> 
> Here's a new one that has more time --
> https://tools.taskcluster.net/task-inspector/#G8OgOJlMTAu_e_710GxPWA/0 ,
> It's still running, I'll update with results, when it's done.

The job looks good; but I'd still prefer for the variables to explicitly use true/false as opposed to set/unset. +1 in any case.

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 15 • 9 years ago

sfink, do you mind look at the TC job posted in comment 13 , and checking to see if things are in order? Are there any other types of jobs you'd like me to run/test? So far I've been testing with 'plain'. I might also be able to get the artifacts uploaded, but that will not happen when the job is run by hand.

	Steve Fink [:sfink] [:s:]
	Comment 16 • 9 years ago

(In reply to Anhad Jai Singh (:ffledgling) from comment #15)
> sfink, do you mind look at the TC job posted in comment 13 , and checking to
> see if things are in order? Are there any other types of jobs you'd like me
> to run/test? So far I've been testing with 'plain'. I might also be able to
> get the artifacts uploaded, but that will not happen when the job is run by
> hand.

I don't know how. I went to that link, but couldn't find any output. Clicking on the log artifact sent me to a server I couldn't resolve.

'compacting' might be a good second variant to try, since it's probably the most different. 'arm-sim' would be good too, since there are some weird 32 vs 64 bit issues there.

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 17 • 9 years ago

(In reply to Steve Fink [:sfink, :s:] from comment #16)
> (In reply to Anhad Jai Singh (:ffledgling) from comment #15)
> > sfink, do you mind look at the TC job posted in comment 13 , and checking to
> > see if things are in order? Are there any other types of jobs you'd like me
> > to run/test? So far I've been testing with 'plain'. I might also be able to
> > get the artifacts uploaded, but that will not happen when the job is run by
> > hand.
> 
> I don't know how. I went to that link, but couldn't find any output.
> Clicking on the log artifact sent me to a server I couldn't resolve.
> 

The logs seems to have disappeared, I re-ran it. See below (hopefully this one is visible).

> 'compacting' might be a good second variant to try, since it's probably the
> most different. 'arm-sim' would be good too, since there are some weird 32
> vs 64 bit issues there.

plain -- https://tools.taskcluster.net/task-inspector/#cbeevlcVSJGS5vFIuv_G9g/2
arm-sim -- https://tools.taskcluster.net/task-inspector/#udEM2amRTNCU3VlVnIH5wg/1
compacting -- https://tools.taskcluster.net/task-inspector/#I61x8USKR4Ww1fkvTQSTcg/

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 18 • 9 years ago

Started all builds on Task Cluster --
arm-sim         TaskURL:  https://tools.taskcluster.net/task-inspector/#oGxI0Z1cSeSsqGJnoZapyg
arm-sim-osx     TaskURL:  https://tools.taskcluster.net/task-inspector/#tiQw1-ZMRx-WGcJ2BAal4g
compacting      TaskURL:  https://tools.taskcluster.net/task-inspector/#Axk2lhu-RoS33oJjVdo5IQ
generational    TaskURL:  https://tools.taskcluster.net/task-inspector/#ke19Q-kWSwGlirbREW3TTQ
plain           TaskURL:  https://tools.taskcluster.net/task-inspector/#RvRt_RoYRdauLV8VpbpvEQ
plaindebug      TaskURL:  https://tools.taskcluster.net/task-inspector/#P96mSBbGQS6sjjO-9zBoMQ
rootanalysis    TaskURL:  https://tools.taskcluster.net/task-inspector/#06gSEg9vSiKjxwpz6wI85w
warnaserr       TaskURL:  https://tools.taskcluster.net/task-inspector/#uelKSkgxT2CHeNytX8unYw
warnaserrdebug  TaskURL:  https://tools.taskcluster.net/task-inspector/#h_bc9dHNRvmmjTW_WR9JKA


Script used to launch this -- https://github.com/ffledgling/taskcluster-matrix-launcher

NI'ing sfink, so that he can take a look.

Side-Note: build-sm.sh exits with a successful error code, 0, even when things like configure fail. It should ideally not do this, so that the build can actually show up as failed.

	Steve Fink [:sfink] [:s:]
	Comment 19 • 9 years ago

Comment on attachment 8624200 [details] [diff] [review]
Bug-1164656.2.patch

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

::: testing/taskcluster/scripts/builder/build-sm.sh
@@ +7,5 @@
> +# cd into the correct directory
> +cd $HOME/workspace/
> +
> +# Run the script
> +./build/src/js/src/devtools/automation/autospider.sh $SPIDERMONKEY_VARIANT

This should remember the status, eg

.../autospider.sh $SPIDERMONKEY_VARIANT
JOB_STATUS=$?

...then at the end of the script...

exit $JOB_STATUS

Note that you could alternatively you could just do a 'set -e' at the top of the script, so that it would abort immediately if any command failed, but I do not want that behavior. I *want* the artifacts from failed runs. In fact, they're probably more useful than the artifacts from successful runs. (At some point, we might even split those up -- successful builds would package up just the shared libraries that people could use with their embedded spidermonkey stuff, and failed builds would archive the whole build. But we don't generate quite the right stuff for successful builds yet, so never mind for now.)

@@ +13,5 @@
> +# Ensure upload dir exists
> +mkdir -p $UPLOAD_DIR
> +
> +# Move artifacts for upload by TaskCluster
> +mv ./build/src/obj-spider/* $UPLOAD_DIR/

This is going to scoop up a lot of temporary files and things. For space saving, this should probably be

  mv build/src/obj-spider/dist/* $UPLOAD_DIR/

Then again, I'm not sure if this works at all. dist/bin/ mostly contains symlinks to the actual binaries, which means $UPLOAD_DIR will have symlinks too. Does whatever uploads stuff dereference the symlinks to get the actual bits? If not, perhaps this should use |cp -rL| instead of |mv|?

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 20 • 9 years ago

Steve, I was wondering if all the SM builds can run on linux64 machines (except the OSX ones? Unless those are cross compiled?), or if some of them need linux32 bit, or Windows or some such.

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 21 • 9 years ago

(In reply to Steve Fink [:sfink, :s:] from comment #19)
> Comment on attachment 8624200 [details] [diff] [review]
> Bug-1164656.2.patch
> 
> Review of attachment 8624200 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: testing/taskcluster/scripts/builder/build-sm.sh
> @@ +7,5 @@
> > +# cd into the correct directory
> > +cd $HOME/workspace/
> > +
> > +# Run the script
> > +./build/src/js/src/devtools/automation/autospider.sh $SPIDERMONKEY_VARIANT
> 
> This should remember the status, eg
> 
> .../autospider.sh $SPIDERMONKEY_VARIANT
> JOB_STATUS=$?
> 
> ...then at the end of the script...
> 
> exit $JOB_STATUS
> 
> Note that you could alternatively you could just do a 'set -e' at the top of
> the script, so that it would abort immediately if any command failed, but I
> do not want that behavior. I *want* the artifacts from failed runs. In fact,
> they're probably more useful than the artifacts from successful runs. (At
> some point, we might even split those up -- successful builds would package
> up just the shared libraries that people could use with their embedded
> spidermonkey stuff, and failed builds would archive the whole build. But we
> don't generate quite the right stuff for successful builds yet, so never
> mind for now.)

This makes sense, willdo, oversight on my part.

> @@ +13,5 @@
> > +# Ensure upload dir exists
> > +mkdir -p $UPLOAD_DIR
> > +
> > +# Move artifacts for upload by TaskCluster
> > +mv ./build/src/obj-spider/* $UPLOAD_DIR/
> 
> This is going to scoop up a lot of temporary files and things. For space
> saving, this should probably be
> 
>   mv build/src/obj-spider/dist/* $UPLOAD_DIR/
> 
> Then again, I'm not sure if this works at all. dist/bin/ mostly contains
> symlinks to the actual binaries, which means $UPLOAD_DIR will have symlinks
> too. Does whatever uploads stuff dereference the symlinks to get the actual
> bits? If not, perhaps this should use |cp -rL| instead of |mv|?

I'm not a 100% sure if the current way works, but I'll find out one way or the other.

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 22 • 9 years ago

Updated Push -- https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd92907122d0

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 24 • 9 years ago

This should be the final version of the patch to get spidermonkey up and running with taskcluser.
The only change from the earlier version should be the proper exist code, and the Boolean *_DISABLE variables the way mrrrgn wanted them.

Once this is okay'd, mrrrgn could you please update the desktop-build image on quay.io to incorporate the changes from the checkout-sources.sh?

Someone will also have to land this on my behalf, since I don't have check-in powers.

	Morgan Phillips [:mrrrgn]
	Comment 25 • 9 years ago

Comment on attachment 8628586 [details] [diff] [review]
Spidermonkey-TC

http://ak-hdl.buzzfed.com/static/2014-08/29/16/enhanced/webdr10/anigif_enhanced-9754-1409344312-11.gif I'll land it :)

	Morgan Phillips [:mrrrgn]
	Comment 26 • 9 years ago

Also, there's no commit message. I added one, but be careful about that in the future. Sheriffs will not be happy if a nude commit makes it into inbound. :]

	Morgan Phillips [:mrrrgn]
	Comment 27 • 9 years ago

Made minor changes to the diff. This is what I'll check in when the tree re-opens.

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 28 • 9 years ago

Created one with the proper user history and commit message. Carrying over the r+. Please use this if you haven't already checked in the older one.

	Pulsebot
	Comment 29 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/0dec8eff4e3d
https://hg.mozilla.org/integration/mozilla-inbound/rev/dcc629da88f2

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 30 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/0dec8eff4e3d
https://hg.mozilla.org/mozilla-central/rev/dcc629da88f2

	Steve Fink [:sfink] [:s:]
	Comment 31 • 9 years ago

(In reply to Anhad Jai Singh (:ffledgling) from comment #20)
> Steve, I was wondering if all the SM builds can run on linux64 machines
> (except the OSX ones? Unless those are cross compiled?), or if some of them
> need linux32 bit, or Windows or some such.

I just realized I never replied to this.

There's a mixture of platforms in there. Some of the variants have configure flags that only work with one platform. You could get a complete listing from

wget -q -O- https://secure.pub.build.mozilla.org/builddata/reports/allthethings.json | egrep 'spidermonkey_try-.*\{$'

but let's see if I can lay it out:

linux32: arm-sim warnaserr warnaserrdebug
linux64: compacting rootanalysis warnaserr warnaserrdebug
win32: compacting plain plaindebug
win64: compacting plain plaindebug
osx64: arm-sim-osx

...but I don't think arm-sim-osx works yet, and really there are debug and opt variants of the compacting build, and not all of these are active by default on try.

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 32 • 9 years ago

(In reply to Steve Fink [:sfink, :s:] from comment #31)
> (In reply to Anhad Jai Singh (:ffledgling) from comment #20)
> > Steve, I was wondering if all the SM builds can run on linux64 machines
> > (except the OSX ones? Unless those are cross compiled?), or if some of them
> > need linux32 bit, or Windows or some such.
> 
> I just realized I never replied to this.
> 
> There's a mixture of platforms in there. Some of the variants have configure
> flags that only work with one platform. You could get a complete listing from
> 
> wget -q -O-
> https://secure.pub.build.mozilla.org/builddata/reports/allthethings.json |
> egrep 'spidermonkey_try-.*\{$'
> 
> but let's see if I can lay it out:
> 
> linux32: arm-sim warnaserr warnaserrdebug
> linux64: compacting rootanalysis warnaserr warnaserrdebug
> win32: compacting plain plaindebug
> win64: compacting plain plaindebug
> osx64: arm-sim-osx
> 
> ...but I don't think arm-sim-osx works yet, and really there are debug and
> opt variants of the compacting build, and not all of these are active by
> default on try.

This is good information to have, but unfortunately, I don't think we're in a position to hook all of these up on the right platforms correctly using taskcluster at the moment.

Taskcluster does not, at the moment, support Windows or OSX (Work is in progress for these).

I'm sceptical about linux32 being supported by docker-worker as well, but I'll look into it and report back.

Currently the status of this is that we have the following versions in tree, all of which are triggered using linux64:
arm-sim
arm-sim-osx
compacting
generational
plain
plaindebug
rootanalysis
warnaserr
warnaserrdebug

All of these except arm-sim-osx work on linux64 iirc.

I've already been made aware that arm-sim-osx is supposed to run on OSX, so I haven't added a try flag for it in https://bugzil.la/1179050 , so that people don't use it.

If you don't want any of these to run on linux64 if they don't make sense for example, or aren't useful or just aren't needed anymore, let me know I can remove them.

A matrix of ones you would like to see and against what platform + anyother info would be nice to have in the comments, for the purposes of posterity if nothing else.
I can work on getting the additional ones in one-by-one.

	Steve Fink [:sfink] [:s:]
	Comment 33 • 9 years ago

I think all of these actually boil down to a very small set of variants. The explosion of different variants is an artifact of how you have to trigger/configure them.

If I were to break things down into different dimensions, I'd say we have:

 - opt vs debug
 - platform (which could be split apart, eg win32 vs win64 = windows x (32 vs 64))
 - plain vs rootanalysis vs compacting (different ways of running the tests)
 - whether warnings are fatal or not
 - the arm-sim oddball (produces a native binary whose JIT generates ARM code that is executed via a simulator. Crazy, man.)

Though the plain vs rootanalysis vs compacting isn't always just different ways of running stuff. When a new mode (eg compacting garbage collection) is being developed, it's often gated by #ifdef JSGC_COMPACTING or whatever, and setting that requires a different configure option. Eventually it gets promoted into the default build, at which time the configure settings difference collapses. So maybe I'll combine that dimension with arm-sim and label things

  opt|debug / platform / plain|rootanalysis|compacting|arm-sim / fatalwarnings?

Some cells in the resulting matrix are not possible (eg we don't have an ARM simulator that runs on Windows, afaik), and populating all of them would result in way too many builds (something like 2x5x4x2=80?). The current set is really the compromise we've settled on.

Focusing only on linux64, I think it'd be great to have TC builds for all 2x1x4x2 possibilities, but only enable by default:

  opt/linux64/plain/fatal (currently: warnaserr)
  debug/linux64/plain/fatal (currently: warnaserrdebug)
  debug/linux64/rootanalysis/nonfatal (currently: rootanalysis)
  debug/linux64/compacting/nonfatal (currently: compacting)
  debug/linux64/arm-sim/nonfatal (currently: arm-sim)

Note that sadly opt vs debug is done via configure options, and therefore shows up as a different variant to autospider.sh, so there's no current way to populate the full matrix. Perhaps I should split these things out into separate autospider.sh args, and have it compose together multiple configure snippets.

As for what to change right now -- can you make plain and plaindebug not run by default on try? Or if not, just remove them. They were created for Windows, where we don't want fatal warnings. On linux64, they don't really add enough coverage to be worth running.

	Anhad Jai Singh (:ffledgling) Assignee
	Comment 34 • 9 years ago

(In reply to Steve Fink [:sfink, :s:] from comment #33)
> I think all of these actually boil down to a very small set of variants. The
> explosion of different variants is an artifact of how you have to
> trigger/configure them.
> 
> If I were to break things down into different dimensions, I'd say we have:
> 
>  - opt vs debug
>  - platform (which could be split apart, eg win32 vs win64 = windows x (32
> vs 64))
>  - plain vs rootanalysis vs compacting (different ways of running the tests)
>  - whether warnings are fatal or not
>  - the arm-sim oddball (produces a native binary whose JIT generates ARM
> code that is executed via a simulator. Crazy, man.)
> 
> Though the plain vs rootanalysis vs compacting isn't always just different
> ways of running stuff. When a new mode (eg compacting garbage collection) is
> being developed, it's often gated by #ifdef JSGC_COMPACTING or whatever, and
> setting that requires a different configure option. Eventually it gets
> promoted into the default build, at which time the configure settings
> difference collapses. So maybe I'll combine that dimension with arm-sim and
> label things
> 
>   opt|debug / platform / plain|rootanalysis|compacting|arm-sim /
> fatalwarnings?
> 
> Some cells in the resulting matrix are not possible (eg we don't have an ARM
> simulator that runs on Windows, afaik), and populating all of them would
> result in way too many builds (something like 2x5x4x2=80?). The current set
> is really the compromise we've settled on.
> 
> Focusing only on linux64, I think it'd be great to have TC builds for all
> 2x1x4x2 possibilities, but only enable by default:
> 
>   opt/linux64/plain/fatal (currently: warnaserr)
>   debug/linux64/plain/fatal (currently: warnaserrdebug)
>   debug/linux64/rootanalysis/nonfatal (currently: rootanalysis)
>   debug/linux64/compacting/nonfatal (currently: compacting)
>   debug/linux64/arm-sim/nonfatal (currently: arm-sim)
> 
> Note that sadly opt vs debug is done via configure options, and therefore
> shows up as a different variant to autospider.sh, so there's no current way
> to populate the full matrix. Perhaps I should split these things out into
> separate autospider.sh args, and have it compose together multiple configure
> snippets.

Splitting these out as different args in autospider.sh would be useful, but not really for the purposes of this bug I think. What would help greatly would be to separate the build and tests, so that you could pass proper flags to try accordingly and call say autospider_build.sh and autospider_test.sh with the right variables.

Opt vs debug is already determined by "-d" or "-o" in the try syntax, so is platform with -p ( (although we don't have anything except linux64 for these for now), so splitting the builds and tests would allow us to create that matrix without the same kind of explosion in try flags.

> 
> As for what to change right now -- can you make plain and plaindebug not run
> by default on try? Or if not, just remove them. They were created for
> Windows, where we don't want fatal warnings. On linux64, they don't really
> add enough coverage to be worth running.

Not sure what you mean by 'run by default', currently you have to specify the try flag for any and all of the spidermonkey builds to run them through taskcluster (The older behaviour for buildbot hasn't been changed afaik). Do you mean you want them disabled completely? Or do you mean these should build everytime on every push?

	Steve Fink [:sfink] [:s:]
	Comment 35 • 9 years ago

(In reply to Anhad Jai Singh (:ffledgling) from comment #34)
> (In reply to Steve Fink [:sfink, :s:] from comment #33)
> > Note that sadly opt vs debug is done via configure options, and therefore
> > shows up as a different variant to autospider.sh, so there's no current way
> > to populate the full matrix. Perhaps I should split these things out into
> > separate autospider.sh args, and have it compose together multiple configure
> > snippets.
> 
> Splitting these out as different args in autospider.sh would be useful, but
> not really for the purposes of this bug I think. What would help greatly
> would be to separate the build and tests, so that you could pass proper
> flags to try accordingly and call say autospider_build.sh and
> autospider_test.sh with the right variables.

To do that, you have to be able to package up the builds, upload them, and download them in the test job. Historically, that didn't really make much sense because it's a bunch of work and the spidermonkey jobs were all really fast so it would probably double the latency. (Also, the dependent build stuff in buildbot is gross.)

Nowadays, many of these jobs are insanely slow, so the latency argument no longer holds. :-( It would still be some effort to do the packaging/unpackaging, though. I'd be fine with splitting up autospider.sh, or maybe just adding --build and --test arguments or whatever, but I don't want to do the packaging stuff right now. It's totally worth filing a bug for, though. Actually, these jobs are now slow enough that I could even imagine splitting up the test suite (which is internally already multiple different test suites) so that they could run in parallel, reducing latency.

> Opt vs debug is already determined by "-d" or "-o" in the try syntax, so is
> platform with -p ( (although we don't have anything except linux64 for these
> for now), so splitting the builds and tests would allow us to create that
> matrix without the same kind of explosion in try flags.

Yeah, that makes sense.

> > As for what to change right now -- can you make plain and plaindebug not run
> > by default on try? Or if not, just remove them. They were created for
> > Windows, where we don't want fatal warnings. On linux64, they don't really
> > add enough coverage to be worth running.
> 
> Not sure what you mean by 'run by default', currently you have to specify
> the try flag for any and all of the spidermonkey builds to run them through
> taskcluster (The older behaviour for buildbot hasn't been changed afaik). Do
> you mean you want them disabled completely? Or do you mean these should
> build everytime on every push?

Oh. The buildbot config stuff has the capability to have jobs marked as "nondefault on try", which controls whether it's included in a -p all push. They'll all always be included in a -p linux64 push, which is not necessarily what you'd want. With your new -p sm-* flags, that'll have to work differently.

I think a try push that touches js/src and requests -p all should get the same set of jobs as you'd get on mozilla-inbound. If you touch js/src and request -p linux64, I'd prefer getting the same linux64 jobs as you would get on mozilla-inbound. If you don't touch js/src but request -p sm-compacting, I would prefer getting the compacting build (on whatever platforms it exists for.) But that is a little weird, since it means that the -p possibilities aren't orthogonal. They're not even pretending to be orthogonal any longer (which they are the way I hid all the spidermonkey builds under the existing platforms, mostly linux64).

To be specific, I'm saying that "-p linux64" should imply "default -p sm-* jobs, restricted to linux64" (where you currently get that restriction for free as a side effect of TC only working on linux!) And "default" there is determined by whether or not you touched js/src, plus a bit  (default on try vs not default on try) associated with each variant.

If we have to stop using the "did it touch js/src?" distinction, then the additional load probably isn't *too* bad. Maybe. Not sure.

Then again, I also kind of feel like the existing set of platforms needs to be broken down in other ways already (eg desktop vs android vs b2g, and someday iOS), and if there are any plans for refactoring along those lines then perhaps this should be wrapped up into that? I don't know if there are plans for that.