cli icon indicating copy to clipboard operation
cli copied to clipboard

Windows support

Open tsibley opened this issue 6 years ago • 11 comments

I would like the CLI to work well on Windows when used with Docker for Windows on Windows 10 Professional or Enterprise in a very vanilla setup. I think it is most of the way there, but there are some showstoppers and several rough edges and sharp corners. It would also be very nice, and serve more users users, to support Docker Toolbox on Windows 10 Home or older versions of Windows. To do any of this requires access to a Windows machine (not a VM, unfortunately) for development and testing, which has been a blocker so far.

This issue is going to serve as a punch list of things that need addressing. Some items may be moved to separate issues in the future.

  • [ ] Python 3 installs just as python, no python3.
  • [ ] netifaces wheel must be available for install. The current version is missing for Python 3.7, but this will be present in the next version (thanks to my PR).
  • [ ] Paths for docker run invocation must not be standard Windows paths, but munged versions (e.g. //c/Users/…). Affects volume mapping.
  • [ ] Unicode not well-supported it seems in cmd.exe or PowerShell
  • [ ] Terminal escapes may not be well-supported as-is
  • [x] os.execvp() doesn't work as expected
  • [ ] Use Travis CI's Windows support or AppVeyor for automated testing, prototyped in #14

tsibley avatar Oct 02 '18 22:10 tsibley

Relatedly, there's been some good news for Linux environments on Windows, including Docker support within WSL (alleviating a lot of problems, including making the Pro vs. Home edition issue moot): https://devblogs.microsoft.com/commandline/announcing-wsl-2/

Note that this issue is about native Windows support, not support for running within WSL. Ideally we support both. The latter should be much easier to achieve once WSL2 is released and widely available.

tsibley avatar Jun 06 '19 16:06 tsibley

We now have a Windows machine that we can use to test the CLI and Docker. I got Nextstrain working with Windows 10 Pro on this machine with only a few rough spots.

To summarize the installation steps that worked:

  1. Install Miniconda3 for Windows.
  2. Install the Nextstrain CLI with Conda in its own environment.
  3. Install Docker Desktop.
  4. Install WSL 2.
  5. Install Linux kernel update.
  6. Restart.
  7. Sign into Docker.
  8. Run nextstrain check-setup to confirm installation.
  9. Run nextstrain update to download the Docker image prior to running a build.
  10. Download a workflow with Git.
  11. Run the workflow with nextstrain build ..
  12. View the workflow output with nextstrain view auspice/

If we can figure out how to best install the CLI with the system-wide Python installation (via the Microsoft store) such that it runs as expected with a nextstrain command, we can skip the Conda steps at the beginning.

huddlej avatar Oct 26 '21 18:10 huddlej

netifaces wheel must be available for install. The current version is missing for Python 3.7, but this will be present in the next version (thanks to my PR).

@tsibley this is an issue again with Python >=3.9:

  • The last commit to netifaces was on 2021-05-31, before AppVeyor added support for Python 3.9.
  • The author of netifaces has stopped maintaining it and archived the repo, so PR to fix is no longer an option.

Is there an alternative to netifaces?

victorlin avatar Nov 11 '21 20:11 victorlin

Hmm. There wasn't a viable netifaces alternative back in 2018. I suppose there might be nowadays, but I wouldn't be surprised if not.

Some options I see (not exhaustive, obviously):

  1. Offer to maintain netifaces at a bare minimum level of occasionally updating CI to build on new Python versions. This seems like only marginal effort above submitting a PR to an active maintainer. There don't currently seem to be any forks doing this work already.
  2. Make netifaces optional in the CLI, or remove it entirely (but makes a small part of the UX worse).

tsibley avatar Nov 16 '21 20:11 tsibley

While reviewing @victorlin's PR for new Windows installation docs, I had some thoughts about this issue:

  1. Asking a Windows user to install the Microsoft C++ Build Tools before they can install the CLI makes the UX worse, too.
  2. Maintaining netifaces even at the lowest level of effort still seems like a distraction from primary Nextstrain development. We rarely prioritize our own CI updates (or manual Bioconda recipe bumps, etc.), for example.

My vote would be to drop netifaces the functionality and dependency from the CLI.

huddlej avatar Nov 22 '21 20:11 huddlej

Asking a Windows user to install the Microsoft C++ Build Tools before they can install the CLI makes the UX worse, too.

Agreed. I wasn't ever suggesting we do that!

Maintaining netifaces even at the lowest level of effort still seems like a distraction from primary Nextstrain development.

Agreed.

My vote would be to drop netifaces the functionality and dependency from the CLI.

This would be fine. Two questions:

  1. Do we drop --allow-remote-access from nextstrain view or still support it but be unable to give the correct address to users?
  2. Instead of dropping, would we rather scope the netifaces dep to platform + Python versions where we know a wheel exists (using environment markers)?

Relatedly, there's a prototype I never got back to finishing where I used mDNS/Zeroconf to advertise nextstrain view on the network, such that nextstrain.your-computer.local (for example) would work for people on the same network as you. This might be a nice way to preserve the good UX of --allow-remote-access, if we want, without needing netifaces.

tsibley avatar Nov 22 '21 22:11 tsibley

Do we drop --allow-remote-access from nextstrain view or still support it but be unable to give the correct address to users?

Yeah, that's a great question where having more user data would help us. :D Does anyone use (or depend on) this feature? Do the people who use it feel comfortable figuring out their own IP address?

Without that information, the original motivation for this feature still seems reasonable. If people use it and know how to get their IP (via whatismyip, etc.), it wouldn't cost us much to support the feature without the nicer UX. The mDNS/Zeroconf approach could eventually make this experience nicer again.

If we didn't have this feature, we would encourage users to upload their builds/narratives to GitHub or a Nextstrain Group to share with others. If Ashley had been interning with us this last summer, that's the path I would have recommended. This direction of sharing via nextstrain.org also seems to be a bigger part of our mission than supporting local services.

Given all of that, I'd be ok with dropping the feature completely and pushing for remote sharing. What do you think?

Instead of dropping, would we rather scope the netifaces dep to platform + Python versions where we know a wheel exists (using environment markers)?

This feels like delaying the inevitable need to drop or support netifaces; eventually there won't be wheels for all of the environments we use. If we need to both keep the --allow-remote-users feature and the ability to report their IP address (if this was a key use case for CLI users), I guess we'd rather support netifaces. 🤷🏻

huddlej avatar Nov 22 '21 23:11 huddlej

Without that information, the original motivation for this feature still seems reasonable. If people use it and know how to get their IP (via whatismyip, etc.), it wouldn't cost us much to support the feature without the nicer UX. The mDNS/Zeroconf approach could eventually make this experience nicer again.

Agreed. (But note that whatismyip, etc. won't/can't show you your LAN address(es), only your WAN address, so folks will have to know a bit more about how to find it.)

Given all of that, I'd be ok with dropping the feature completely and pushing for remote sharing. What do you think?

Agreed that remote sharing is probably better for many sharing situations, but I think we should still provide a way for someone who knows what they're doing to have nextstrain view listen on an interface other than the loopback. This could be keeping --allow-remote-access and the 0.0.0.0 behaviour or removing it and adding a new --listen option instead.

This feels like delaying the inevitable need to drop or support netifaces;

Yeah, that's fair! Let's not do it.

tsibley avatar Nov 24 '21 17:11 tsibley

keeping --allow-remote-access and the 0.0.0.0 behaviour

@tsibley Am I right in thinking this just requires deleting these lines, the best_remote_address function, and the netifaces imports?

removing it and adding a new --listen option instead.

This seems like a nicer long-term solution. Would this approach allow users to provide their preferred IP address to --listen?

Could we drop the netifaces dependency now and then implement the --listen interface in the future?

huddlej avatar Dec 03 '21 00:12 huddlej

@huddlej Yep, I believe that's all that'd be needed.

There's no technical reason why we can't drop netifaces now and implement --listen or similar later, but it also should be quite quick to implement --listen and this avoids a functionality gap.

We'll still be degrading the UX, but hopefully that's not an issue (and if we hear otherwise, we can reconsider).

tsibley avatar Dec 14 '21 01:12 tsibley

In #138 I've dropped netifaces and implemented --host instead of --listen since --port already existed. (--listen by convention takes a combination of host:port.)

tsibley avatar Dec 17 '21 20:12 tsibley