Disable MOZ_B2G_RIL on Flatfish

RESOLVED FIXED

Status

Firefox OS
RIL
P1
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: vicamo, Assigned: viralwang)

Tracking

(Blocks: 1 bug)

unspecified
Other
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:koi+)

Details

(Whiteboard: [Flatfish only][developer+][POVB])

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
+++ This bug was initially created as a clone of Bug #920551 +++
(Reporter)

Comment 1

4 years ago
nominate for koi+ for it blocks bug 911684 and bug 921912, which are both koi+.
blocking-b2g: --- → koi?
(Reporter)

Comment 2

4 years ago
I think one can simply set `GECKO_CONFIGURE_ARGS` to '--disable-b2g-ril' in per device BoardConfig.mk (or any Android.mk) to disable RIL.  See https://github.com/mozilla-b2g/gonk-misc/blob/master/Android.mk#L273

Comment 3

4 years ago
according to triage result, change to Koi+
blocking-b2g: koi? → koi+

Comment 4

4 years ago
Askening, could you help add the '--disable-b2g-ril' ARG addressed in Comment 2 to TWCI flatfish build, then we could evaluate it quickly. 

thanks
Flags: needinfo?(fyen)

Comment 5

4 years ago
Askening, Sorry its build param in BoardConfig.mk, not for CI params
Flags: needinfo?(fyen)
(In reply to Fred Lin [:gasolin] from comment #4)
> Askening, could you help add the '--disable-b2g-ril' ARG addressed in
> Comment 2 to TWCI flatfish build, then we could evaluate it quickly. 
> 
> thanks

After added GECKO_CONFIGURE_ARGS="--disable-b2g-ril" into environment variable, the build failed.
(Reporter)

Comment 7

4 years ago
(In reply to Askeing Yen[:askeing] from comment #6)
> After added GECKO_CONFIGURE_ARGS="--disable-b2g-ril" into environment
> variable, the build failed.

Have you applied patches from bug 814556 and bug 915604?  Could you detail the error messages?
There is a issue Bug 929854 for creating the mater flatfish config profile.
After we have the config profile, then TWCI can build master flatfish.
depend on Bug 929854
Depends on: 929854
(Assignee)

Updated

4 years ago
No longer depends on: 929854

Comment 11

4 years ago
Thomas, could you help add environment variable described in Comment 2 into flatfish config profile?
Flags: needinfo?(ttsai)

Comment 12

4 years ago
It will lead to build-break that nsIWifi.idl won't be parsed, so there will no nsIWifi.h be found.
Flags: needinfo?(ttsai)

Updated

4 years ago
Flags: needinfo?(sku)

Comment 13

4 years ago
Vicamo, do we still miss some configurations after bug 920551 landed?
Flags: needinfo?(vyang)

Comment 14

4 years ago
It works in master, so v1.2 is waiting for the patch.

Comment 15

4 years ago
Due ot Thomas already replied, clear ni?sku.
Flags: needinfo?(sku)
(Reporter)

Comment 16

4 years ago
Created attachment 825185 [details] [diff] [review]
default-gecko-config-without-ril.global.example

This patch illustrates how to disable RIL on all b2g builds.  DO NOT COMMIT IT.
(Reporter)

Comment 17

4 years ago
Created attachment 825188 [details] [diff] [review]
Android.mk.device.example

This patch demos how to disable RIL in ordinary Android.mk.  DO NOT COMMIT IT.
(Reporter)

Comment 18

4 years ago
So, to realize what I had in comment 2, here are examples for disabling RIL.  No additional patch is required (in master branch for now.  I'm going to uplift bug 920551 to aurora).
Flags: needinfo?(vyang)
(Reporter)

Comment 19

4 years ago
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #18)
> I'm going to uplift bug 920551 to aurora

Done.

Comment 20

4 years ago
check with 11/1 flatfish master build, `window.navigator.mozMobileConnection` API still exist

Does there any settings we can do from gaia or gecko side to disable these API?
Flags: needinfo?(vyang)
I think that we don't want to ship different gecko builds to different devices. So removing RIL apis from flatfish should rather be done via a pref and the appropriate webidl functionality that let you hide apis based on a pref.
(Reporter)

Comment 22

4 years ago
(In reply to Fabrice Desré [:fabrice] from comment #21)
> I think that we don't want to ship different gecko builds to different
> devices. So removing RIL apis from flatfish should rather be done via a pref
> and the appropriate webidl functionality that let you hide apis based on a
> pref.

WebIDL is awesome.  WebIDL is the final solution.  But it just can't be NOW.

And please note Flatfish doesn't even build Gecko from Mozilla's hg/git tree.  There is another git tree hosted on bitbucket where not everyone has access.  That's an even more serious problem.
Flags: needinfo?(vyang)
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #22)
> (In reply to Fabrice Desré [:fabrice] from comment #21)
> > I think that we don't want to ship different gecko builds to different
> > devices. So removing RIL apis from flatfish should rather be done via a pref
> > and the appropriate webidl functionality that let you hide apis based on a
> > pref.
> 
> WebIDL is awesome.  WebIDL is the final solution.  But it just can't be NOW.

Why? we use it with many apis already as you know.

> And please note Flatfish doesn't even build Gecko from Mozilla's hg/git
> tree.  There is another git tree hosted on bitbucket where not everyone has
> access.  That's an even more serious problem.

Terrible indeed. I don't see how we can provide any support in these conditions.
(Reporter)

Comment 24

4 years ago
(In reply to Fabrice Desré [:fabrice] from comment #23)
> (In reply to Vicamo Yang [:vicamo][:vyang] from comment #22)
> > (In reply to Fabrice Desré [:fabrice] from comment #21)
> > WebIDL is awesome.  WebIDL is the final solution.  But it just can't be NOW.
> 
> Why? we use it with many apis already as you know.

Just not in CellBroadcast/Icc/MobileConnection/MobileMessage/Voicemail yet.  It's true some event targets had been converted, but not all.  Besides, I don't want to interfere DSDS work, either.

Comment 25

4 years ago
Checked b2g-manifest and I think it use standard gaia/gecko from https://git.mozilla.org/releases

https://bitbucket.org/thomastsai/a31-b2g-manifest/src/d8899d7620670bb7ebc7316640978fc4f9530b78/base-jb.xml?at=master

They did build Gecko/gaia from Mozilla's hg/git tree
(Reporter)

Comment 26

4 years ago
(In reply to Fred Lin [:gasolin] from comment #25)
> They did build Gecko/gaia from Mozilla's hg/git tree

Only after bug 929854.  And there is still https://bitbucket.org/thomastsai/a31-b2g-manifest/src/d8899d762067/flatfish.xml#cl-23 , a hidden patch pool to Gecko that require some membership to access.

Besides, the way Gaia determines whether a API set is supported is by checking the validity of |navigator.foo|.  You can do this on any platform with a customized Gecko, patched with 5 or 6 new lines that always returns null on attributes you're interested in.  You're not blocked by anything.  Never has been.

Comment 27

4 years ago
Hi Vicamo, this bug is yours...:)
Assignee: nobody → vyang

Comment 28

4 years ago
Hi vicamo, we do have some patches that are prepared and tested via your suggestion, but some reviewer ask us test on real tablet device.
That's why we're care of these platform/gecko issues :)

Comment 29

4 years ago
Viral, please help this bugs. Thanks.
Assignee: vyang → vwang

Comment 30

4 years ago
gecko and gaia of flatfish are all from mozilla hg/git.
Only few vendor specific folder like device, vendors and are from bitbucket.
Now gecko can be built from master or gaia.
Please refer to https://bugzilla.mozilla.org/show_bug.cgi?id=904377.

Comment 31

4 years ago
how to build v1.2 or master for flatfish:
https://bugzilla.mozilla.org/show_bug.cgi?id=904377#c16
(In reply to thomas tsai from comment #30)
> Please refer to https://bugzilla.mozilla.org/show_bug.cgi?id=904377.

Only Mozilla employees?
(Assignee)

Comment 33

4 years ago
Hi Vicamo,

we have plan to remove all gecko/gaia patches and merge it into mozilla hg/git in bug 934831.
I'm not sure we should provide a temporary gecko build for gaia testing only or consider merge it into our hg/git tree.

Any suggestion will be help, thanks.
(Assignee)

Updated

4 years ago
Flags: needinfo?(vyang)

Updated

4 years ago
Whiteboard: [Flatfish only] → [Flatfish only][developer+]
(Reporter)

Comment 34

4 years ago
(In reply to thomas tsai from comment #31)
> how to build v1.2 or master for flatfish:
> https://bugzilla.mozilla.org/show_bug.cgi?id=904377#c16

[ -n "`cat config.sh |grep flatfish`" ] || echo "not available"

And I got "not available".

It's really irrelevant for me actually.  I know it might be still in development and unavailable for the public.  The reason I had this bug open is I want to remind us that we need somewhere to send the pull request.  And as far as I know now, we're not ready yet.
Flags: needinfo?(vyang)
(Assignee)

Comment 35

4 years ago
This bug should be fixed now, we already add "--disable-b2g-ril" for flatfish.
I think we can have build in TWCI server tomorrow. (both master/v1.2 should be fine in daily build today)
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [Flatfish only][developer+] → [Flatfish only][developer+][POVB]
(In reply to viral [:viralwang] from comment #35)
> This bug should be fixed now, we already add "--disable-b2g-ril" for
> flatfish.
> I think we can have build in TWCI server tomorrow. (both master/v1.2 should
> be fine in daily build today)

I'll bring this up in the appropriate mailing lists, but I still thinks that having a custom gecko build is really not what we want. We have time to fix CellBroadcast/Icc/MobileConnection/MobileMessage/Voicemail before we ship.

Comment 37

4 years ago
compile error after add ac_add_options --disable-b2g-ril in  default-gecko-config

gecko/dom/bluetooth/bluedroid/BluetoothHfpManager.cpp:20:27: fatal error: nsIDOMIccInfo.h: No such file or directory

master branch
(Reporter)

Comment 38

4 years ago
(In reply to steve from comment #37)
> compile error after add ac_add_options --disable-b2g-ril in 
> default-gecko-config
> 
> gecko/dom/bluetooth/bluedroid/BluetoothHfpManager.cpp:20:27: fatal error:
> nsIDOMIccInfo.h: No such file or directory
> 
> master branch

Filed new bug 943758.
(Reporter)

Updated

4 years ago
Duplicate of this bug: 1034196
You need to log in before you can comment on or make changes to this bug.