Closed Bug 994985 Opened 5 years ago Closed 5 years ago

All Tarako builds are userdebug, not user or eng

Categories

(Release Engineering :: General, defect)

defect
Not set

Tracking

(blocking-b2g:1.3T+, b2g-v1.3T fixed)

RESOLVED FIXED
blocking-b2g 1.3T+
Tracking Status
b2g-v1.3T --- fixed

People

(Reporter: jgriffin, Assigned: vliu)

References

Details

(Whiteboard: [tarako_only])

Attachments

(1 file, 1 obsolete file)

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.
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.
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?
Flags: needinfo?(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.
Flags: needinfo?(vliu)
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?
Marionette is also enabled on userdebug builds, so that's expected.
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?
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?
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.
Flags: needinfo?(vliu)
Flags: needinfo?(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.
Flags: needinfo?(dhylands)
: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.
Flags: needinfo?(jgriffin)
Flags: needinfo?(dhylands)
(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.
Flags: needinfo?(jgriffin) → needinfo?(nhirata.bugzilla)
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?
Flags: needinfo?(nhirata.bugzilla) → needinfo?(jgriffin)
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...
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
Flags: needinfo?(dhylands)
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.
Attached patch config.patch (obsolete) — Splinter Review
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.
Attachment #8410873 - Flags: feedback?(dhylands)
Flags: needinfo?(vliu)
Comment on attachment 8410873 [details] [diff] [review]
config.patch

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

That looks reasonable to me.
Attachment #8410873 - Flags: feedback?(dhylands) → feedback+
Flags: needinfo?(jgriffin)
Assignee: nobody → vliu
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.
Attachment #8410873 - Attachment is obsolete: true
Attachment #8411571 - Flags: review?(mwu)
I'm hoping you meant hat that the user can switch user user-debug or eng by assigning VARIANT in .userconfig
Yes, I think it is VARIANT and LUNCH=PRODUCT_NAME-VARIANT.
(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.
blocking-b2g: --- → 1.3T?
Attachment #8411571 - Flags: review?(mwu) → review+
Keywords: checkin-needed
let's take it for tarako. 1.3T+
blocking-b2g: 1.3T? → 1.3T+
Whiteboard: [tarako_only]
https://github.com/mozilla-b2g/B2G/commit/d6bcffaa5bb50d28359c46eba2a8139e4ccca42a
Status: NEW → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.