994985 - All Tarako builds are userdebug, not user or eng

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Jonathan Griffin (:jgriffin)]

Both the "regular" and _eng versions of the Tarako builds being produced for 1.3T are userdebug builds.  Typically, the regular build is a user build, and the eng build is an eng build.

Is this intentional?  If so, we probably don't need to produce both builds, since they look the same.

Comment 1

[Dave Hylands [:dhylands]]

This is caused by:
https://github.com/mozilla-b2g/B2G/blob/master/config.sh#L138

forcing LUNCH to be userdebug forces the VARIANT to be ignored.

Comment 2

[Jonathan Griffin (:jgriffin)]

Vincent, I think you own this line in config.sh:

https://github.com/mozilla-b2g/B2G/blob/master/config.sh#L138

Is it intentional that all Tarako builds should be userdebug?  Do we not care about user and eng builds?

Comment 3

[Vincent Liu[:vliu]]

There is no any special reason why we uses userdebug as default build but integrating from partners. Tarako has limited resource and currently we tend to run some tests to make it build more stable. For shipping purpose, partners will change to user build.

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

Interesting; we're using builds from https://pvtbuilds.mozilla.org/pvt/mozilla.org/b2gotoro/tinderbox-builds/mozilla-b2g28_v1_3t-tarako-eng/latest/ for the perf tests (http://selenium.qa.mtv2.mozilla.com:8080/view/B2G%20Tarako/job/b2g.tarako.mozilla-b2g28_v1_3t.perf/), and those seem to be running "fine" with Marionette enabled, which would/should mean they're engineering builds, no?

Comment 5

[Jonathan Griffin (:jgriffin)]

Marionette is also enabled on userdebug builds, so that's expected.

Comment 6

[Stephen Donner [:stephend] Not actively reading bugmail]

Doesn't this adversly affect our performance metrics?  I get that we wouldn't want to switch (to engineering without debug) without taking into effect the change in metrics, but seems like measuring userdebug perf is sub-optimal, no?

Comment 7

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

After speaking with Aki, found out that this might be the cause of bug 991304. 

We need a user build for smoke testing purposes; without the user build there's no ftu and it fails one of our smoke tests.  Could we get the regular build to work as a user build please?

Comment 8

[Jonathan Griffin (:jgriffin)]

So it sounds like we should remove the LUNCH from config.sh for Tarakos.  Dave, Vincent, would this cause any problems?  I don't have a Tarako locally so can't test.

Comment 9

[Dave Hylands [:dhylands]]

I don't have one either.

I think that LUNCH should be removed and that the VARIANT should be used to select the variant, just like all of the other phones.

Comment 10

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

:jgriffin, :dhylands, would you like me to test this out?  I have a device and a repo I can use in order to test.

Please needinfo me if you do.

Comment 11

[Jonathan Griffin (:jgriffin)]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #10)
> :jgriffin, :dhylands, would you like me to test this out?  I have a device
> and a repo I can use in order to test.
> 
> Please needinfo me if you do.

Yes, that would be very helpful!  I've been trying to find someone with a Tarako not hooked up to automation to test it.

Comment 12

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Removing the LUNCH line and trying to compile VARIANT=user MOZILLA_OFFICIAL=1 ./build.sh -j1 will output : 

including device/sprd/sp6821a_gonk/vendorsetup.sh
build/core/product_config.mk:205: *** No matches for product "full_sp6821a_gonk".  Stop.

** Don't have a product spec for: 'full_sp6821a_gonk'
** Do you have the right repo manifest?

Comment 13

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Doesn't the LUNCH part compile the gonk layer?

For Hamachi, we use the base build, having said that there are other builds we compile the gonk layer from the repo... I thought?  Maybe I'm wrong...

Comment 14

[Dave Hylands [:dhylands]]

You should confirm that LUNCH from your .config file as well.

LUNCH is normally set in the load-config.sh script to be:

full_${DEVICE}-${VARIANT}

The script was setting DEVICE=sp6821a_gonk and LUNCH=sp6821a_gonk-userdebug

So load-config.sh for every other device expects that the LUNCH of full_sp6821a_gonk-user should work fine, but they seem to be setup to use sp6821a_gonk-user rather than full_sp6821a_gonk-user

So I think that the tarako device tree needs to be updated to expect full_sp6821a_gonk rather than just sp6821a

Comment 15

[Kai-Zhen Li [:seinlin][:kli]]

I think it could be better to have in .config "PRODUCT_NAME=sp6821a_gonk".

And do not hard code LUNCH=sp6821a_gonk-userdebug.

LUNCH will be set as what Dave said in comment 14.

Comment 16

[Vincent Liu[:vliu]]

Attachment: config.patch

I think it would be better removing the LUNCH=sp6821a_gonk-userdebug from config.sh. After that, it is more flexible to define VARIANT through .userconfig.

For the issue happens in Comment 12, we can add |echo PRODUCT_NAME=sp6821a_gonk >> .tmp-config &&| to replace |echo LUNCH=sp6821a_gonk-userdebug >> .tmp-config &&| we set originally. PRODUCT_NAME in load-config.sh than set by PRODUCT_NAME passed from config.sh and not full_${DEVICE}

I will try to make a clean build by the attached patch and see if there is some other issues I missed.

@Dave, would you please give any comment for the patch? Thanks.

Comment 17

[Dave Hylands [:dhylands]]

Comment on attachment 8410873 [details] [diff] [review]
config.patch

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

That looks reasonable to me.

Comment 18

[Vincent Liu[:vliu]]

Attachment: pull request for bug 994985.html

Hi mwu,
Please have a review for the patch. The patch assign PRODUCT_NAME instead of LUNCH on tarako. After that, developer can switch user, user-debug or eng by assigning LUNCH in .userconfig.

Comment 19

[Dave Hylands [:dhylands]]

I'm hoping you meant hat that the user can switch user user-debug or eng by assigning VARIANT in .userconfig

Comment 20

[Kai-Zhen Li [:seinlin][:kli]]

Yes, I think it is VARIANT and LUNCH=PRODUCT_NAME-VARIANT.

Comment 21

[Vincent Liu[:vliu]]

(In reply to Dave Hylands [:dhylands] from comment #19)
> I'm hoping you meant hat that the user can switch user user-debug or eng by
> assigning VARIANT in .userconfig

Yes, It should be assigning VARIANT in .userconfig to switch different mode.

Comment 22

[Joe Cheng [:jcheng] (please needinfo)]

let's take it for tarako. 1.3T+

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://github.com/mozilla-b2g/B2G/commit/d6bcffaa5bb50d28359c46eba2a8139e4ccca42a