Skip to content

Conversation

@michaellee8
Copy link
Contributor

Currently, FuchsiaPM.serve() called in FuchsiaDevice.startApp() uses host, which is the ip
address of the host device in the emulator's point of view that ends in sth like ensx7. It is incorrect and will cause
errors when binding that address in pm serve, and preventing flutter run on fuchsia devices to
start properly. I have considered to use the id as the bind address for pm serve as 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 of
the qemu network interface, which can be obtained by ifconfig or ip -o a. I have considered
supplying 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 a here, if the Flutter team
does not like exposing ports to all interfaces, but I am uncertain if ip -o a would work on non-Linux enviroment
like macOS.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above. (N/A)
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 17, 2021
@flutter-dashboard
Copy link

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.

@google-cla google-cla bot added the cla: yes label Jun 17, 2021
@michaellee8
Copy link
Contributor Author

@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 flutter run, it seems that this bug should be affecting both in tree (build fuchsia from source) and out-of-tree (using the fuchsia idk) scenarios.

@jonahwilliams
Copy link
Contributor

Analyzing 3 directories...                                      

   info • The value of the local variable 'host' isn't used • packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart:312:18 • unused_local_variable

These codepaths are not currently used for actual fuchsia development, its more aspirational.

@jonahwilliams
Copy link
Contributor

(and I don't think it would be expected to work via qemu, which we don't have access to through the SDK yet)

@michaellee8
Copy link
Contributor Author

(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 qemu interface is setup according to https://fuchsia.dev/fuchsia-src/get-started/set_up_femu#configure-ipv6-network which is to enable networking in Fuchsia emulator. I suppose it should be the standard workflow for the Fuchsia team?

@michaellee8
Copy link
Contributor Author

Analyzing 3 directories...                                      

   info • The value of the local variable 'host' isn't used • packages/flutter_tools/lib/src/fuchsia/fuchsia_device.dart:312:18 • unused_local_variable

These codepaths are not currently used for actual fuchsia development, its more aspirational.

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.

@jonahwilliams
Copy link
Contributor

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

@michaellee8
Copy link
Contributor Author

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.

@zanderso
Copy link
Member

@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.

@nmcain
Copy link

nmcain commented Jun 23, 2021

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

@michaellee8
Copy link
Contributor Author

@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.

@michaellee8
Copy link
Contributor Author

It looks like it is fixed in 6dd9e8c, hence closing.

@michaellee8 michaellee8 closed this Jul 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants