893724 - [Gaia Build] GAIA_MAKE_FLAG is different between "./flash.sh gaia" and "./build.sh gaia".

Status: RESOLVED FIXED

(Firefox OS Graveyard :: GonkIntegration, defect, P1)

Description

[Marco Chen [:mchen]]

"./flash.sh gaia":
  GAIA_MAKE_FLAGS only hardcoded to take care ADB and PRODUCTION flags.
"./build.sh gaia"
  Android.mk can refer to flags from Android build environment then apply into GAIA_MAKE_FLAGS. (ex: GAIA_DEV_PIXELS_PER_PX, B2G_SYSTEM_APPS)

So if we use "./flash.sh gaia" to update gaia then we will miss some make flags from Android.mk.

Comment 1

[Marco Chen [:mchen]]

Attachment: Part 1 v1: Gaia - Android.mk

This patch is used to store the make flags produced in Android.mk to a .makeflags file. Then that file will be used by "flash.sh gaia".

Comment 2

[Marco Chen [:mchen]]

Attachment: Part 2 v1: for flash.sh gaia

Add the script for "flash.sh gaia" to read .makeflag then apply to make flags.

Comment 3

[Marco Chen [:mchen]]

If developers used "./flash.sh gaia" to update gaia then correct resources will not be installed without this patch.

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

This should be a QA blocker per bug 894013, unfortunately.

Comment 5

[Marco Chen [:mchen]]

Hi Michael,

Could you help to review this patch or suggest any reviewer to do this?
Thanks.

Comment 6

[Michael Wu [:mwu]]

Can the gaia makefile automatically include .makeflag ?

Comment 7

[Marco Chen [:mchen]]

