fix: decode subprocess output as UTF-8 in save.py on Windows (#6218)
* Fix UnicodeDecodeError on Windows reading subprocess output in save path On Windows the default text encoding is the locale code page (cp1252), not UTF-8. The text-mode subprocess calls in save.py (text=True / universal_newlines=True) set no explicit encoding, so they decode llama.cpp / Ollama output with cp1252. When a child process emits a byte undefined in cp1252 -- e.g. 0x9d, which appears inside the UTF-8 encoding of common punctuation / box-drawing glyphs and in non-ASCII file paths -- the read raises UnicodeDecodeError and aborts GGUF export. Add encoding="utf-8", errors="replace" to all 8 text-mode subprocess calls. errors="replace" also avoids silent mojibake for inputs whose bytes happen to be valid-but-wrong in cp1252. Add tests/saving/test_save_subprocess_utf8_encoding.py: - an AST drift detector asserting every text-mode subprocess call in save.py pins encoding="utf-8" (runs without importing torch/unsloth_zoo) - a behavioural test reproducing the cp1252 failure and the utf-8 fix Relates-to: #2660 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
This commit is contained in:
parent
14ed91e39a
commit
51f1c8732d
2 changed files with 164 additions and 0 deletions
148
tests/saving/test_save_subprocess_utf8_encoding.py
Normal file
148
tests/saving/test_save_subprocess_utf8_encoding.py
Normal file
|
|
@ -0,0 +1,148 @@
|
|||
# Unsloth - 2x faster, 60% less VRAM LLM training and finetuning
|
||||
# Copyright 2023-present Daniel Han-Chen, Michael Han-Chen & the Unsloth team. All rights reserved.
|
||||
#
|
||||
# This program is free software: you can redistribute it and/or modify
|
||||
# it under the terms of the GNU Lesser General Public License as published by
|
||||
# the Free Software Foundation, either version 3 of the License, or
|
||||
# (at your option) any later version.
|
||||
#
|
||||
# This program is distributed in the hope that it will be useful,
|
||||
# but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
# GNU Lesser General Public License for more details.
|
||||
|
||||
"""Regression tests for unslothai/unsloth#2660.
|
||||
|
||||
On Windows the default text encoding is the locale code page (e.g. cp1252),
|
||||
not UTF-8. ``subprocess.Popen`` / ``subprocess.run`` opened in text mode
|
||||
(``text=True`` / ``universal_newlines=True``) without an explicit
|
||||
``encoding`` therefore decode child-process output with cp1252. When
|
||||
llama.cpp / Ollama emit a byte that is undefined in cp1252 (e.g. ``0x9d``,
|
||||
which appears inside the UTF-8 encoding of common punctuation and box-drawing
|
||||
glyphs), the read raises ``UnicodeDecodeError`` and aborts the GGUF export.
|
||||
|
||||
Two checks:
|
||||
|
||||
* ``test_save_subprocess_text_calls_declare_utf8_encoding`` -- a source-level
|
||||
drift detector. It parses ``unsloth/save.py`` (no import, so it runs under
|
||||
the GPU/torch-free harness) and fails if any text-mode subprocess call is
|
||||
missing ``encoding="utf-8"``. This is the regression guard: it is red
|
||||
before the fix and green after.
|
||||
* ``test_utf8_replace_decodes_non_cp1252_subprocess_output`` -- a behavioural
|
||||
check that documents the bug and the fix deterministically on any platform:
|
||||
raw child output that is invalid under cp1252 raises, while the
|
||||
``encoding="utf-8", errors="replace"`` kwargs used by the fix read it
|
||||
cleanly.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import ast
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
SAVE_PY = Path(__file__).resolve().parents[2] / "unsloth" / "save.py"
|
||||
|
||||
|
||||
def _is_subprocess_call(node: ast.Call) -> bool:
|
||||
"""True for ``subprocess.Popen(...)`` / ``subprocess.run(...)``."""
|
||||
func = node.func
|
||||
return (
|
||||
isinstance(func, ast.Attribute)
|
||||
and func.attr in {"Popen", "run"}
|
||||
and isinstance(func.value, ast.Name)
|
||||
and func.value.id == "subprocess"
|
||||
)
|
||||
|
||||
|
||||
def _kw(node: ast.Call, name: str):
|
||||
for kw in node.keywords:
|
||||
if kw.arg == name:
|
||||
return kw.value
|
||||
return None
|
||||
|
||||
|
||||
def _is_true(value) -> bool:
|
||||
return isinstance(value, ast.Constant) and value.value is True
|
||||
|
||||
|
||||
def _is_text_mode(node: ast.Call) -> bool:
|
||||
"""Text mode = ``text=True`` or ``universal_newlines=True``."""
|
||||
return _is_true(_kw(node, "text")) or _is_true(_kw(node, "universal_newlines"))
|
||||
|
||||
|
||||
def _collect_text_mode_subprocess_calls() -> list[ast.Call]:
|
||||
tree = ast.parse(SAVE_PY.read_text(encoding = "utf-8"), filename = str(SAVE_PY))
|
||||
return [
|
||||
node
|
||||
for node in ast.walk(tree)
|
||||
if isinstance(node, ast.Call) and _is_subprocess_call(node) and _is_text_mode(node)
|
||||
]
|
||||
|
||||
|
||||
def test_text_mode_subprocess_calls_exist():
|
||||
"""Guard the guard: if save.py stops using text-mode subprocess calls the
|
||||
drift test below would vacuously pass, so make sure we are actually
|
||||
inspecting something."""
|
||||
calls = _collect_text_mode_subprocess_calls()
|
||||
assert len(calls) >= 6, (
|
||||
f"Expected several text-mode subprocess calls in {SAVE_PY.name}, "
|
||||
f"found {len(calls)} -- has the file been restructured?"
|
||||
)
|
||||
|
||||
|
||||
def test_save_subprocess_text_calls_declare_utf8_encoding():
|
||||
"""Every text-mode subprocess call in save.py must pin encoding='utf-8'.
|
||||
|
||||
Without it, reading llama.cpp/Ollama output crashes on Windows (cp1252).
|
||||
Fails before the #2660 fix, passes after.
|
||||
"""
|
||||
offenders = []
|
||||
for node in _collect_text_mode_subprocess_calls():
|
||||
enc = _kw(node, "encoding")
|
||||
ok = isinstance(enc, ast.Constant) and enc.value == "utf-8"
|
||||
if not ok:
|
||||
offenders.append(node.lineno)
|
||||
|
||||
assert not offenders, (
|
||||
"Text-mode subprocess call(s) in unsloth/save.py missing "
|
||||
'encoding="utf-8" (UnicodeDecodeError on Windows, #2660) at line(s): '
|
||||
+ ", ".join(map(str, sorted(offenders)))
|
||||
)
|
||||
|
||||
|
||||
def test_utf8_replace_decodes_non_cp1252_subprocess_output():
|
||||
"""Document the failure and the fix with a real subprocess.
|
||||
|
||||
The child emits U+201D (right double quote), whose UTF-8 encoding
|
||||
``E2 80 9D`` contains byte 0x9D -- undefined in cp1252. Decoding the raw
|
||||
bytes as cp1252 raises (the bug); the fix's kwargs read it cleanly.
|
||||
"""
|
||||
# All-ASCII argv; the child builds the non-ASCII char itself so this is
|
||||
# deterministic regardless of the parent's locale.
|
||||
child = (
|
||||
"import sys; "
|
||||
"sys.stdout.buffer.write(('tensor ' + chr(0x201D) + ' x\\n').encode('utf-8'))"
|
||||
)
|
||||
|
||||
raw = subprocess.run([sys.executable, "-c", child], capture_output = True).stdout
|
||||
assert b"\x9d" in raw # precondition: output carries the cp1252-undefined byte
|
||||
|
||||
# Failing behaviour before the fix: cp1252 (the Windows default) cannot
|
||||
# decode this output.
|
||||
with pytest.raises(UnicodeDecodeError):
|
||||
raw.decode("cp1252")
|
||||
|
||||
# Correct behaviour after the fix: the exact kwargs save.py now uses.
|
||||
result = subprocess.run(
|
||||
[sys.executable, "-c", child],
|
||||
capture_output = True,
|
||||
text = True,
|
||||
encoding = "utf-8",
|
||||
errors = "replace",
|
||||
)
|
||||
assert result.stdout.startswith("tensor ")
|
||||
assert "”" in result.stdout
|
||||
|
|
@ -186,6 +186,8 @@ def _quantize_q2_k_l(
|
|||
command,
|
||||
shell = False,
|
||||
text = True,
|
||||
encoding = "utf-8",
|
||||
errors = "replace",
|
||||
stdout = subprocess.PIPE,
|
||||
stderr = subprocess.STDOUT,
|
||||
bufsize = 1,
|
||||
|
|
@ -206,6 +208,8 @@ def _quantize_q2_k_l(
|
|||
check = True,
|
||||
capture_output = True,
|
||||
text = True,
|
||||
encoding = "utf-8",
|
||||
errors = "replace",
|
||||
)
|
||||
except subprocess.CalledProcessError as e:
|
||||
if print_output and hasattr(e, "stdout") and e.stdout:
|
||||
|
|
@ -1989,6 +1993,8 @@ def create_ollama_model(username: str, model_name: str, tag: str, modelfile_path
|
|||
["curl", "http://localhost:11434"],
|
||||
capture_output = True,
|
||||
text = True,
|
||||
encoding = "utf-8",
|
||||
errors = "replace",
|
||||
timeout = 3,
|
||||
)
|
||||
if init_check.returncode == 0:
|
||||
|
|
@ -2011,6 +2017,8 @@ def create_ollama_model(username: str, model_name: str, tag: str, modelfile_path
|
|||
text = True,
|
||||
bufsize = 1,
|
||||
universal_newlines = True,
|
||||
encoding = "utf-8",
|
||||
errors = "replace",
|
||||
)
|
||||
|
||||
for line in iter(process.stdout.readline, ""):
|
||||
|
|
@ -2031,6 +2039,8 @@ def push_to_ollama_hub(username: str, model_name: str, tag: str):
|
|||
["curl", "http://localhost:11434"],
|
||||
capture_output = True,
|
||||
text = True,
|
||||
encoding = "utf-8",
|
||||
errors = "replace",
|
||||
timeout = 3,
|
||||
)
|
||||
if init_check.returncode == 0:
|
||||
|
|
@ -2047,6 +2057,8 @@ def push_to_ollama_hub(username: str, model_name: str, tag: str):
|
|||
text = True,
|
||||
bufsize = 1,
|
||||
universal_newlines = True,
|
||||
encoding = "utf-8",
|
||||
errors = "replace",
|
||||
)
|
||||
|
||||
for line in iter(process.stdout.readline, ""):
|
||||
|
|
@ -2758,6 +2770,8 @@ def unsloth_convert_lora_to_ggml_and_push_to_hub(
|
|||
stderr = subprocess.PIPE,
|
||||
bufsize = 1,
|
||||
universal_newlines = True,
|
||||
encoding = "utf-8",
|
||||
errors = "replace",
|
||||
) as sp:
|
||||
for line in sp.stdout:
|
||||
print(line, end = "", flush = True)
|
||||
|
|
@ -2838,6 +2852,8 @@ def unsloth_convert_lora_to_ggml_and_save_locally(
|
|||
stderr = subprocess.PIPE,
|
||||
bufsize = 1,
|
||||
universal_newlines = True,
|
||||
encoding = "utf-8",
|
||||
errors = "replace",
|
||||
) as sp:
|
||||
for line in sp.stdout:
|
||||
print(line, end = "", flush = True)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue