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

WSL detection is set based on build system, which is often Real Linux™ #5619

Closed
smanning opened this issue Feb 5, 2019 · 6 comments
Closed

Comments

@smanning
Copy link

smanning commented Feb 5, 2019

Found #5298 after spending quite some time tracking down why fish (3.0.0) was giving me "Could not send own process..." pipe errors on my Windows 10 WSL install (1803). However, the issue states that a message was added to instruct people to upgrade to "Windows 10 1809/17763 or higher". I never got this message. Looking at the code on the master I see this:

            if (is_windows_subsystem_for_linux() && errno == EPERM) {
                debug_safe(1, "Please update to Windows 10 1809/17763 or higher to address known issues "
                        "with process groups and zombie processes.");
            }

The definition of is_windows_subsystem_for_linux() shows this:

/// Detect if we are Windows Subsystem for Linux by inspecting /proc/sys/kernel/osrelease
/// and checking if "Microsoft" is in the first line.
/// See https://github.com/Microsoft/WSL/issues/423 and Microsoft/WSL#2997
constexpr bool is_windows_subsystem_for_linux() {
    // This function is called after fork() and before exec() in postfork.cpp. Make sure we
    // don't allocate any memory here!
#ifdef WSL
    return true;
#else
    return false;
#endif
}

The comment states that it is going to inspect /proc/sys/kernel/osrelease to determine if fish is running on WSL. But when I scan all of the files in the project I find nothing that does this inspection and the only way the "WSL" is set is if fish was built using cmake on a WSL machine. I tested this and it does print the error if you build it that way, but that's an unlikely scenario for most users of WSL.

fish, version 3.0.0-309-gc7656e66
Linux MYHOSTNAME 4.4.0-17134-Microsoft #523-Microsoft Mon Dec 31 17:49:00 PST 2018 x86_64 x86_64 x86_64 GNU/Linux
xterm-256color (WSL default terminal)

@smanning smanning changed the title Error message for users of Windows 10 pre-1809/17763 not displayed. Debug/Info message for users of WSL on Windows 10 pre-1809/17763 not displayed. Feb 5, 2019
@mqudsi
Copy link
Contributor

mqudsi commented Feb 6, 2019

That file/value is currently checked at compile-time to make it a no-op at runtime. After that implementation was coded up it was pointed out in #5115 (comment) that with the current method of package management in use on most WSL-installed distros, it would not suffice.

@mqudsi mqudsi changed the title Debug/Info message for users of WSL on Windows 10 pre-1809/17763 not displayed. WSL detection is set based on build system, which is often Real Linux™ Feb 6, 2019
@floam
Copy link
Member

floam commented Feb 6, 2019

I think we're going to want to go back to actually checking the /proc entry.

@mqudsi
Copy link
Contributor

mqudsi commented Feb 6, 2019

Just a call to uname(2) at startup will suffice.

@zanchey zanchey added this to the fish-future milestone Feb 10, 2019
@zanchey
Copy link
Member

zanchey commented Feb 10, 2019

Do you want this for 3.0.1?

@zanchey
Copy link
Member

zanchey commented Feb 11, 2019

Added it to the release notes instead.

@mqudsi
Copy link
Contributor

mqudsi commented Feb 11, 2019

Sorry @zanchey, it was a very busy weekend to cap off of a very busy week for me.

@mqudsi mqudsi closed this as completed in 9796a33 Feb 15, 2019
@faho faho modified the milestones: fish-future, fish 3.1.0 Feb 20, 2019
@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

No branches or pull requests

5 participants