1074200 - Build should fail on error

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Build, defect)

Description

[Dale Harvey (:daleharvey)]

Put a console.log in some build file to debug, console isnt defined, that apps build errors.

I expect the makefile to quit now, its hard to notice what is an error and what is just random things being spit out

Comment 1

[Alexandre Poirot [:ochameau]]

We have some explicit code trying to do that right:
https://github.com/mozilla-b2g/gaia/blob/master/build/xpcshell-commonjs.js#L111-L118
// When an xpcshell module throws exception which is already captured by
// JavaScript module, some exceptions will make xpcshell returns error code.
// We put quit(0); to override the return code when all exceptions are
// handled in JavaScript module.
  quit(0);
} catch(e) {
  dump('Exception: ' + e + '\n' + e.stack + '\n');
  throw(e);

But it either changed over time or xpcshell doesn't always exit with an error code when an exception hits its top level execution thread.
I would say we would just need calling quit(1) instead of throw(e).

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 24945

That may just be it, but I could easily expect many failure to happen now because of this strictier code...
Let's see what try says to start with!

Comment 3

[Alexandre Poirot [:ochameau]]

This behavior is really hurting us badly.
For ex in bug 1053703, we could have seen immediatly that builds were broken instead of looking at why mochitest/reftest were so broken!

Comment 4

[Alexandre Poirot [:ochameau]]

Comment on attachment 8501751 [details] [review]
Pull request 24945

This patch may raise many error and introduce intermittent in gaia,
but it would help us a lot detecting issue in build system.
I would like to spawn some more try run before landing such patch to ensure we don't introduce obvious intermittents.

Comment 5

[Alexandre Poirot [:ochameau]]

A gecko try with more tests, just to ensure that everything is green:
  https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ce7f7571ff08

Comment 6

[(Inactive after June) George Duan [:gduan] [:喬智]]

Comment on attachment 8501751 [details] [review]
Pull request 24945

Thanks !! That would helps a lot!

Comment 7

[Alexandre Poirot [:ochameau]]

Sherrifs, this changeset may introduce orange.
It will make gaia build scripts to throw and stop if any exception happens.
Whereas, before this patch we just ignore any exception and continue...

I spawn a custom gecko try and several gaia try runs and it looks as green as before this patch.
(I just rebased the gaia patch to spawn yet another try run)

Comment 8

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/6108f0f04fea446cba5935d72b5cbab3deadd9af