Skip to content
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

help command makes assumptions about PATH in WSL environment #5756

Closed
samdmarshall opened this issue Mar 20, 2019 · 19 comments · Fixed by #5759
Closed

help command makes assumptions about PATH in WSL environment #5756

samdmarshall opened this issue Mar 20, 2019 · 19 comments · Fixed by #5759

Comments

@samdmarshall
Copy link
Contributor

I've encountered a problem with the code in ../share/fish/functions/help.fish that has variable behavior based on user's /etc/wsl.conf file. Here is what mine looks like:

[automount]
enabled = true
mountFsTab = true

[network]
generateHosts = true
generateResolvConf = true

[interop]
enabled = true
appendWindowsPath = false

And the important bit here is the last option in this file (appendWindowsPath = false), means that I don't have the %PATH% variable from the windows environment appended to my path in this flavor of Linux I am running (Ubuntu 16.04). I've documented this in another, similar, issue where code added that makes the assumption that this setting is always set to true (the default) and cannot find the windows executable that it needs to function.

This configuration value has been available as of build 17743, release notes for more info. This seems like a very simple fix of:

  • use the full path to cmd.exe (/mnt/c/Windows/System32/cmd.exe) to avoid having to figure out if the PATH variable has the right directories added to it

-OR-

  • don't add custom behavior for WSL, instead just invoke what the system thinks it should do with open/xdg-open/wsl-open/alternatives/etc.

  • Fish Version:
    • fish, version 3.0.2
  • System Info (uname --all):
    • Linux 4.4.0-17758-Microsoft #1-Microsoft Fri Sep 07 15:36:00 PST 2018 x86_64 GNU/Linux
  • Environment Variable TERM:
    • xterm-256color
@faho
Copy link
Member

faho commented Mar 20, 2019

don't add custom behavior for WSL, instead just invoke what the system thinks it should do with open/xdg-open/wsl-open/alternatives/etc.

As far as I'm concerned, the ideal way for us to deal with WSL is to pretend it doesn't exist. To a certain degree if that doesn't work, the system has failed.

So I'd very much like for us to be able to use some open tool, but that only really works if one is installed and working by default (that most likely rules out wsl-open). Otherwise we'd be better off using the full path.

use the full path to cmd.exe (/mnt/c/Windows/System32/cmd.exe)

Is that path guaranteed to be correct, though?

@floam
Copy link
Member

floam commented Mar 20, 2019

Is that path guaranteed to be correct, though?

I don't think so. Windows can be installed into a differently-named directory, or we may not even be on the C: disk. The right way to refer to the path would be $WINDIR\system32 or $SYSTEM32. Of course, Windows environment variables are not exposed.

@samdmarshall
Copy link
Contributor Author

samdmarshall commented Mar 20, 2019

@faho I think I agree with you in regards to how to handle the existence of "WSL" is to pretend it doesn't -- meaning that for the vast majority of things the user is going to do, being using a distribution via WSL or running it on the hardware directly doesn't make dramatic difference. My impression here, like the other issue I filed elsewhere, is that the software knowing it is operating within this "WSL" environment allows us to take certain short-cuts that in an out-of-the-box scenario, makes things more convenient/pleasant experience overall.


