fix: use wslpath to resolve Windows paths

[G-Rath]

While the C drive is mounted in /mnt/c/ by default in WSL, it's possible to change it.
Luckly WSL provides wslpath for dealing with this exact situation: you toss it a Windows path, and it'll translate it to the Linux path, including resolving the mount point.

closes #110
closes #188

[patrickhulce]

thanks for the contribution @G-Rath! I'm unfortunately unable to test this myself.

any other contributors lurking who have a WSL setup handy that can give this a go for their project? :)

[patrickhulce]

tentatively LGTM!

great idea on the new names to clear things up and thanks for the new test suite @G-Rath 👍

I'd love at least one other person on WSL to sanity check it works for their setup still too before merging if that's possible, any takers lurking in chrome-launcher contributors? :D now is your time to shine!

[patrickhulce]

(btw have some type fixing to adjust for the sinon tests)

[G-Rath]

(btw have some type fixing to adjust for the sinon tests)

That's very odd - The errors are exactly what I'd expect to get, but didn't locally 🤔

[G-Rath]

I suspect my node_modules was bonked for some reason.

I've replaced the usage of deepStrictEqual with just strictEqual because I think it otherwise could look like there's something magic in there ("why do we have to deeply compare two strings...?") - originally my IDE was reporting strictEqual as deprecated (which is silly really because I recently rebuilt the types for assert in @types/node to support assertion guards, which included making sure all the @deprecated tags were up to date, so that should have been my first clue... 😅)

[patrickhulce]

Is anyone watching this repo able to give this a spin on their WSL? Would love to merge this :)

@molant any chance you would be willing to help us out? I believe you might be the only chrome-launcher author who's up on the WSL knowledge 😅

[molant]

Sure, I'll te to test I out tomorrow.
Can you assign it to me so I don't forget or do I have to be part of the org?

[molant]

Got a few errors when running the tests from WSL:

Errors

wslpath: some_path: No such file or directory

  !․․․․!!!․․․․․․․․․․!․․․․․․

  20 passing (41s)
  5 failing

  1) Launcher
       sets default launching flags:
     Error: Command failed: wslpath -w some_path
wslpath: some_path: No such file or directory

      at checkExecSyncError (child_process.js:611:11)
      at Object.execFileSync (child_process.js:629:15)
      at Object.invoke (node_modules/sinon/lib/sinon/behavior.js:176:34)
      at Object.functionStub (node_modules/sinon/lib/sinon/stub.js:39:43)
      at Function.invoke (node_modules/sinon/lib/sinon/proxy-invoke.js:47:47)
      at Object.execFileSync (node_modules/sinon/lib/sinon/proxy.js:230:26)
      at Object.toWin32Path (src/utils.ts:81:10)
      at Launcher.get flags [as flags] (src/chrome-launcher.ts:167:45)
      at Launcher.<anonymous> (src/chrome-launcher.ts:255:76)
      at Generator.next (<anonymous>)
      at fulfilled (src/chrome-launcher.ts:10:58)
      at processTicksAndRejections (internal/process/task_queues.js:97:5)

  2) Launcher
       doesn't fail when killed twice:
     Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/molant/projects/chrome-launcher/test/chrome-launcher-test.ts)
      at listOnTimeout (internal/timers.js:549:17)
      at processTimers (internal/timers.js:492:7)

  3) Launcher
       doesn't fail when killing all instances:
     Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/molant/projects/chrome-launcher/test/chrome-launcher-test.ts)
      at listOnTimeout (internal/timers.js:549:17)
      at processTimers (internal/timers.js:492:7)

  4) Launcher
       doesn't launch multiple chrome processes:
     Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/molant/projects/chrome-launcher/test/chrome-launcher-test.ts)
      at listOnTimeout (internal/timers.js:549:17)
      at processTimers (internal/timers.js:492:7)

  5) Launcher
       exposes expected interface when launched:
     Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/molant/projects/chrome-launcher/test/launch-signature-test.ts)
      at listOnTimeout (internal/timers.js:549:17)
      at processTimers (internal/timers.js:492:7)

Chrome opened a few times but didn't close.

[patrickhulce]

Thanks a million @molant! 🙏

@G-Rath looks like we might need to resolve the userDataDir relative to CWD or something before we pass it to wslpath? Or maybe just handle the non-existent directory case better?

