1228254 - [DataSync] Refine FIREFOX_SYNC flag sequence in Makefile

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Build, defect)

Description

[Sean Lee [:seanlee][:weilonge]]

There would be an issue if device.mk is loaded after FIREFOX ?= 0.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] weilonge:seanlee/DataSync/master/Bug1228254 > mozilla-b2g:master

Comment 2

[Sean Lee [:seanlee][:weilonge]]

Comment on attachment 8692397 [details] [review]
[gaia] weilonge:seanlee/DataSync/master/Bug1228254 > mozilla-b2g:master

Hi Alex,

I found that ?= statement will only affect to the first statement. The following statement won't override the same build flag, e.g. FIREFOX_SYNC. So I would like to fix the issue in this bug.

Could you help to review it?
Thank you!

Comment 3

[Alexandre Poirot [:ochameau]]

Comment on attachment 8692397 [details] [review]
[gaia] weilonge:seanlee/DataSync/master/Bug1228254 > mozilla-b2g:master

That's the expected behavior of ?=
The first time, in Makefile, there is not value so it sets it to 0,
then in device.mk, it isn't going to change the value if you also uses ?=
as the variable already has a value set.

I think you should use FIREFOX_SYNC=1 in device.mk instead of doing this.
At least, with existing code, there is a default value if device.mk forgets setting it.
But that's also ok to apply your change if you find it handy to still be able to override device.mk behavior with env variable.

Comment 4

[Sean Lee [:seanlee][:weilonge]]

(In reply to Alexandre Poirot [:ochameau] from comment #3)
> Comment on attachment 8692397 [details] [review]
> [gaia] weilonge:seanlee/DataSync/master/Bug1228254 > mozilla-b2g:master
> 
> That's the expected behavior of ?=
> The first time, in Makefile, there is not value so it sets it to 0,
> then in device.mk, it isn't going to change the value if you also uses ?=
> as the variable already has a value set.
> 
> I think you should use FIREFOX_SYNC=1 in device.mk instead of doing this.
> At least, with existing code, there is a default value if device.mk forgets
> setting it.
> But that's also ok to apply your change if you find it handy to still be
> able to override device.mk behavior with env variable.

Yes, I would like to see a user can override FIREFOX_SYNC flag in command line, e.g. FIREFOX_SYNC=1 make .
That's the reason that I don't use FIREFOX_SYNC=1 in device.mk. 

Thanks a lot for your quick response. :)

Comment 5

[Sean Lee [:seanlee][:weilonge]]

landed on master: https://github.com/mozilla-b2g/gaia/commit/6a8f8a094cf29dd3ebe6b9db73473525e30b5131
gaia-test: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=9a7f5fe9c94701e03e364da23191da14fec7ce7f

Comment 6

[Sean Lee [:seanlee][:weilonge]]

Comment on attachment 8692397 [details] [review]
[gaia] weilonge:seanlee/DataSync/master/Bug1228254 > mozilla-b2g:master

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 1194108
[User impact] if declined: In some case, the build flag won't work.
[Testing completed]: Tested on B2G Desktop
[Risk to taking this patch] (and alternatives if risky): none
[String changes made]: none

Comment 7

[Josh Cheng [:josh]]

Comment on attachment 8692397 [details] [review]
[gaia] weilonge:seanlee/DataSync/master/Bug1228254 > mozilla-b2g:master

Approved for TV 2.5

Comment 8

[Carsten Book [:Tomcat]]

https://github.com/mozilla-b2g/gaia/commit/9e75e4d1cf2732102b3c88f210f92a81dfb766a4