837936 - Need per-commit builds of B2G desktop build (on all mozilla-central merging trees)

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Jonathan Griffin (:jgriffin)]

In order to start running Gaia Unit tests in automation (bug 833666) we'll need to have b2g desktop builds built per-commit.  AFAICT, we are only generating them as nightlies right now.

The nightly b2g desktop build we're creating includes a bundled gaia, but this is a non-debug version.  The per-commit builds that we will use for Gaia Unit tests need a debug gaia - the output of 'DEBUG=1 make' in the gaia directory.

Comment 1

[Jonathan Griffin (:jgriffin)]

(In reply to Jonathan Griffin (:jgriffin) from comment #0)
> The per-commit builds that we will use for
> Gaia Unit tests need a debug gaia - the output of 'DEBUG=1 make' in the gaia
> directory.

Make that 'DEBUG=1 NOFTU=1 make'

Comment 2

[Jonathan Griffin (:jgriffin)]

(In reply to Jonathan Griffin (:jgriffin) from comment #1)
> (In reply to Jonathan Griffin (:jgriffin) from comment #0)
> > The per-commit builds that we will use for
> > Gaia Unit tests need a debug gaia - the output of 'DEBUG=1 make' in the gaia
> > directory.
> 
> Make that 'DEBUG=1 NOFTU=1 make'

More experimentation shows that we'll need to generate the gaia profile from within the mozharness script of the test job, so it doesn't matter how it's generated as part of the build...we could use the exact same job as produces the current nightly.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

Yes, please.

Yesterday, a Gaia commit caused these builds to break, but was not caught immediately due to the low checkin volume on the b2g18 branches. Having these builds on inbound/m-c would have enabled us to find this bustage much faster. Moreover, given that we're frequently uplifting patches from m-c to the release branches where these builds are generated per-commit, we should be making sure we aren't breaking them beforehand.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Another data point - the moz.build landing broke these nightlies on m-c, but due to their hidden status, nobody noticed.

Comment 5

[Aki Sasaki (not active)]

(In reply to Jonathan Griffin (:jgriffin) from comment #0)
> In order to start running Gaia Unit tests in automation (bug 833666) we'll
> need to have b2g desktop builds built per-commit.  AFAICT, we are only
> generating them as nightlies right now.

It's really easy to turn on depend desktop builds.

> The nightly b2g desktop build we're creating includes a bundled gaia, but
> this is a non-debug version.  The per-commit builds that we will use for
> Gaia Unit tests need a debug gaia - the output of 'DEBUG=1 make' in the gaia
> directory.

However, this step needs more info.

* I don't think we currently run make in gaia explicitly currently.  When should we do this?  Before make -f client.mk build, or after, but before adding locales, or ...?
* Does it make sense to do a DEBUG=1 make in an opt desktop build? Or do we need to turn on debug desktop builds?

Comment 6

[Jonathan Griffin (:jgriffin)]

(In reply to Aki Sasaki [:aki] from comment #5)
> * I don't think we currently run make in gaia explicitly currently.  When
> should we do this?  Before make -f client.mk build, or after, but before
> adding locales, or ...?
> * Does it make sense to do a DEBUG=1 make in an opt desktop build? Or do we
> need to turn on debug desktop builds?

It turns out we'll need to build gaia on the test slaves (I already verified we can) in the mozharness script for these tests, so we don't need to worry about DEBUG=1 or gaia's make; we can use the exact same mechanism that is used to generate the B2G nightlies.

Comment 7

[Phil Ringnalda (:philor)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #4)
> Another data point - the moz.build landing broke these nightlies on m-c, but
> due to their hidden status, nobody noticed.

Not quite literally true - I've noticed that particular bustage repeatedly, and told several people, some of whom expressed temporary interest, but I didn't feel like filing a bug about it if absolutely nobody was missing them.

Comment 8

[Aki Sasaki (not active)]

Attachment: depend desktop builds

Localizer builds are still nightly-only.
This patch has the side effect of enabling b2g desktop builds on Try, which seems desirable.
Passes test-masters.sh.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

FYI, desktop B2G builds are currently broken on Windows (bug 852107).

Comment 10

[Aki Sasaki (not active)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)
> FYI, desktop B2G builds are currently broken on Windows (bug 852107).

I'm going to guess this isn't blocking landing this bug?

Comment 11

[Ed Morley [:emorley]]

(In reply to Aki Sasaki [:aki] from comment #10)
> (In reply to Ryan VanderMeulen [:RyanVM] from comment #9)
> > FYI, desktop B2G builds are currently broken on Windows (bug 852107).
> 
> I'm going to guess this isn't blocking landing this bug?

Correct, we can hide them for now :-)

Comment 12

[Aki Sasaki (not active)]

Comment on attachment 725665 [details] [diff] [review]
depend desktop builds

http://hg.mozilla.org/build/buildbot-configs/rev/384a353ceae9

Comment 13

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

This is in production now.

Comment 14

[Aki Sasaki (not active)]

I see some desktop Beegees on inbound.

Comment 15

[Jonathan Griffin (:jgriffin)]

(In reply to Aki Sasaki [:aki] from comment #14)
> I see some desktop Beegees on inbound.

Awesome, thanks!

Comment 16

[Aki Sasaki (not active)]

* make check appears to have issues in these builds; that would seem to be a separate bug, unless we need those to be turned off.
* try builds aren't pulling gaia, and are perma-red. Reopening for this issue.

Comment 17

[Jonathan Griffin (:jgriffin)]

We don't do 'make check' on B2G desktop builds on mozilla-b2g18, so I'm guessing we should turn them off here as well.  But, I'll file a bug for the current failures since people may be interested.

Comment 18

[Aki Sasaki (not active)]

Attachment: fix gaia source on try builds

TryBuildFactory completely overrides MercurialBuildFactory.addSourceSteps(), which contains the gaia logic.

I'm basically pulling the gaia bits out of addSourceSteps(), into addGaiaSourceSteps().  Then I'm calling that method from both MercurialBuildFactory.addSourceSteps() and TryBuildFactory.addSourceSteps().

The diff chose to represent this in a weird way; hopefully it's clear.

I'm pretty sure this'll fix it; let me know if you need me to test further than test-masters.sh.

Comment 19

[Jonathan Griffin (:jgriffin)]

'make check' problem filed as bug 852320.

Comment 20

[Aki Sasaki (not active)]

Weird, we explicitly globally turn on 'enable_checktests' for these platforms, then turn them off on b2g18 and b2g18_v1_0_1.

Easy enough to turn them off everywhere; patch incoming.

Comment 21

[Aki Sasaki (not active)]

Attachment: disable b2g desktop checktests everywhere

Haven't been able to figure out why these are on for non-b2g18* branches, and they appear to be perma-orange.

Comment 22

[Ed Morley [:emorley]]

(In reply to Aki Sasaki [:aki] from comment #21)
> Created attachment 726380 [details] [diff] [review]
> disable b2g desktop checktests everywhere
> 
> Haven't been able to figure out why these are on for non-b2g18* branches,
> and they appear to be perma-orange.

Since bug 852320 has now hopefully just been fixed, do we need to turn these off? Should we try to enable them on b2g18 too?

Comment 23

[Aki Sasaki (not active)]

I imagine we'll want that patch to land in b2g18*, and any other fixes that aren't uplifted, before we turn them on there.

I'm happy to do whatever that results in good use of compute time and useful information.  If bug 852320 is fixed on inbound-only, I think that means I should obsolete this patch and leave things the way they are?

Comment 24

[Ed Morley [:emorley]]

(In reply to Aki Sasaki [:aki] from comment #23)
> If bug 852320 is fixed on inbound-only, I think that means I
> should obsolete this patch and leave things the way they are?

I would imagine so; who from the B2G team can we get to weigh in on whether we want them enabled for all trees (not that it need block this bug)?

Comment 25

[Phil Ringnalda (:philor)]

That was only one of the two failures showing so far, and it remains to be seen whether there were others below that one which put a stop to everything.

Comment 26

[Jonathan Griffin (:jgriffin)]

(In reply to Ed Morley [:edmorley UTC+0] from comment #24)
> (In reply to Aki Sasaki [:aki] from comment #23)
> > If bug 852320 is fixed on inbound-only, I think that means I
> > should obsolete this patch and leave things the way they are?
> 
> I would imagine so; who from the B2G team can we get to weigh in on whether
> we want them enabled for all trees (not that it need block this bug)?

Sounds like an issue for Jonas.  Jonas, is there value in running 'make check' on B2G desktop builds?  Currently we're not doing so on mozilla-b2g18, and there is a problem or two with these tests on mozilla-inbound, where they've just been enabled.

Comment 27

[[e-mail] (:bhearsum)]

Comment on attachment 726380 [details] [diff] [review]
disable b2g desktop checktests everywhere

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

It sounds like you probably don't want to land this, given comment #24, but r+ in case you need to.

Comment 28

[[e-mail] (:bhearsum)]

Comment on attachment 726377 [details] [diff] [review]
fix gaia source on try builds

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

I don't see a gecko l10n root being set for try: https://github.com/mozilla/build-buildbot-configs/blob/master/mozilla/b2g_config.py#L926. I think that's going to make this patch fail at runtime.

::: process/factory.py
@@ +1180,1 @@
>          if self.gaiaRepo:

I think it'd be clearer to move this check to addSourceSteps so that we avoid calling addGaiaSourceSteps unless it's actually going to do something.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

Hidden on m-c due to bug 852543.

Comment 30

[Phil Ringnalda (:philor)]

Hidden on try due to too many people asking me why there was a red build that had nothing to do with them on their try push.

Comment 31

[Aki Sasaki (not active)]

Attachment: try gecko_l10n_root

Good catch.

Comment 32

[Aki Sasaki (not active)]

Attachment: fix gaia source on try builds take 2

Comment 33

[Aki Sasaki (not active)]

Comment on attachment 726780 [details] [diff] [review]
try gecko_l10n_root

http://hg.mozilla.org/build/buildbot-configs/rev/d7a16feeb117

Comment 34

[Aki Sasaki (not active)]

Comment on attachment 726782 [details] [diff] [review]
fix gaia source on try builds take 2

http://hg.mozilla.org/build/buildbotcustom/rev/d18bddd3a1d1

Comment 35

[Ed Morley [:emorley]]

Reason for b2g18 make check being disabled:
https://bugzilla.mozilla.org/show_bug.cgi?id=821401#c20

Comment 36

[Aki Sasaki (not active)]

Heh, I guess I've forgotten.

Bug 853024 and history working in these 1.0 projects leads me to believe that test failures are going to be low priority compared to shipping features and fixing blocker bugs.  I'm inclined to disable |make check| everywhere until that changes.

Comment 37

[Aki Sasaki (not active)]

Comment on attachment 726380 [details] [diff] [review]
disable b2g desktop checktests everywhere

I'm making a call here in the absence of any other information.
Disabling |make check|, since a broken build would cause dev action; broken tests will just hide broken builds.

Easy enough to reenable should dev priorities change.

http://hg.mozilla.org/build/buildbot-configs/rev/5a8166d1c4bd

Comment 38

[Aki Sasaki (not active)]

Attachment: enable make check for try only

For RyanVM.  Allows devs to green these up without hiding/wasting build cycles on the other branches.

Comment 39

[[e-mail] (:bhearsum)]

Comment on attachment 727345 [details] [diff] [review]
enable make check for try only

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

How long do these tests take? It still seems wasteful to turn these on if no one is _actively_ looking at them.

Comment 40

[Ryan VanderMeulen [:RyanVM]]

I'm intending to at least get the broken tests disabled within the next day or two. If I can't run these builds on Try, I don't know how I'll be able to do so.

Comment 41

[[e-mail] (:bhearsum)]

Comment on attachment 727345 [details] [diff] [review]
enable make check for try only

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

shrug

Comment 42

[Aki Sasaki (not active)]

Comment on attachment 727345 [details] [diff] [review]
enable make check for try only

http://hg.mozilla.org/build/buildbot-configs/rev/2e1f693fa05b

Comment 43

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

(In reply to Jonathan Griffin (:jgriffin) from comment #26)
> Sounds like an issue for Jonas.  Jonas, is there value in running 'make
> check' on B2G desktop builds?  Currently we're not doing so on
> mozilla-b2g18, and there is a problem or two with these tests on
> mozilla-inbound, where they've just been enabled.

I don't actually know. Needinfo'ing some people that might.

Comment 44

[Ryan VanderMeulen [:RyanVM]]

The fact that they're failing when desktop Firefox builds aren't is worrying IMO...

Comment 45

[Ed Morley [:emorley]]

(In reply to Ben Hearsum [:bhearsum] from comment #39)
> Comment on attachment 727345 [details] [diff] [review]
> enable make check for try only
> 
> How long do these tests take? It still seems wasteful to turn these on if no
> one is _actively_ looking at them.

Or we could use |try_by_default: False|

Comment 46

[Ryan VanderMeulen [:RyanVM]]

I'm perfectly fine with checktests being off by default as long as there's *some* way for me to push with them enabled.

Comment 47

[Ed Morley [:emorley]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #46)
> I'm perfectly fine with checktests being off by default as long as there's
> *some* way for me to push with them enabled.

try_by_default is at the job level; make check would have to be on for all try B2g desktop builds (which is fine) - I'm just saying that if bhearsum was concerned about load, we could just make the B2G desktop builds not run by default (ie the try feature sfink recently added).

Comment 48

[Armen [:armenzg]]

Merged change from this bug and I reconfigured the masters.

Comment 49

[Aki Sasaki (not active)]

I think we're done here, though we could still use further information about whether we care about |make check| on other branches.

-> RESO FIXED; reopen or file new to enable |make check| elsewhere (or disable on try, or w/e).

Comment 50

[Ryan VanderMeulen [:RyanVM]]

Thanks for all the help, aki! Agreed that any follow-up work can be done in other bugs.