972089 - Create SpiderMonkey builds for Windows in automation again

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

As per bug 971426 comment 9,

We've had standalone SpiderMonkey builds for Linux on TBPL. We used to have them for Windows, until bug 785798 removed them due to breakage of some sort. Not having standalone builds for the platform that most of our users are on (though not devs) seems ... slightly counter-intuitive.

As a result, we have had to deal with compilation failures which restricts the amount of fuzzing I can do as well as bisection:

Bug 948301
Bug 951587 (64-bit - but that's where we're headed to, eventually)
Bug 971426

(and more might be coming, as :glandium mentioned that he might be rewriting more of the SpiderMonkey build code)

The last one was particularly terrible - the regressing patch landed mid-Jan, but I was out on PTO and by the time I got back and tested that it was indeed a build failure, it was already early Feb. In other words, the build failure lasted a few weeks.

Please reinstate standalone SpiderMonkey builds for Windows. Releng might want a re-assurance that someone will be on the ball to fix build errors as necessary, but I'll leave it to the JS folks to decide.

Comment 1

[Phil Ringnalda (:philor)]

See bug 778469 comment 9 - this is number 3, and along with not having number 0 I don't think you have number 1 or number 2, since your log in bug 971426 has warnings in it.

Comment 2

[Phil Ringnalda (:philor)]

And just to be crystal clear, "run SpiderMonkey warnings-as-errors builds on Windows again" is absolutely unfixable until some JS hacker says directly to me, "yeah, every single time that we pick up a Windows-only warning, I'm personally on the hook for having you yell at me until I fix it."

Comment 3

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I've posted to https://groups.google.com/d/msg/mozilla.dev.tech.js-engine.internals/jbNorfOVcZ0/n8SYHAZPjgoJ to give the JS folks a headsup.

I cannot fix JS build errors, but I also cannot test Windows builds if I cannot compile their builds, so this ball unfortunately is in the JS folks' court (I can't decide for them who should fix their builds!), so only someone from their team can answer your comment 1.

(In reply to Phil Ringnalda (:philor) from comment #2)
> And just to be crystal clear, "run SpiderMonkey warnings-as-errors builds on
> Windows again" is absolutely unfixable until some JS hacker says directly to
> me, "yeah, every single time that we pick up a Windows-only warning, I'm
> personally on the hook for having you yell at me until I fix it."

I'll leave it to them to decide whether they would like a normal build or a warnings-as-errors build.

Setting needinfo? from jorendorff, JS module owner.

Comment 4

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

To add on, ideally there should at least be an equivalent js binary with the same configure options that we ship Windows Firefox with. (i.e. without warnings-as-errors, for a start)

Secondly, even with a warnings-as-errors build, if someone lands a patch that causes an orange/red in a warnings-as-errors build, the patch should be backed out.

This can be accomplished as per Valgrind by compiling standalone SM on multiple branches (mozilla-inbound, fx-team) and per-commit - the overhead isn't large as it only takes a few minutes to compile each shell.

Spoke to Waldo in-person, he thinks sfink might know more.

Comment 5

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

> This can be accomplished as per Valgrind by compiling standalone SM on
> multiple branches (mozilla-inbound, fx-team)

and Try server too, of course.

Comment 6

[Steve Fink [:sfink] [:s:]]

(In reply to Gary Kwong [:gkw] [:nth10sd] catching up on email/bugmail from comment #4)
> To add on, ideally there should at least be an equivalent js binary with the
> same configure options that we ship Windows Firefox with. (i.e. without
> warnings-as-errors, for a start)
> 
> Secondly, even with a warnings-as-errors build, if someone lands a patch
> that causes an orange/red in a warnings-as-errors build, the patch should be
> backed out.

AIUI, making warnings-as-errors tier1 hasn't really worked for us. The current setup, where they're all hidden but philor keeps an eye on them and yells at whoever breaks them, *does* seem to be working afaict. They don't get fixed immediately, but they're green right now and to me that means that the process works -- warnings are not building up over time.

> This can be accomplished as per Valgrind by compiling standalone SM on
> multiple branches (mozilla-inbound, fx-team) and per-commit - the overhead
> isn't large as it only takes a few minutes to compile each shell.

Sure, adding a warnings-as-errors Windows build is just a matter of backing out 219fae018c4d from bug 785798. But personally, I don't think the nuisance :: value threshold is nearly high enough for Windows warnings-as-errors, so I don't think we should revive them. I don't think Windows-only warnings are likely enough to reveal real bugs as compared to the trouble of getting a whole bunch of developers to occasionally have to set up an unfamiliar build environment just to suppress some nuisance warning. If we really really think that Windows warnings have substantial value, I'd still argue for them to be nightly (or even better, weekly) to encourage a model where someone can come along occasionally and fix them all up. Yes, some people will hate me for saying that. :)

Anyway, I would propose enabling the SM(ggc) builds on Windows and using that as our Windows shell builds. It should be as easy to do as backing out bug 785798. In fact, I'll do a try push with the equivalent setup right now, since I understand glandium just repaired the Windows shell builds. We'd still have to agree to keep these builds alive. I'd prefer for them to be unhidden, unlike the warnings-as-errors builds. philor's list in bug 778469 comment 9, which I wholeheartedly agree with, is much easier to fulfill for warnings-as-warnings ggc builds. (philor, if you disagree, please let me know.)

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

> Anyway, I would propose enabling the SM(ggc) builds on Windows and using
> that as our Windows shell builds.

How about going all the way and enabling it for Mac as well? Then we'll have the dream team of Win/Linux/Mac SM(ggc) builds.

Comment 8

[Steve Fink [:sfink] [:s:]]

Working on this.

Comment 9

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Steve Fink [:sfink] from comment #8)
> Working on this.

Thanks, Steve!

Comment 10

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Steve, any updates on this?

Also, it will be nice to also have ARM-simulator builds (32-bit to start off, and also eventually 64-bit ones) on treeherder. Randorderfuzz (bug 1100132) found issues with running just jit-tests using ARM-simulator builds. Will a new bug be needed?

Comment 11

[Jan de Mooij [:jandem]]

(In reply to Gary Kwong [:gkw] [:nth10sd] GMT+8 after Oct 9 from comment #10)
> Also, it will be nice to also have ARM-simulator builds (32-bit to start
> off, and also eventually 64-bit ones) on treeherder. Randorderfuzz (bug
> 1100132) found issues with running just jit-tests using ARM-simulator
> builds. Will a new bug be needed?

We test the ARM simulator on Linux 32-bit. It also runs jit-tests.

https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-inbound&revision=465b7f68072b&searchQuery=arm-sim

Comment 12

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Jan de Mooij [:jandem] from comment #11)
> We test the ARM simulator on Linux 32-bit. It also runs jit-tests.

Makes one wonder why bug 1100083 wasn't caught then, but we'll continue this elsewhere.

Comment 13

[Steve Fink [:sfink] [:s:]]

Yeah, the latency on this bug is measured in years. :(

There are some (nondefault) builds available already on try. Which is good, because it allows me to test the necessary autospider.sh changes. Apparently I have to set custom paths because the environment isn't already set up. Which is bogus -- that means that we're committing hardcoded paths into the Gecko tree (eg build/win32/mozconfig.vs2010). It seems to me like the automation environment ought to feed in the paths where it put things.

But I'll try hacking through it for now, with some disgusting config duplication.

Latest push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=7c4b4df8b288

Comment 14

[Steve Fink [:sfink] [:s:]]

\o/

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=c4525d366173

My changes are really ugly, and I'm not sure what I'll need to do to get them landed. But at least it works again finally.

Comment 15

[Steve Fink [:sfink] [:s:]]

Attachment: Fix paths for Windows SM(...) builds

Hopefully this patch won't try glandium's patience too much.

Sadly, we seem to store paths for finding the relevant Windows compiler in mozconfig files, which results in some interesting techniques for getting at them from a shell script.

Comment 16

[Mike Hommey [:glandium]]

Comment on attachment 8531935 [details] [diff] [review]
Fix paths for Windows SM(...) builds

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

I want to stab my eyes but meh.

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d467571ac703

Comment 18

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Is this really fixed? I don't see SpiderMonkey Windows builds on:

https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-searchStr=sm

Comment 19

[Steve Fink [:sfink] [:s:]]

(In reply to Gary Kwong [:gkw] [:nth10sd] GMT+8 Dec 12 - 22 from comment #18)
> Is this really fixed? I don't see SpiderMonkey Windows builds on:
> 
> https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&filter-
> searchStr=sm

No, sorry, I should have tagged this with [leave-open]. Currently, you can explicitly request these builds on try with |try: -p win32,win64|, but nothing runs automatically there nor anywhere else. The jobs do seem to be green now, so it's about time to try to get them running for real.

I don't think the win64 ones are actually win64 yet, though, so I'll skip them for now.

Comment 20

[Steve Fink [:sfink] [:s:]]

Attachment: Add in win32 spidermonkey builds

On try, run a win32-debug build by default (for anything touching js/src, that is.)

On mozilla-central and related trees, make plain win32-debug builds and generational builds. For win32 opt, just make plain builds. ("warnaserr" and "warnaserrdebug" are the plain builds; the "warn as err" part is irrelevant for MSVC.)

The logic is that generational (which is really compacting GC) should catch more or less a superset of problems, but only when run on a debug build. So we build win32-debug generational on both try and mozilla-central.

Then in addition, we make plain win32 and win32-debug builds on the main trees because those are the builds that will be useful for bisecting and fuzzing and whatever. In theory, those should rarely break if the win32-debug generational build is not broken, and I'm opting to be as conservative as possible with Windows build resources. Afaict, this is all kosher with respect to the sheriff job visibility policies.

Comment 21

[Steve Fink [:sfink] [:s:]]

https://hg.mozilla.org/build/buildbot-configs/rev/2bf0e905fbf7

I'll admit that I've kind of forgotten the exact state of this and bug 1106707, but I really want to just get it done finally.

Comment 22

[Chris Cooper [:coop] (he/him)]

In production: https://hg.mozilla.org/build/buildbot-configs/rev/2bf0e905fbf7

Comment 23

[Phil Ringnalda (:philor)]

Completely broken, hidden on all trees.

Comment 24

[Steve Fink [:sfink] [:s:]]

Yes, it's not getting a configuration option passed through, which was supposed to happen with bug 1106707. Looks like I'm pulling it from the wrong place.

Comment 25

[Steve Fink [:sfink] [:s:]]

They're back!

Comment 26

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.