From d5ed266f2208367e4300c29fa4cbda9cd2db0fb9 Mon Sep 17 00:00:00 2001 From: Eliah Kagan Date: Sat, 25 Nov 2023 05:06:23 -0500 Subject: [PATCH] Use bytes in bash.exe check; retest no-distro case This removes text=True from the subprocess.run call, changing str literals to bytes where appropriate and (less importantly) using "%r" instead of "%s" in log messages so it's clear that printing the repr of a bytes object is, at least for now, intentional. The reason for this is that the encoding of error messages produced by running the WSL bash.exe, when it attempts but fails to use a WSL system, varies based on what error occurred. When no systems are installed, the output can be decoded as UTF-8. When an error from "deeper down" is reported, at least for Bash/Service errors, the output is encoded in UTF-16LE, and attempting to decode it as UTF-8 interleaves lots of null characters in the best case. With a bytes object, loss of information is avoided, and it is clear on inspection that the output requires decoding. The most common case of such an error is *probably*: Insufficient system resources exist to complete the requested service. Error code: Bash/Service/CreateInstance/CreateVm/HCS/0x800705aa However, that is tricky to produce intentionally on some systems. To produce a test error, "wsl --shutdown" can be run repeatedly while a _WinBashStatus.check() call is in progress. This produces: The virtual machine or container was forcefully exited. Error code: Bash/Service/0x80370107 Assuming the output always includes the text "Error code:", it may be feasible to reliably detect which cases it is. This could especially improve the log message. But for now that is not done. In adddition to changing from text to binary mode, this commit also temporarily removes the setup-wsl step from the CI workflow again, to verify on CI that the text-to-binary change doesn't break the WslNoDistro case. Manual testing shows the other cases still work. --- .github/workflows/pythonpackage.yml | 6 ------ test/test_index.py | 13 +++++++------ 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/.github/workflows/pythonpackage.yml b/.github/workflows/pythonpackage.yml index 3915296ef..cef24799e 100644 --- a/.github/workflows/pythonpackage.yml +++ b/.github/workflows/pythonpackage.yml @@ -35,12 +35,6 @@ jobs: python-version: ${{ matrix.python-version }} allow-prereleases: ${{ matrix.experimental }} - - name: Set up WSL (Windows) - if: startsWith(matrix.os, 'windows') - uses: Vampire/setup-wsl@v2.0.2 - with: - distribution: Debian - - name: Prepare this repo for tests run: | ./init-tests-after-clone.sh diff --git a/test/test_index.py b/test/test_index.py index f60a86309..8824d6648 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -94,10 +94,11 @@ def check(cls): if os.name != "nt": return cls.Inapplicable() - no_distro_message = "Windows Subsystem for Linux has no installed distributions." + # Use bytes because messages for different WSL errors use different encodings. + no_distro_message = b"Windows Subsystem for Linux has no installed distributions." def error_running_bash(error): - log.error("Error running bash.exe to check WSL status: %s", error) + log.error("Error running bash.exe to check WSL status: %r", error) return cls.ErrorWhileChecking(error) try: @@ -106,7 +107,7 @@ def error_running_bash(error): # information on ways to check for WSL, see https://superuser.com/a/1749811. script = 'test -e /proc/sys/fs/binfmt_misc/WSLInterop; echo "$?"' command = ["bash.exe", "-c", script] - proc = subprocess.run(command, capture_output=True, check=True, text=True) + proc = subprocess.run(command, capture_output=True, check=True) except FileNotFoundError: return cls.Absent() except OSError as error: @@ -117,11 +118,11 @@ def error_running_bash(error): return error_running_bash(error) status = proc.stdout.rstrip() - if status == "0": + if status == b"0": return cls.Wsl() - if status == "1": + if status == b"1": return cls.Native() - log.error("Strange output checking WSL status: %s", proc.stdout) + log.error("Strange output checking WSL status: %r", proc.stdout) return cls.ErrorWhileChecking(proc)