[DataSync] Refine FIREFOX_SYNC flag sequence in Makefile

RESOLVED FIXED

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: selee, Unassigned)

Tracking

unspecified
ARM
Gonk (Firefox OS)

Firefox Tracking Flags

(feature-b2g:2.5+, b2g-v2.5 fixed, b2g-master fixed)

Details

(Whiteboard: [ft:conndevices][partner-cherry-pick])

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
There would be an issue if device.mk is loaded after FIREFOX ?= 0.
Created attachment 8692397 [details] [review]
[gaia] weilonge:seanlee/DataSync/master/Bug1228254 > mozilla-b2g:master
(Reporter)

Comment 2

3 years ago
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!
Attachment #8692397 - Flags: review?(poirot.alex)
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.
Attachment #8692397 - Flags: review?(poirot.alex) → review+
(Reporter)

Comment 4

3 years ago
(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. :)
(Reporter)

Comment 5

3 years ago
landed on master: https://github.com/mozilla-b2g/gaia/commit/6a8f8a094cf29dd3ebe6b9db73473525e30b5131
gaia-test: https://treeherder.mozilla.org/#/jobs?repo=gaia&revision=9a7f5fe9c94701e03e364da23191da14fec7ce7f
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Whiteboard: [ft:conndevices][partner-cherry-pick]
(Reporter)

Comment 6

3 years ago
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
Attachment #8692397 - Flags: approval-gaia-v2.5?

Comment 7

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

Approved for TV 2.5
Attachment #8692397 - Flags: approval-gaia-v2.5? → approval-gaia-v2.5+

Updated

3 years ago
status-b2g-v2.5: --- → affected
status-b2g-master: --- → fixed

Updated

3 years ago
feature-b2g: --- → 2.5+
You need to log in before you can comment on or make changes to this bug.