773015 - [*AMO] Upgrade spidermonkey

Status: RESOLVED FIXED

(Cloud Services :: Operations: Marketplace, task)

Description

[Wil Clouser [:clouserw]]

+++ This bug was initially created as a clone of Bug #650728 +++

Please upgrade the spidermonkey installs on the AMO boxes - as before, this is only needed on the celeryd boxes and the hudson master/slaves.

Bug 618173 has step by step instructions for building spidermonkey.  Thanks.

Comment 1

[Jason Thomas [:jason]]

Upgraded spidermonkey on hosts.

Comment 2

[Wil Clouser [:clouserw]]

(In reply to Jason Thomas [:jason] from comment #1)
> Upgraded spidermonkey on hosts.

Can you specify which hosts?  (ie. did this hit production already?)

Comment 3

[Jason Thomas [:jason]]

(In reply to Wil Clouser [:clouserw] from comment #2)
> (In reply to Jason Thomas [:jason] from comment #1)
> > Upgraded spidermonkey on hosts.
> 
> Can you specify which hosts?  (ie. did this hit production already?)

This was updated on all hosts, dev,stage and prod.

Comment 4

[Wil Clouser [:clouserw]]

(In reply to Jason Thomas [:jason] from comment #3)
> (In reply to Wil Clouser [:clouserw] from comment #2)
> > (In reply to Jason Thomas [:jason] from comment #1)
> > > Upgraded spidermonkey on hosts.
> > 
> > Can you specify which hosts?  (ie. did this hit production already?)
> 
> This was updated on all hosts, dev,stage and prod.

And hudson?

Comment 5

[Kris Maglione [:kmag]]

Did someone run the validator test suite against the latest Spidermonkey before the upgrade? Next time we should upgrade dev at least a week or so before prod as changes to the reflect output have potential to cause serious breakage.

Comment 6

[Jason Thomas [:jason]]

Yes, this was also added to jenkins1.dmz.phx1.mozilla.com.

Comment 7

[Matt Basta [:basta]]

When the validator encounters a node type that it doesn't recognize, it tries to traverse each branch as an expression. If it fails, it simply continues traversing where it left off. Upgrading Spidermonkey should prevent the compilation errors form happening and the validator should continue to run as it normally does (sans comprehensive support for the new JS constructs).

Comment 8

[Kris Maglione [:kmag]]

Jason, what exact Spidermonkey version are we using now? We're still getting parse errors for features that were added in Gecko 13.

Comment 9

[Jason Thomas [:jason]]

(In reply to Kris Maglione [:kmag] from comment #8)
> Jason, what exact Spidermonkey version are we using now? We're still getting
> parse errors for features that were added in Gecko 13.

Is this right way to get the version?

[[e-mail]1 ~]$ tracemonkey 
js> version()
185

Comment 10

[Kumar McMillan [:kumar]]

jason, the executable path we look for is `js` unless the SPIDERMONKEY setting has been configured to use tracemonkey. Can you check the version of js and/or check the settings for AMO? The default is None (which falls back to js) https://github.com/mozilla/zamboni/blob/master/lib/settings_base.py#L1219

Comment 11

[Jason Thomas [:jason]]

Current AMO settings:

settings_local_base.py:SPIDERMONKEY = '/usr/bin/tracemonkey'

Comment 12

[Kris Maglione [:kmag]]

The return value of version() isn't all that useful at the moment. If the following snippet runs without issue it should be sufficient:

for (a of []) ;

Comment 13

[Jason Thomas [:jason]]

[[e-mail]1 ~]$ tracemonkey 
js> for (a of []) ;              
typein:2: SyntaxError: missing ; after for-loop initializer:
typein:2: for (a of []) ;
typein:2: .......^

Comment 14

[Kris Maglione [:kmag]]

That's not new enough, then.

Comment 15

[Jason Thomas [:jason]]

I tried building this again with instructions in Bug 618173 with the same results. Can someone verify that these instructions are correct?

[[e-mail]1 spidermonkey]$ hg summary
parent: 73897:74b2b46fca7d tip
 Dummy merge to revert tracemonkey to mozilla-inbound tip so we can run TM-specific tests on the CLOSED TREE known as tracemonkey (r=testing-gods)
branch: default
commit: (clean)
update: (current)

[[e-mail]1 Linux_DBG.OBJ]$ ./js
js> for (a of []) ;
typein:1: SyntaxError: missing ; after for-loop initializer:
typein:1: for (a of []) ;
typein:1: .......^

Comment 16

[Kris Maglione [:kmag]]

Oh... you should probably be building from mozilla-central. The last update to that tree was in 2011.

Comment 17

[Jason Thomas [:jason]]

I rebuilt js using mozilla-central. I have deployed it on -dev and jenkins.

[[e-mail]1 ~]# tracemonkey 
js> for (a of []) ;
js> 

Let me know when it is okay to roll on stage/prod.

Comment 18

[Kris Maglione [:kmag]]

I just ran the test suite against the shell in m-c and it looks like it now requires running options("allow_xml") in order for E4X to work, so the upgrade needs to wait for the validator to support that.

Comment 19

[Matt Basta [:basta]]

Is "allow_xml" required even for the parser API?

Comment 20

[Kris Maglione [:kmag]]

Yep. Like I said, tests were failing if they used E4X.

Comment 21

[Matt Basta [:basta]]

Fixed:

https://github.com/mozilla/amo-validator/commit/f3b588eb997b67c8e972a0d2f2f271c9596f166d

Comment 22

[Kris Maglione [:kmag]]

Thanks, Matt. Reopening until Spidermonkey is actually updated.

I think it should be OK to update dev and stage now. Maybe wait a couple of days for prod?

Comment 23

[Jason Thomas [:jason]]

Update is already on dev. I just installed this on stage.

Comment 24

[Jason Thomas [:jason]]

Are we okay to update this in prod on the next code push?

Comment 25

[Matt Basta [:basta]]

The validator has been updated, so it's fine to push the Spidermonkey update.

Comment 26

[Jason Thomas [:jason]]

Updated on all prod hosts.