[patrickhulce]

My hunch is that the closing failures are just related to the fact that launching didn't go as planned 🤞

[G-Rath]

Chrome opened a few times but didn't close.

This is because of microsoft/WSL#1614 which is not currently solvable: WSL launches exes by using a low level Linux API whose name I forget, that means the process running in the Linux side is "/init", and that's whos PID you get & kill when trying to close chrome.

Currently killing that process does not kill the actual exe program, nor is there a way to map the /init process to the Windows host process to be able to kill it with taskkill.

My hunch is that the closing failures are just related to the fact that launching didn't go as planned 🤞

That's what my bet is too, as while Chrome doesn't won't close due to the above, successfully killing the /init process should work.

---

@molant thanks for testing this. Could you tell me a bit more about your WSL setup, such as:

• Where the project files are located (i.e are they on the Windows host, or in Linux)
• The contents of /etc/wsl.conf
• Where you've got Chrome installed on the Windows host, and if you have Chrome installed on Linux

[molant]

• Running WSL2 with Ubuntu
• Project is in Linux and I'm running it from the terminal
• Don't have an /etc/wsl.conf (maybe because of WSL2?)
• Chrome is installed on Windows and Linux but the Windows version is the executed one. To make it work on Linux I need to run it in headless mode or otherwise I get [386:386:0522/142607.902708:ERROR:browser_main_loop.cc(1485)] Unable to open X display.
  • Linux: /usr/bin/google-chrome
  • Windows: C:\Program Files (x86)\Google\Chrome\Application\chrome.exe

[G-Rath]

Running WSL2 with Ubuntu

Ah this is only for WSLv1, since v2 is still in public preview and not yet released. I want to say it'll hopefully not be a problem, but thinking about the changes they've apparently made to the file system...

Is there any chance you could test this using WSLv1, to confirm if it is the VFS change causing trouble?

(I personally don't have a machine with WSLv2 on it)

Don't have an /etc/wsl.conf (maybe because of WSL2?)

wsl.conf is the file used to configure WSL in general - it doesn't exist unless you make it :)

This tells me that you're using the defaults settings, including that your C:\ is mounted in /mnt/c.

i.e my /etc/wsl.conf has

[automount]
root = /
options = "metadata"

[molant]

since v2 is still in public preview and not yet released

That's changing soon 😊

WSL 2 is only available in Windows 10, Version 2004, Build 19041 or higher. You will need to update your Windows version and join the Windows Insider program on the "Release Preview" ring until the public release in late May.

I'll see if I can create a distro using WSL without messing my current one and test it.

[G-Rath]

That's changing soon

For sure, but it's been changing soon for a few months now 😉

In general, WSLv2 support is its own thing due to the changes in implementation - we should totally support it, but I expect that changes will be required for a lot of libraries.

I'm happy to try and see if this can be made to work with WSLv2 with minimal extra work, but outside of that I'd prefer personally to land this under a "WSLv1" banner since I don't have access to WSLv2 myself.

(fwiw: I'm excited for v2 to land as well 🥳)

Can you confirm if the tests pass on master?

I'll see if I can create a distro using WSL without messing my current one and test it.

My understanding is that you should just be able to create a WSLv1 distro, as the two are being developed in parallel due to the pros & cons of v2 vs v1.

[molant]

Tests don't pass on master on WSL2 neither.

I've used Suse on WSL1 and 11 Launcher tests are failing with the following:

11) Launcher
       exposes expected interface when launched:
     Error: EACCES: permission denied, mkdir '/mnt/undefined'

Will try to convert my Ubuntu WSL2 to WSL1 and see what happens after the weekend :)

[G-Rath]

Thanks for the info - another thing to check is that you have wslpath available, and that process.env.PATH contains your AppData folder, so that it matches the RegExp here.

Will try to convert my Ubuntu WSL2 to WSL1

You should just be able to create a new WSL1 distro, that runs alongside existing ones.

I've not actually done this before, nor any of the converting, so I'd be interested in hearing how that goes for you as a side note :)

[molant]

I have Ubuntu with wsl2 and suse with wsl1. I haven't found a way to create another Ubuntu image from the one I have instaled already 😔
I want to try with Ubuntu in case there's something different between both distros. It's weird those tests are failing on my side and not yours.
What distro are you running?

