-
Notifications
You must be signed in to change notification settings - Fork 29.8k
fix: bind fuchsia package server to all interfaces #84759
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
@jonahwilliams this is a continuation of #55664 which attempted to fix a similar issue. I am uncertain if this change can be tested since it is just changing function parameters. Just for curiousity, is there another fork of the flutter repo that is maintained for Fuchsia? I am wondering how do engineers developing for Fuchsia do |
These codepaths are not currently used for actual fuchsia development, its more aspirational. |
|
(and I don't think it would be expected to work via qemu, which we don't have access to through the SDK yet) |
Can you clarify more on this one? This |
Hmm can you point me to what are the Fuchsia team actually using currently? It would be beneficial to Fuchsia if a non-Google developer can also try to develop apps on it. |
|
That is a workflow for checking out the Fuchsia source tree, that is different than depending on a Fuchsia SDK. The flutter tool is not intended for development with the Fuchsia source tree, you need to use fx |
Do you mean this one https://fuchsia.dev/fuchsia-src/development/languages/dart/mods? It looks quite different from normal flutter development flow through. I tried to find guides for debugging flutter apps in Fuchsia but it looks like there are not much mention for Dart Observatory? I only got a zxdb which is for native code. I got the qemu working by using the gn frontend sdk in https://fuchsia.googlesource.com/samples. If you want to see my notes you can have a look at https://github.com/michaellee8/flutter_fuchsia_toolchain/blob/master/README.md which I somehow got flutter sdk and fuchsia sdk working together. It is much more convenient to develop Flutter apps for Fuchsia out of tree with the SDK without checking out the sources, which is like 150 gb with source only and more than 200 gb binaries. So for the sake of convenience of myself and those who are interested in playing with Flutter apps with Fuchsia, if Google is not using these code path, I guess I can help maintaining those Fuchsia code in flutter_tools for usage with the Fuchsia SDK. |
|
@michaellee8 Instead of jumping right into a PR, I think for this we should start out with an issue in the issue tracker. That way we can discuss the problem and potential options with Fuchsia experts before discussing any specific fix. |
|
I've filed that issue with the Fuchsia team about a month ago, they also provided some solutions/places to look when fixing it - https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=77566 |
|
@zanderso Sorry for skipping the bug report process. This is juat a continuation of some previous PR with very small changes, and I thought there are no one else is hitting this bug so I tried not to add one to that 5k+ issue count. It looks like that there are others' hitting this bug after all, so I guess a proper issue will has to be opened to track the progress of Fuchsia support. Maybe let's use @nmcain 's #83609 as a starting point. |
|
It looks like it is fixed in 6dd9e8c, hence closing. |
Currently, FuchsiaPM.serve() called in FuchsiaDevice.startApp() uses
host, which is the ipaddress of the host device in the emulator's point of view that ends in sth like
ensx7. It is incorrect and will causeerrors when binding that address in
pm serve, and preventingflutter runon fuchsia devices tostart properly. I have considered to use the
idas the bind address forpm serveas well,but have found that it is the ip address of emualator in the host's point of view that
ends in sth like
qemu. The correct address to be used here should be the interface address ofthe
qemunetwork interface, which can be obtained byifconfigorip -o a. I have consideredsupplying the correct IP address here but it would involve some considerable amount of work, so
I changed it to listening on all ipv6 interfaces (or ipv4 if the fuchsia network is on ipv4), which
is certainly going to work and more convenient.
I can offer an implementation to retrieve the ip address for binding via
ip -o ahere, if the Flutter teamdoes not like exposing ports to all interfaces, but I am uncertain if
ip -o awould work on non-Linux enviromentlike macOS.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.