832165 - [Build] GAIA_PATH not working when you set it in .userconfig file as env of |./build.sh|

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: GonkIntegration, defect)

Description

[Walter Chen[:ypwalter][:wachen]]

When you tried to set GAIA_PATH, it's actually not working. It still use the old GAIA path to build.

Comment 1

[Walter Chen[:ypwalter][:wachen]]

For example, Android.mk in gaia folder would reset GAIA_PATH.
Android.mk in gonk-misc would forced to use {B2G}/gaia
And, also, profile folder & its tar.gz file would produced in {B2G}/gaia instead of the GAIA_PATH assigned.

Comment 2

[Walter Chen[:ypwalter][:wachen]]

need help from others to solve this bug

Comment 3

[Andrew Overholt [:overholt]]

This should be fixed so we're not hurting developers but we can't block on it.

Comment 4

[Vincent Liu[:vliu]]

If we added GAIA_PATH in .userconfig, Android.mk in B2G/gaig/ should get this definition. Because we have defined GAIA_PATH, we shouldn't overwrite it. Simply check GAIA_PATH in Android.mk should be the way to go.

ifndef GAIA_PATH
  GAIA_PATH := $(abspath $(LOCAL_PATH))
endif

If we defined GAIA_PATH in .userconfig, we should ignore GAIA_PATH := $(abspath $(LOCAL_PATH)). I will then try it in our build system.

Comment 5

[Walter Chen[:ypwalter][:wachen]]

Please also notice that there are more than one place of this.
Another example is Android.mk in gonk-misc would forced to use {B2G}/gaia instead of GAIA_PATH assigned. 
But, please try with the solution you provide. Thanks for the helping.

Comment 6

[Alan Huang]

Attachment: proposed fix, v1

Hi Walter,
Per talked, please use this patch for testing, thanks.

Comment 7

[Walter Chen[:ypwalter][:wachen]]

Comment on attachment 707493 [details] [diff] [review]
proposed fix, v1

I think its cool.

Comment 8

[Alan Huang]

Comment on attachment 707493 [details] [diff] [review]
proposed fix, v1

Hi Dave, 
Can you help to review this patch? QA tested it and works. If it is also fine to you, I'll apply pull request for below 2 commits. Thank you!

https://github.com/AlanHuang/gonk-misc/commit/cdc0b4456aef26c5e0e26c1f0766db25671d8947

https://github.com/AlanHuang/gaia/commit/265537e5c0b509f98fb88099897eefab42afb2fd

Comment 9

[Dave Hylands [:dhylands]]

Comment on attachment 707493 [details] [diff] [review]
proposed fix, v1

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

r=me with a slightly better comment about the GAIA_PATH := $(GAIA_PATH) line.

::: Android.mk
@@ +6,5 @@
>  
>  include $(CLEAR_VARS)
> +GAIA_PATH ?= $(abspath $(LOCAL_PATH))
> +# This is for gonk-misc/Android.mk, otherwise LOCAL_PATH would be different.
> +GAIA_PATH := $(GAIA_PATH)

tricky.

I'd probably say that you need to assign using := because there is no ?:= and LOCAL_PATH can take on multiple values.

Comment 10

[Alan Huang]