[G-Rath]

Yeah I was getting that vib from the docs, but it seems you can export & import your distro with a new name, as a way to clone which is at least something.

I want to try with Ubuntu in case there's something different between both distros

Completely agree. Here's my distro info:

g-rath@DESKTOP-MUGH739:/c/Users/G-Rath$ wslsys
Release Install Date: 0x5e1ce429
Branch: 19h1_release
Build: 18363
Full Build: 18362.1.amd64fre.19h1_release.190318-1202
Uptime: 6d 16h 15m
Linux Release: Ubuntu 18.04.4 LTS
Linux Kernel: Linux 4.4.0-18362-Microsoft
Packages Count: 1094

I've only got the single WSLv1 distro running, which is what I use for all my deving.

Does the Suse v1 distro pass on master? If not, then I'd say the problem is a difference in the distros; otherwise if it does pass then it'll most likely be because of the changes to resolving where the C drive is mounted, since yours is mounted in the default /mnt/c which is supported by master.

[molant]

Does the Suse v1 distro pass on master?

This was with Master, didn't have the time to add your remote.

I'll try the import/export. Thanks!

[G-Rath]

Ok interesting - I'd say that something is different with the Suse distro then compared to Ubuntu.

I'm definitely going to push a commit up adding a section to the readme with these details, to document what "supports WSL" actually covers (i.e right now, WSLv1 running Ubuntu distro).

My guess is that it'll be a difference with your PATH variable; when you've got a spare minute, would you mind grabbing the output of this commend:

node -p '/\/([a-z])\/Users\/([^\/:]+)\/AppData\//.exec(process.env.PATH).slice(0)'

Also if you could run which wslpath to just confirm wslpath is installed by default in Suse that'd be cool too :)

[molant]

With Ubuntu 20.04 all tests except one are passing

1) Launcher
       sets default launching flags:
     Error: Command failed: wslpath -w some_path
wslpath: some_path: No such file or directory

Property	Ubuntu	Suse
wslpath	/usr/bin/wslpath	/bin/wslpath
AppData	[ '/c/Users/antonmo/AppData/', 'c', 'antonmo' ]	Cannot read property 'slice' of null

stale before molant's findings, will want to rereview

[G-Rath]

Interesting - I'm assuming for Suse the PATH variable doesn't have the Windows envs; not much that can be done about that since it's a requirement for this to work in the first place (but the error message could be improved).

That "No such file or directory" error is interesting tho. Note that WSLv1 doesn't yet support Ubuntu 20.04 due to a critical bug (but annoyingly fresh installs grab it since it's the latest version 🙄) - I don't think it's the problem here, but just something to be aware of.

Apparently Appveyor supports WSLv1, so I'm going to have a go setting it up for webdrivers (as I recently added WSL support), and if it works out I'll bring it over here asap, which'll hopefully give us a good baseline to check against.

[G-Rath]

@patrickhulce I'm finally getting back to this after being swamped with work.

I'd really like to get this landed as it's the last blocker at my company for having things "just work" out of the box for our devs using WSLv1.

While I'd love to have support for as many distros as possible on both versions of WSL, I think that'll require a lot more work especially since I've not got any additional distros or versions setup.

I'm wondering if you'd be open to landing this PR as is to ship support for Ubuntu 18 on WSLv1, and then further support could be landed in follow-up PRs as people want them?

I'm happy to help with the follow-up work, I'd just prefer not to have to get it perfect first time :)

[patrickhulce]

I'd be happy to merge some iterative support here @G-Rath. I'm just not super confident this doesn't have any regressions for the default setup based on molant's comment about fatal failure for wslpath. If we can throw a try/catch on the execFileSync for path conversion to fallback to the previous behavior then I'm fine with this as-is 👍

[G-Rath]

@patrickhulce I totally agree! I've adjusted the implementation to fallback to the original values and methods if wslpath errors for any reason, and included the original tests to confirm the fallbacks are unchanged :)

[connorjclark]

Thanks for sticking with this! I will review this tomorrow Monday.

[connorjclark]

Having no knowledge of WSL/or a machine to test this, I'm still pretty confident to approve this because of the fallbacks provided, so thanks for adding those. Anyway, from reading everything it seems like it should work great. Thanks for the PR!