diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 3666358..815133e 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -22,3 +22,39 @@ jobs: go-version: ${{ matrix.go }} - name: Test Race run: go test -race ./... + +# WSL Test: disabled for now since it's very slow (~5 minutes) +# +# name: Test fastwalk on Windows WSL +# +# on: +# push: +# branches: [ master ] +# pull_request: +# branches: [ master ] +# +# jobs: +# build: +# runs-on: windows-latest +# strategy: +# matrix: +# go: [1.22] +# steps: +# - uses: actions/checkout@v4 +# with: +# fetch-depth: 1 +# - uses: Vampire/setup-wsl@v3 +# with: +# distribution: Ubuntu-24.04 +# - name: Set up Go +# uses: actions/setup-go@v5 +# with: +# go-version: ${{ matrix.go }} +# - name: Build Test +# run: go test -c -o fastwalk.test.exe +# - name: Test WSL +# shell: wsl-bash {0} +# run: | +# cp ./fastwalk.test.exe /tmp/fastwalk.test.exe +# cd /tmp +# ./fastwalk.test.exe -test.v -test.run TestRunningUnderWSL -test-wsl diff --git a/fastwalk.go b/fastwalk.go index 53107bd..2f3bcd0 100644 --- a/fastwalk.go +++ b/fastwalk.go @@ -88,35 +88,59 @@ func DefaultNumWorkers() int { } // DefaultToSlash returns true if this is a Go program compiled for Windows -// running in an environment ([WSL] or [MSYS/MSYS2]) that uses forward slashes -// as the path separator instead of the native backslash. -// On all other platforms this is a no-op and returns false since the native -// path separator is a forward slash and does not need to be converted. +// running in an environment ([MSYS/MSYS2] or [Git for Windows]) that uses +// forward slashes as the path separator instead of the native backslash. // -// This check does not apply to programs compiled in [WSL] [MSYS/MSYS2] or for -// Linux (GOOS=linux). It only applies to Go programs compiled for Windows -// (GOOS=windows) that are executed from [WSL] or [MSYS/MSYS2]. +// On non-Windows OSes this is a no-op and always returns false. // -// To detect if we're running in [MSYS/MSYS2] we check that "MSYSTEM" environment -// variable is either "MINGW64", "MINGW32", or "MSYS". +// To detect if we're running in [MSYS/MSYS2] we check if the "MSYSTEM" +// environment variable exists. // -// The following heuristics are used to detect if we're running in [WSL]: +// DefaultToSlash does not detect if this is a Windows executable running in [WSL]. +// Instead, users should (ideally) use programs compiled for Linux in WSL. // -// - Existence of "/proc/sys/fs/binfmt_misc/WSLInterop". -// - If the "WSL_DISTRO_NAME" environment variable is set. -// - If "/proc/version" contains either "Microsoft" or "microsoft". +// See: [github.com/junegunn/fzf/issues/3859] // -// The result of the WSL check is cached for performance reasons. +// NOTE: The reason that we do not check if we're running in WSL is that the +// test was inconsistent since it depended on the working directory (it seems +// that "/proc" cannot be accessed when programs are ran from a mounted Windows +// directory) and what environment variables are shared between WSL and Win32 +// (this requires explicit [configuration]). // -// See: https://github.com/junegunn/fzf/issues/3859 -// -// [WSL]: https://learn.microsoft.com/en-us/windows/wsl/about // [MSYS/MSYS2]: https://www.msys2.org/ +// [WSL]: https://learn.microsoft.com/en-us/windows/wsl/about +// [Git for Windows]: https://gitforwindows.org/ +// [github.com/junegunn/fzf/issues/3859]: https://github.com/junegunn/fzf/issues/3859 +// [configuration]: https://devblogs.microsoft.com/commandline/share-environment-vars-between-wsl-and-windows/ func DefaultToSlash() bool { if runtime.GOOS != "windows" { return false } - return useForwardSlash() + // Previously this function attempted to determine if this is a Windows exe + // running in WSL. The check was: + // + // * File /proc/sys/fs/binfmt_misc/WSLInterop exist + // * Env var "WSL_DISTRO_NAME" exits + // * /proc/version contains "Microsoft" or "microsoft" + // + // Below are my notes explaining why that check was flaky: + // + // NOTE: This appears to fail when ran from WSL when the current working + // directory is a Windows directory that is mounted ("/mnt/c/...") since + // "/proc" is not accessible. It works if ran from a directory that is not + // mounted. Additionally, the "WSL_DISTRO_NAME" environment variable is not + // set when ran from WSL. + // + // I'm not sure what causes this, but it would be great to find a solution. + // My guess is that when ran from a Windows directory it uses the native + // Windows path syscalls (for example os.Getwd reports the canonical Windows + // path when a Go exe is ran from a mounted directory in WSL, but reports the + // WSL path when ran from outside a mounted Windows directory). + // + // That said, the real solution here is to use programs compiled for Linux + // when running in WSL. + _, ok := os.LookupEnv("MSYSTEM") + return ok } // DefaultConfig is the default Config used when none is supplied. diff --git a/path_portable.go b/path_portable.go deleted file mode 100644 index 24dda83..0000000 --- a/path_portable.go +++ /dev/null @@ -1,7 +0,0 @@ -//go:build !windows - -package fastwalk - -func useForwardSlash() bool { - return false -} diff --git a/path_windows.go b/path_windows.go deleted file mode 100644 index b828917..0000000 --- a/path_windows.go +++ /dev/null @@ -1,62 +0,0 @@ -//go:build windows - -package fastwalk - -import ( - "bytes" - "os" - "runtime" - "sync" -) - -func useForwardSlash() bool { - // Use a forward slash as the path separator if this a Windows executable - // running in either MSYS/MSYS2 or WSL. - return runningUnderMSYS() || runningUnderWSL() -} - -// runningUnderMSYS reports if we're running in a MSYS/MSYS2 enviroment. -// -// See: https://github.com/sharkdp/fd/pull/730 -func runningUnderMSYS() bool { - switch os.Getenv("MSYSTEM") { - case "MINGW64", "MINGW32", "MSYS": - return true - } - return false -} - -var underWSL struct { - once sync.Once - wsl bool -} - -// runningUnderWSL returns if we're a Widows executable running in WSL. -// See [DefaultToSlash] for an explanation of the heuristics used here. -func runningUnderWSL() bool { - if runtime.GOOS != "windows" { - return false - } - w := &underWSL - w.once.Do(func() { - w.wsl = func() bool { - // Best check (but not super fast) - if _, err := os.Lstat("/proc/sys/fs/binfmt_misc/WSLInterop"); err == nil { - return true - } - // Fast check, but could provide a false positive if the user sets - // this on the Windows side. - if os.Getenv("WSL_DISTRO_NAME") != "" { - return true - } - // If the binary is compiled for Windows and we're running under Linux - // then honestly just the presence of "/proc/version" should be enough - // to determine that we're running under WSL, but check the version - // string just to be pedantic. - data, _ := os.ReadFile("/proc/version") - return bytes.Contains(data, []byte("microsoft")) || - bytes.Contains(data, []byte("Microsoft")) - }() - }) - return w.wsl -}