(In reply to Dave Hylands [:dhylands] from comment #9)
> Comment on attachment 707493 [details] [diff] [review]
> proposed fix, v1
> 
> Review of attachment 707493 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with a slightly better comment about the GAIA_PATH := $(GAIA_PATH) line.

Hi Dave,
I've revise comment with better description. I did below 2 pull request, thanks!

https://github.com/mozilla-b2g/gonk-misc/pull/71
https://github.com/mozilla-b2g/gaia/pull/7921

Comment 11

[Michael Vines [:m1] [:evilmachines]]

Looks like the gonk-misc patch has landed but not the gaia patch.  Bustage on v1-train.

Comment 12

[Dave Hylands [:dhylands]]

jhford is backing out the gonk-misc/master change.

It turns out gonk-misc didn't have a v1-train branch. Once gonk-misc has the v1-train branch, then we can land both changes on gaia/master and gonk-misc/master at the same time.

Comment 13

[John Ford [:jhford] CET/CEST Berlin Time]

Backed out

commit f46dac73e6256c25763764d5fa26ac1bdf1417ba
Author: John Ford <[e-mail]>
Date:   Fri Feb 1 14:10:27 2013 -0800

    backout bug 832165 because it caused bug 837175 and broke everyone

Comment 14

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

It looks like QA had tested as per above, however this does cause bustage.
Is there further QA involvement with the patch?  It's been backed out.

Removing QA Wanted.

Comment 15

[Walter Chen[:ypwalter][:wachen]]

I believe this is the right thing to do with this patch. Is there anyone who can take further investigation or help on this?

Comment 16

[John Ford [:jhford] CET/CEST Berlin Time]

The approach used here uses the Android.mk file from the repo-managed copy of Gaia with the other files from the GAIA_PATH copy of gaia.  This means that a change in the GAIA_PATH Android.mk file is not used, which could be confusing.

Aiui, the main reason for having GECKO_PATH is because Gecko's canonical source is stored in HG, which doesn't integrate into repo, nor the repositories it manages.

I think between adding git remotes, git branches, git rebase and git stash, all the use cases needed for GAIA_PATH should be taken care of.  Further, this is a fairly complicated bug and it looks like no one is able to it right now.  Let's file a new bug if this becomes an issue again.

Comment 17

[:gerard-majax]

This is going to be a pity to work on transition, let's fix this.

Comment 18

[:gerard-majax]

Comment on attachment 707493 [details] [diff] [review]
proposed fix, v1

Looks like we realy only need:
|GAIA_PATH ?= $(abspath $(LOCAL_PATH))|
and the PHONY changes. 

You need to make sure your gaia checkout in B2G build tree contains the ":=" to "?=" change. If not, then GAIA_PATH will keep getting overriden.

Comment 19

[:gerard-majax]

Ok, we indeed need GAIA_PATH := $(GAIA_PATH). Looks like there is already fixes landed on gonk-misc/ side.

Comment 20

[GitHub Autolander Bot]

Attachment: [gaia] lissyx:bug832165 > mozilla-b2g:master

Comment 21

[:gerard-majax]

This looks as much as green as previous merges on master: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=09b80df32f277e5862055e7891f3a9b4eccd2cf2

Comment 22

[:gerard-majax]

https://github.com/mozilla-b2g/gaia/commit/06cd6c8935a419f9e526894d843e9d962949db27

Comment 23

[Sebastian Kümpel]

(In reply to Alexandre LISSY :gerard-majax from comment #22)
> https://github.com/mozilla-b2g/gaia/commit/
> 06cd6c8935a419f9e526894d843e9d962949db27

Hey Guys, I tried to compile B2G for the Fairphone 2 but it failed with the message:
make: *** Keine Regel vorhanden, um das Target »gaia/profile.tar.gz«, 
  benötigt von »out/target/product/FP2/obj/DATA/gaia_intermediates/gaia«, zu erstellen.  Schluss.
For me it seems to be caused by this commit.

Comment 24

[:gerard-majax]

(In reply to Sebastian Kümpel from comment #23)
> (In reply to Alexandre LISSY :gerard-majax from comment #22)
> > https://github.com/mozilla-b2g/gaia/commit/
> > 06cd6c8935a419f9e526894d843e9d962949db27
> 
> Hey Guys, I tried to compile B2G for the Fairphone 2 but it failed with the
> message:
> make: *** Keine Regel vorhanden, um das Target »gaia/profile.tar.gz«, 
>   benötigt von »out/target/product/FP2/obj/DATA/gaia_intermediates/gaia«, zu
> erstellen.  Schluss.
> For me it seems to be caused by this commit.

Except I tested locally and on try and it was working: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=09b80df32f277e5862055e7891f3a9b4eccd2cf2

Without more logging we cannot conclude anything.

Comment 25

[:gerard-majax]

Ok, I cannot tell why but I now reproduce the issue too.

Comment 26

[:gerard-majax]

I think I have a fix ...

Comment 27

[:gerard-majax]

So, the definition of Gaia module per Android build framework, done in gaia/Android.mk (LOCAL_MODULE := gaia) includes a dependency against the profile being generated, as profile.tar.gz. The resulting dependency at Makefile level is |$(LOCAL_MODULE)/$(LOCAL_SRC_FILES)|. Hence, this is resulting in |gaia/profile.tar.gz|.

That latter dependency is supposedly picked up by the phony target defined by:
> .PHONY: $(LOCAL_PATH)/profile.tar.gz
> $(LOCAL_PATH)/profile.tar.gz: [...]

Which I changed to make use of GAIA_PATH. This is why this fails. That should not be changed in fact.

Comment 28

[:gerard-majax]

While I can just change it back to LOCAL_PATH, it turns out the real only viable solution is clearly not to it this way: gaia/Android.mk definitions are still valid and this is confusing more than helping.

Comment 29

[:gerard-majax]

Attachment: Followup PR

Comment 30

[:gerard-majax]

As John stated in comment 16, we cannot really have any hope of getting this to work easily. I am going to backout this from master and kanikani.

Comment 31

[GitHub Autolander Bot]

Attachment: [gaia] lissyx:bug832165_followup_kanikani > mozilla-b2g:kanikani

Comment 32

[:gerard-majax]

Reverted in:
https://github.com/mozilla-b2g/gaia/commit/7143f186e7d1e5c3e5ddf125510c6ec4e1361b44
https://github.com/mozilla-b2g/gaia/commit/c4a51139beee99b3e117ae8db7c17f734f94eca1