@floam additionally your comment made me also realize I skipped over something else wrt the wsl.conf file. The ability for the "Linux" vs "Windows" filesystem and binary execution inter-op is also optional. so, for the path I gave, the /mnt/* may not exist if the distribution's wsl.conf disables the auto-mount of the windows file-system and the overall interop functionality. So it isn't just a question of where is their install directory, but can the environment that fish is running in can even see it at all.

Also: I believe that the environment variables from windows shell environments (the equivalents), should be made available via the WSLENV environment variable. however, that environment variable always ends up defined but empty for me (unknown if that is a result of something I did or it doesn't get populated if the appended path option is disabled, I don't know). theoretically it should have whatever variables would normally be defined and available via using cmd.exe/powershell.exe.


EDIT: sorry I went off on a thing talking about clipboards, but that isn't the issue here, trying to rewrite it now.

This makes me think that the over-all best way is to just hand things directly over to browser or whatever program that whatever is running fish sees as it's url opening mechanism. in my case that is going to be the wsl-open program, which will interact with Microsoft edge, it could also be sent to firefox or chrome within the user's Linux distribution, rather than browsers they have installed in the windows desktop environment. Since I use a X server that runs on the windows side, and my Linux/WSL programs use that as a display, I get automatic interop with stuff link clip/paste-board, alt-tabbing app windows (and other window management), etc. so it makes sense for me to want to pass things over to windows vs keep them in the Linux side -- and that is all due to the rest of my desktop environment configuration on the Linux/WSL side of things to do that, and fish should respect that.

@faho
Copy link
Member

faho commented Mar 20, 2019

This makes me think that the over-all best way is to just hand things directly over to browser or whatever program that whatever is running fish sees as it's url opening mechanism

Fish already respects $BROWSER here on other systems, so this would simply require not automatically using just cmd.exe on wsl.

We also already have an open and xdg-open special-case, as we want to use them before any specific browser (since they allow the user to configure their preferred browser). We should presumably add wsl-open and maybe cmd.exe here.

One other question: If we can find e.g. firefox or xdg-open on wsl, does that work?

(Also cmd.exe needs to be handled a bit differently)

@faho
Copy link
Member

faho commented Mar 20, 2019

Oh, also: The way xdg-open is supposed to work is that that's the thing you're supposed to call, and it then calls the appropriate tool for the environment, e.g. gvfs-open on gnome or kde-open5 on kde.

So what wsl-open should be is a backend for xdg-open, so that we still only need to know about xdg-open. Does it currently work that way?

@samdmarshall
Copy link
Contributor Author

samdmarshall commented Mar 20, 2019

@faho yes, I believe that is the whole purpose of wsl-open, to act as the back-end opener for invocations of xdg-open. I just confirmed on my system by running command xdg-open "https://fishshell.com" and was immediately brought out of my terminal and into Microsoft edge loading a new tab with "https://fishshell.com". That said, I did have to make modifications to the wsl-open script on my end to account for the lack of windows-paths in my PATH variable so powershell.exe and cmd.exe could each be found correctly. However, I don't think is super important as the take-away here is that it means fish should be able to drop this url-sending maneuver by letting something better at it do the job.

EDIT:
Also, I should note that in order for xdg-open to send things over to the windows side, it needs to have wsl-open or similarly functioning tool, after disabling wsl-open on my machine (chmod -x (which wsl-open)), and re-ran xdg-open "https://fishshell.com" this was the output I got:

/usr/bin/xdg-open: 709: /usr/bin/xdg-open: /home/linuxbrew/.linuxbrew/bin/wsl-open: Permission denied
/usr/bin/xdg-open: 778: /usr/bin/xdg-open: x-www-browser: not found
/usr/bin/xdg-open: 778: /usr/bin/xdg-open: firefox: not found
/usr/bin/xdg-open: 778: /usr/bin/xdg-open: iceweasel: not found
/usr/bin/xdg-open: 778: /usr/bin/xdg-open: seamonkey: not found
/usr/bin/xdg-open: 778: /usr/bin/xdg-open: mozilla: not found
/usr/bin/xdg-open: 778: /usr/bin/xdg-open: epiphany: not found
/usr/bin/xdg-open: 778: /usr/bin/xdg-open: konqueror: not found
/usr/bin/xdg-open: 778: /usr/bin/xdg-open: chromium-browser: not found
/usr/bin/xdg-open: 778: /usr/bin/xdg-open: google-chrome: not found
/usr/bin/xdg-open: 778: /usr/bin/xdg-open: www-browser: not found
/usr/bin/xdg-open: 778: /usr/bin/xdg-open: links2: not found
/usr/bin/xdg-open: 778: /usr/bin/xdg-open: elinks: not found
/usr/bin/xdg-open: 778: /usr/bin/xdg-open: links: not found

with it eventually opening the webpage in lynx, which I do have installed in the Linux/WSL environment.

@faho
Copy link
Member

faho commented Mar 20, 2019

Also, I should note that in order for xdg-open to send things over to the windows side, it needs to have wsl-open or similarly functioning tool,

Okay, so that means we want xdg-open if wsl-open is installed but cmd.exe (or rather /mnt/c/.../cmd.exe) if it isn't. At which point we could just try wsl-open.

If neither exist, there isn't really anything we can do automatically so those users will just have to set $BROWSER.

@floam
Copy link
Member

floam commented Mar 20, 2019

I think WSLENV actually is supposed to be empty by default. It looks like you need to set it up manually from the windows side to expose stuff into the Linux environment. It has modifiers for stuff like colon-separating path lists.

https://medium.com/@kevinv92/how-to-use-wslenv-to-share-environment-variables-between-windows-and-wsl-c06099d5b901

@mqudsi
Copy link
Contributor

mqudsi commented Mar 21, 2019

WSLENV is apart from the automated PATH population.

Old: https://docs.microsoft.com/en-us/windows/wsl/interop
New: https://docs.microsoft.com/en-us/windows/wsl/interop#creators-update-and-anniversary-update

Also, I published a utility for opening files/urls/folders/apps from within WSL or cmd.exe: https://github.com/neosmart/open

Since it is compiled to open.exe if it is present in the runtime PATH fish automatically uses it (because our convention is to use file presence checks over uname checks whenever possible) as it shares a name with the macOS utility that does the same, which fish checks for first.

@faho
Copy link
Member

faho commented Mar 22, 2019

Since it is compiled to open.exe if it is present in the runtime PATH fish automatically uses it (because our convention is to use file presence checks over uname checks whenever possible) as it shares a name with the macOS utility that does the same, which fish checks for first.

This is not currently true for help specifically:

https://github.com/fish-shell/fish-shell/blob/master/share/functions/help.fish#L68-L74

# On OS X, we go through open by default
            if test (uname) = Darwin
                if type -q open
                    set fish_browser open
                    set need_trampoline 1
                end
end

That check needs to be command -sq, which is also what we check to stop creating our open function: https://github.com/fish-shell/fish-shell/blob/master/share/functions/open.fish.

faho added a commit that referenced this issue Mar 22, 2019
@faho
Copy link
Member

faho commented Mar 22, 2019

Okay, now it's true.

@faho
Copy link
Member

faho commented Mar 22, 2019

So, how about this check then:

# If the OS appears to be Windows (graphical), try to use cygstart
if type -q cygstart
    set fish_browser cygstart
# If xdg-open is available, just use that
# but only if an X session is running
else if type -q xdg-open; and set -q -x DISPLAY
    set fish_browser xdg-open
end

# `command -s` also works on paths. It'll print the path again if it's an executable
if set -l cmd (command -s cmd.exe /mnt/c/Windows/System32/cmd.exe)
    set fish_browser $cmd[1]
end

if type -q wsl-open
    set fish_browser wsl-open
end

# "command" only because we define a function that might not have a backend.
# (Also TODO think about adding these as backends)
if command -sq open
   set fish_browser open
end

The idea being that the last of cygstart, xdg-open, cmd.exe, wsl-open, open wins. So we e.g. use cmd.exe even if xdg-open is installed because it might not be usable without wsl-open.

Then we remove the special wsl check, and it's all nice and os-agnostic.

@samdmarshall
Copy link
Contributor Author

Sorry, am i missing something here? -- I thought we recognized the fact the the prefix of /mnt/c/Windows/System32/ to the cmd.exe was a crude/inaccurate assumption to make? if we are going to assume that then we should make a variable for it so that the user can modify it to match their windows install path if necessary (or just not touch cmd.exe unless we are actually running on windows, not via WSL?

@faho
Copy link
Member

faho commented Mar 22, 2019

I thought we recognized the fact the the prefix of /mnt/c/Windows/System32/ to the cmd.exe was a crude/inaccurate assumption to make?

It's a heuristic, it doesn't have to be perfect. We try cmd.exe, both via $PATH and via /mnt/c/..., but we also try wsl-open and open. If any one of them exists we've won.

And if none of them do the user will just have to set $fish_help_browser.

if we are going to assume that then we should make a variable for it so that the user can modify it

Or just set $fish_help_browser.

@samdmarshall
Copy link
Contributor Author

oh, oops, I didn't connect that part to it. Thanks for the info, and what seems to be a quick fix!

faho added a commit to faho/fish-shell that referenced this issue Mar 22, 2019
We now try cmd.exe via $PATH and via a common location, wsl-open, and
an open command.

Fixes fish-shell#5756.

[ci skip]
@faho
Copy link
Member

faho commented Mar 26, 2019

@samdmarshall: Can you confirm that #5759 works for you?

@samdmarshall
Copy link
Contributor Author

@faho I just built fish locally (your branch) and that doesn't work for me. The command help command results in trying to invoke:

open https://fishshell.com/docs/3.0/index.html

which causes an error for me (Couldn't get a file descriptor referring to the console). I want to say this is a WSL thing perhaps. On the other hand if alter the resulting command to be xdg-open … instead, it works perfectly. So while I do have the standard open (/bin/open) on my system, it isn't gonna work or doesn't work correctly.

@faho
Copy link
Member

faho commented Mar 27, 2019

Ah krampus. I forgot that this is Debian, which ships a thing called "open", only it's a symlink to "openvt" (see #5091, https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=732796).

Okay, we need to put open < xdg-open.

faho added a commit to faho/fish-shell that referenced this issue Mar 27, 2019
Some systems like Debian have "open" as a symlink to "openvt" (for... historical
reasons).

See fish-shell#5756.

[ci skip]
faho added a commit to faho/fish-shell that referenced this issue Mar 27, 2019
We now try cmd.exe via $PATH and via a common location, wsl-open, and
an open command.

Fixes fish-shell#5756.

[ci skip]
faho added a commit that referenced this issue Mar 27, 2019
Some systems like Debian have "open" as a symlink to "openvt" (for... historical
reasons).

See #5756.

[ci skip]
faho added a commit that referenced this issue Mar 27, 2019
We now try cmd.exe via $PATH and via a common location, wsl-open, and
an open command.

Fixes #5756.

[ci skip]
@samdmarshall
Copy link
Contributor Author

Thanks for the super quick turn-around on this, and thank you (again) for all your work as maintainers!!
🙇‍♀️

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants