Our Unagi engineering builds [1] are missing the test apps (like Keyboard, etc.) Possible regression from bug 840609? [1] https://pvtbuilds.mozilla.org/pub/mozilla.org/b2g/nightly/mozilla-b2g18-unagi-eng/latest/

(Told I should move this -> Gaia, from Builds.)

(In reply to Stephen Donner [:stephend] from comment #0) > Possible regression from bug 840609? Yes. Any reason to have DOGFOOD=1 for eng builds ? The sole reason to have DOGFOOD=1 (or PRODUCTION=1 for that matter) is to change which apps to push and to set GAIA_OPTIMIZE=1 and B2G_SYSTEM_APPS=1 (which releng config is already doing BTW).

See bug 840609.

aki, I know very well this bug, nothing there says why we actually need DOGFOOD=1. Could you please explain what the rationale was ?

"Set DOGFOOD=1 in the environment for b2g (phone) builds (to enable test SUPL server)" See also https://bugzilla.mozilla.org/show_bug.cgi?id=837839

Thanks, I can read ;) Let me rephrase: why DOGFOOD=1 is necessary for SUPL servers ?

No idea. Ask cjones? We were getting a lot of pressure to enable DOGFOOD=1 for SUPL.

Vivien, any insight you might be able to add, here?

The breakage seems to be caused by Bug 851281. We should back this one out and get a clear picture of everyone's needs before re-landing.

I strongly disagree. Backing out bug 851281 will bring a bigger breakage than this one. The real breakage was done by adding DOGFOOD=1 to the build configuration without a clear understanding of the consequences, this already produced 3 regressions. If one should be backed out, it is Bug 840609.

My take is : user builds should have PRODUCTION=1 (this is Bug 854454), eng builds should have nothing special beside GAIA_OPTIMIZE=1 and B2G_SYSTEM_APPS=1. But as Fabrice said, we need to understand why we needed Bug 840609 in the first place. Could this wait a few days or does this block real work ?

(In reply to Julien Wajsberg [:julienw] from comment #11) > My take is : user builds should have PRODUCTION=1 (this is Bug 854454), eng > builds should have nothing special beside GAIA_OPTIMIZE=1 and > B2G_SYSTEM_APPS=1. But as Fabrice said, we need to understand why we needed > Bug 840609 in the first place. > > Could this wait a few days or does this block real work ? It can wait a few days (the app we've actively supported and developed tests against is the keyboard, which we've xfailed in https://github.com/mozilla/gaia-ui-tests/commit/32c635d2a6cb5f47355a94ef5a243acac6d11d60). Still, I'm not sure whom else might be affected by this issue.

So DOGFOOD=1 was added for all builds because that's what controls that the prefs for the supl (a-gps) servers are used. Then Bug 851281 made |DOGFOOD=1| behave like |make dogfood| because otherwise v1.0.1 builds were borked. But as a result it made all apps but production apps + feedback disappear. I'll see how to change how the supl prefs are added, and then change the build configurations, which should fix this.

Confirmed with Stephen over IRC that the keyboard app is called "UI Tests". The DOGFOOD/PRODUCTION variables feel very overloaded in what they control, but that's for another bug. This patch adds the test_apps directory to the list of apps included in a DOGFOOD=1 build, which seems to accomplish what this bug is aiming to do. Without this patch: unset DOGFOOD ; make profile # includes uitest.gaiamobile.org DOGFOOD=1 make profile # does not include uitest.gaiamobile.org PRODUCTION=1 make profile # does not include uitest.gaiamobile.org With this patch: unset DOGFOOD ; make profile # includes uitest.gaiamobile.org DOGFOOD=1 make profile # includes uitest.gaiamobile.org PRODUCTION=1 make profile # does not include uitest.gaiamobile.org From my reading of the bug, this seems like the right thing to do.

Comment on attachment 730280 [details] [diff] [review] include the test_apps for DOGFOOD=1 Review of attachment 730280 [details] [diff] [review]: ----------------------------------------------------------------- ::: Makefile @@ +79,4 @@ > endif > > ifeq ($(DOGFOOD), 1) > +GAIA_APP_SRCDIRS+=dogfood_apps test_apps Uhh...isn't this going to cause daily nightly builds to include test apps by default? I don't think that's what we should be doing here.

Agreed with Jason. I'm really making progress here (see Bug 837839 comment 11) and only waiting for fabrice's opinion on this. I'm confident to fix this properly before the end of this week. Hope that's soon enough for you ?

(In reply to Julien Wajsberg [:julienw] from comment #16) > Agreed with Jason. > > I'm really making progress here (see Bug 837839 comment 11) and only waiting > for fabrice's opinion on this. I'm confident to fix this properly before the > end of this week. Hope that's soon enough for you ? If you're asking me, that's fine, yes -- doing it the right way is best, so I can wait :-) Thanks!

I found out that the original goal for adding DOGFOOD=1 in the first place is now achieved without this env variable, so I asked to remove this from our build configurations in Bug 855673.

Comment on attachment 730280 [details] [diff] [review] include the test_apps for DOGFOOD=1 marking this as obsolete, we don't want this. :)

Could you please now check the new builds are correct ?

(In reply to Julien Wajsberg [:julienw] from comment #20) > Could you please now check the new builds are correct ? Yes, yesterday's were fine -- please mark fixed and I'll verify; thanks!

fixed in Bug 855673

Verified FIXED with today's build; thanks!