(In reply to Michael Wu [:mwu] from comment #6)
> Can the gaia makefile automatically include .makeflag ?

I think it is feasible and can avoid to modify flash.sh. Thanks for the suggestion.
Will update the patch tomorrow.

By the way, may I know this is a right direction to solve this issue?

Comment 8

[Michael Wu [:mwu]]

It's more correct for sure. This would ensure that "./flash.sh gaia" and "cd gaia && make install-gaia" do the same thing. Things might still be weird for people who only work on gaia without a full build environment to generate .makeflag. I wonder if we can store these variables on the device so gaia can automatically pick up the right variables in that case. However, that's a separate issue we can address later if it's actually a problem.

Comment 9

[Marco Chen [:mchen]]

Attachment: Part 2 v1: Gaia - Makefile

To include .makeflags created from Android build system (./build.sh gaia) in Makefile directly.

Comment 10

[Marco Chen [:mchen]]

Attachment: Part 1 v2: Gaia - Android.mk

Add the code for
  1. if there is no variable we concerned been defined then there is no need to create .makeflags.

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Marco Chen [:mchen] from comment #10)
> Created attachment 778333 [details] [diff] [review]
> Part 1 v2: Gaia - Android.mk
> 
> Add the code for
>   1. if there is no variable we concerned been defined then there is no need
> to create .makeflags.

1. You should add .makeflags into .gitignore
2. You should remove .makeflags if we no longer need it.

Thanks.

Comment 12

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: Patch v2.5 part 1 + part 2

I was going to steal this bug, but the more I work on this the more I suspect this is indeed the right direction -- should be include _all_ GAIA_MAKE_FLAGS into .b2g.mk, or should we just drop the GAIA_DEV_PIXELS_PER_PX into it? This patch does the latter and I need feedback to know we should do the former or not.

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 778407 [details]
Patch v2.5 part 1 + part 2

>diff --git a/Android.mk b/Android.mk
>index 940b246..374967c 100644
>--- a/Android.mk
>+++ b/Android.mk
>@@ -27,8 +27,12 @@ endif
> 
> # Gaia currently needs to specify the default scale value manually or pictures
> # with correct resolution will not be applied.
>+# We will keep this flag in .b2g.mk so |./flash.sh gaia| follows
>+# will correctly pick up the flag.
> ifneq (,$(GAIA_DEV_PIXELS_PER_PX))
>-GAIA_MAKE_FLAGS += GAIA_DEV_PIXELS_PER_PX=$(GAIA_DEV_PIXELS_PER_PX)
>+       echo "GAIA_DEV_PIXELS_PER_PX=$(GAIA_DEV_PIXELS_PER_PX)" > $(GAIA_PATH)/.

This line should end with |> $(GAIA_PATH)/.b2g.mk|.
Sorry about the copy-paste failure.

Comment 14

[Michael Wu [:mwu]]

Saving all of GAIA_MAKE_FLAGS sounds like a good idea. That also ensures we install into the right partition.

Comment 15

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: Patch v2.5 (including all flags)

Per discussion on IRC w/ :mwu, including all flags from Android.mk to .b2g.mk

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I've created bug 896145 to make the build script saner.

Comment 17

[Alexandre Poirot [:ochameau]]

Comment on attachment 778780 [details] [diff] [review]
Patch v2.5 (including all flags)

Looks good.

Comment 18

[Dominic Kuo [:dkuo]]

Landed on master: 56c6505d680c79a21a5743b447ef64d32db31206

Comment 19

[Dominic Kuo [:dkuo]]

v1.1.0hd: 495cccd731f268ae37532db88c6a8b5d7a0fa036

Comment 20

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Al, could you check with releng to make sure QA are indeed having builds come with correct hi-res graphics as this bug has been fixed?

Comment 21

[Swaroopa Vantipalli [:swaroopa]]

After running release engineering version of flash.sh for the helix mozril build,  I get the below error 
"exec '/system/bin/sh' failed: No such file or directory (2) -" (Complete log at the end of the comment)
At this point the device reboots and hangs.

Does the patch mentioned in comment 17, fix the above mentioned issue?

Gecko  http://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/0e7c1dff9722
Gaia   03280104b9528b74fe2ce6b2987bb5530392fdaa
BuildID 20130722070208
Version 18.0

Log:

$ ./flash.sh 
< waiting for device >
sending 'userdata' (20888 KB)...
OKAY [  1.928s]
writing 'userdata'...
OKAY [  4.432s]
finished. total time: 6.360s
sending 'system' (136784 KB)...
OKAY [ 12.460s]
writing 'system'...
OKAY [ 32.598s]
finished. total time: 45.058s
rebooting...

finished. total time: 0.001s
Attempting to set the time on the device
- exec '/system/bin/sh' failed: No such file or directory (2) -
- exec '/system/bin/sh' failed: No such file or directory (2) -

Comment 22

[Swaroopa Vantipalli [:swaroopa]]

Attachment: flash.sh used for issue in Comment 21

Comment 23

[Al Tsai [:atsai]]

I think leo device has the same problem in Comment 21 as well.
That should not be a blocker.

AFAIK, QA will use Helix base provided by partners and flash our gaia/gecko on it only. Therefore, we should only run "./flash.sh gaia" and "./flash.sh gecko" to replace gaia/gecko only.

I am checking with the device engineering team to confirm the issue.
Let me come back to you later with more information.

Comment 24

[Marco Chen [:mchen]]

(In reply to Swaroopa Vantipalli [:swaroopa] from comment #21)
> After running release engineering version of flash.sh for the helix mozril
> build,  I get the below error 
> "exec '/system/bin/sh' failed: No such file or directory (2) -" (Complete
> log at the end of the comment)
> At this point the device reboots and hangs.

Hi,

Please refer to the commit as below.
https://github.com/mozilla-b2g/device-helix/commit/ba93ac39168a51b5f3d2b8e6d3d82204a16b8869

The FxOS image from partner modified the partition table came from Android image so the current system.img from moz build is too large then actual partition size. Please update that commit and re-build your system.img. After flashing new system.img device should be workable. I can recover device from your status.

Comment 25

[Swaroopa Vantipalli [:swaroopa]]

Marco,
Can you provide detail steps to recover from this state?

Comment 26

[Marco Chen [:mchen]]

(In reply to Swaroopa Vantipalli [:swaroopa] from comment #25)
> Marco,
> Can you provide detail steps to recover from this state?

Since the issue you met is not related to this bug, I will send you a mail for this discussion not replied there. Thanks.

Comment 27

[Jason Smith [:jsmith]]

Mass-modify - removal of no longer relevant blocking flags.