Skip to content

Commit

Permalink
Use bytes in bash.exe check; retest no-distro case
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
EliahKagan committed Nov 25, 2023
1 parent 7ff3cee commit d5ed266
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 12 deletions.
6 changes: 0 additions & 6 deletions .github/workflows/pythonpackage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 7 additions & 6 deletions test/test_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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)


Expand Down

0 comments on commit d5ed266

Please sign in to comment.