From 51f1c8732dba3cb6076a5929142fc2f933e6b353 Mon Sep 17 00:00:00 2001 From: dylanschroers <60888108+dylanschroers@users.noreply.github.com> Date: Fri, 12 Jun 2026 04:31:31 -0400 Subject: [PATCH] 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 * [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 Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .../test_save_subprocess_utf8_encoding.py | 148 ++++++++++++++++++ unsloth/save.py | 16 ++ 2 files changed, 164 insertions(+) create mode 100644 tests/saving/test_save_subprocess_utf8_encoding.py diff --git a/tests/saving/test_save_subprocess_utf8_encoding.py b/tests/saving/test_save_subprocess_utf8_encoding.py new file mode 100644 index 0000000000..4a609cd7b7 --- /dev/null +++ b/tests/saving/test_save_subprocess_utf8_encoding.py @@ -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 diff --git a/unsloth/save.py b/unsloth/save.py index 629cbb9548..a6cc665d3e 100644 --- a/unsloth/save.py +++ b/unsloth/save.py @@ -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)