Repository navigation
Encode dfs file names for the platform instead of cp1252 - #63
Merged
Merged
Conversation
ryan-kipawa
force-pushed
the
fix/non-ascii-filenames
branch
from
October 12, 2026 13:28
a9ad337 to
0c1581f
Compare
The native library opens char* paths. Windows reads those in the ANSI code page, and Linux passes them to the filesystem as bytes (UTF-8), so hardcoding cp1252 broke paths outside cp1252 on Windows (e.g. Chinese) and any non-ASCII path on Linux. Names that can't be represented in the Windows ANSI code page now raise a ValueError that says so. Closes #23 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
getpreferredencoding returns utf-8 in Python UTF-8 mode, so the error message named the wrong code page; ask Windows for it instead. The eum and MzCart setup paths on Linux were encoded as ascii, failing for installs under a non-ASCII directory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Read the code page with locale.getencoding() instead of calling kernel32, type EncodeFileName's parameter, keep the UnicodeEncodeError as the cause and name the offending character, encode before any file system access in Open, close test files in finally, and base the test skip on the machine's code page rather than on the encoder. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
It names a UI label that can change, and the setting was never verified. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ryan-kipawa
force-pushed
the
fix/non-ascii-filenames
branch
from
October 13, 2026 11:24
64db468 to
31e7977
Compare
ryan-kipawa
marked this pull request as ready for review
October 13, 2026 11:25
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #23
What and why
Opening or creating a dfs file fails when its path contains characters outside cp1252, for example a Chinese folder name on a Chinese Windows machine (#23). The cause:
DfsFile.OpenandDfsBuilder.CreateFileencoded the path ascp1252before handing it to the native library'schar*open. Windows reads thatchar*in the machine's ANSI code page (Python'smbcs), and Linux passes the bytes to the filesystem, which expects UTF-8, so on Linux any non-ASCII path failed, Danish included. A newDfsDLLUtil.EncodeFileNameencodes withmbcson Windows andos.fsencodeelsewhere, and turns a name the ANSI code page cannot represent into aValueErrorthat names the code page and the offending character.DfsFile.Openencodes before any file system check, so such a name fails before any I/O. The Linux library setup paths (EUM.xml,CARTSETUPLINUX) wereascii-encoded the same way, so importing mikecore from a non-ASCII install directory failed; they now useos.fsencode.Commits
1fd0f92Encode dfs file names for the platform, not as cp1252:EncodeFileNameand its two call sites, plustests/test_filename_encoding.py. The test is in the same commit as the fix, so checking out a test-only commit to see it fail is not possible; the Before evidence below runs it againstmasterinstead.b09e84fReport the ANSI code page correctly; fsencode Linux lib paths: the error message asks Windows for the code page (getpreferredencodingsaysutf-8in UTF-8 mode), andeum.py/Projections.pyuseos.fsencodefor the library directory. The message is built with an f-string, as ruff on master requires.323690fAddress review of the file name encoding:locale.getencoding()replacesctypes.windll.kernel32.GetACP()(it returns the ANSI code page even in UTF-8 mode, and drops the pyrefly warning);filename: str | os.PathLike[str], with theos.fspathresult in a newname: strinstead of rebinding the parameter; theUnicodeEncodeErroris chained (from e) and the message names the offending character;Openencodes beforeos.path.isfile/os.access;DfsDLLUtil's docstring now covers marshalling; the tests close files infinallyand skip based onlocale.getencoding()instead of copying the encoder.31e7977Drop the Windows settings advice from the file name error: it named a Windows UI label that can change, and the setting was never verified. The message now states the code page, the unsupported character and the path.Code Review Guide
Glossary
char*file APIs; cp1252 in Western Europe, cp936 in China. Python'smbcscodec encodes to it.os.fsencode: encodes a path the way the OS filesystem expects; UTF-8 (withsurrogateescape) on Linux.char*open: the nativeufslibrary'sdfsFileRead/dfsFileEdit/dfsFileAppend/dfsFileCreatetake a byte string, notwchar_t*, which is why a Windows path outside the ACP cannot be opened at all.Where the change sits
Every dfs open and create goes through the two green call sites into one encoder; the import path changes only run on Linux. Line numbers are at the PR head
323690f.Conventions followed
os.name == "nt"DfsDLL.py:62
DfsFile.py:1579
DfsDLLUtil(extend the existing class, no new module)DfsFile.py:1592
DfsFile.py:1575
testdata/files and pytest'stmp_pathtest_dfsu_builder.py:31 (master)
test_filename_encoding.py:30
Deviations
None. Review asked for two that the first version had (
ctypes.windllfor the code page, and an untypedfilename), and commit 3 removes both.Evidence
Linux runs below are on head
31e7977(rebased ontomaster8c0502b), unless marked "master" or "Before". The Windows runs marked "(earlier)" were made on the first two commits merged locally ontoae7ef82, before the review fixes; I couldn't reach a Windows machine for the new head, so Windows on323690fis covered by CI'swindows-latest(cp1252) jobs only.Before (Linux, master code + this PR's test file):
uv run pytest -q -rfs tests/test_filename_encoding.pyThe Danish open fails with
Exception: Could not load file .../Øresund æøå/Øresund æøå.dfsu (Error code 2003), and the Danish create writes a file under the cp1252-mangled name sofilename.exists()is false. This confirms the Linux failure that the original PR description could only infer.After (Linux):
Windows (cp1252 machine), opening a dfsu in a
项目 合界数模folder (earlier) (script in Verify):UnicodeEncodeError: "'charmap' codec can't encode characters in position 45-46: character maps to <undefined>"ValueError: "File name can not be represented in the Windows ANSI code page (cp1252): C:\\...\\tmp18w1kvn7\\\u9879\u76ee \u5408\u754c\u6570\u6a21\\OresundHD.dfsu. Rename the file or folder, or enable 'Beta: Use Unicode UTF-8 for worldwide language support' in Windows region settings."Windows
pytest -q -rs tests/test_filename_encoding.py(earlier):3 passed, 2 skipped(the two Chinese cases skip as not representable in cp1252; the error-message test runs and passes).Linux import from a non-ASCII install directory (
mikecore/copied under.../søren/):UnicodeEncodeError: 'ascii' codec can't encode character '\xf8' in position 100ateumFilePath.encode("ascii")imported .../after/søren/mikecore/__init__.pyRepo checks (AGENTS.md), on head
31e7977:uv run pytest(Linux)183 passed, 1 skipped, 8 xfailed, 8 subtests passeduv run pytest(Windows, cp1252, earlier)151 passed, 2 skipped, 8 xfailed, 8 subtests passeduv run pyrefly check0 errors; warnings 328, same asmaster8c0502b(ceiling 330): this PR adds noneuvx pre-commit run --all-filesCI on head
323690f(before the rebase; CI re-runs on31e7977):ruff,pyrefly,test (ubuntu-latest / windows-latest, 3.11 / 3.14)all pass. On both Windows jobstests\test_filename_encoding.py .s.s.(181 passed, 2 skipped, 8 xfailed): the Chinese cases skip on cp1252 andtest_unrepresentable_path_gives_clear_errorruns and passes, exercising the newlocale.getencoding()message path.MIKE IO downstream (
test_mikeio.yml, MIKE IOmain), branch64db468, before the rebase onto8c0502bvs mastera5b84a1: the same failures on all 4 jobs, both already failing on master and unrelated to paths:test_eum.py::test_eum_conversion(all jobs) andtest_dfs2.py::test_select_area_rotated_UTM_2(Linux, last-digit float). Counts are identical: ubuntu2 failed, 978 passed, windows1 failed, 978 passed, 1 skipped. Each job's log shows the checkout'smikecore 0.3.0a1installed.Verify it yourself
Windows Chinese-folder script
Run from a checkout's root with
uv run python cn.py .:Linux non-ASCII install directory
Risk
mbcsencodes everythingcp1252did. That rests on the ACP being cp1252 on those machines: thenmbcsandcp1252produce the same bytes for every representable name. Proved on a cp1252 machine by the Danish cases and the full suite (151 passedon Windows, before the review fixes). On a machine whose ACP is not cp1252, a name with e.g.øused to be sent as cp1252 bytes the native library would misread; now it is sent in the ACP or rejected with theValueError. Not tested on a non-cp1252 Windows machine (unproven; the Chinese cases would run there instead of skipping).os.fsencodegives UTF-8, which is what the filesystem stores; ASCII paths encode to the same bytes as before. Proved by the Linux suite and the Before/After above.char*. Users get a clear error instead of a crash incp1252.py.ValueErrorinstead ofUnicodeEncodeError.UnicodeEncodeErroris itself aValueError, soexcept ValueErrorcallers are unaffected; only a caller catchingUnicodeEncodeErrorspecifically would miss it (the MIKE IO suite shows no new failures, see Evidence).char*path with the ANSI file APIs, so that with Windows' UTF-8 setting on, thembcs(= UTF-8) bytes open correctly. Not tested here, and the error message does not suggest it.Not in this PR
DfsBuilder.py:349-350, 511); those are strings stored inside the file, not paths.wchar_t*open in the native ufs library.GetShortPathNameW) could open existing files outside the ANSI code page without the OS setting; not attempted.windll, typefilename,finallyin the tests, theDfsDLLUtildocstring) are done in commit 3.🤖 Generated with Claude Code