kj: tolerate ERROR_INVALID_FUNCTION from GetFileInformationByHandleEx on Wine#2633
Conversation
|
The fix seems reasonable but I am slightly offended by the LLM's assumptions about me! (lol)
I certainly did actually test this under WINE! But it was over eight years ago. Perhaps WINE has changed since then. Would you mind adjusting the commit message to say the error seems to have changed? |
|
@kentonv sorry about that :-) It was probably my fault: I asked it figure out which version of Wine introduced a change. It couldn't find the answer and so it blamed you. I'll adjust the test and look into the CI failure. |
|
Don't worry about the CI failure. FreeBSD 15 has a bug (in FreeBSD, not Cap'n Proto) which has made it broken fro some time. And I think windows-2019 isn't running because they don't have action runners for it anymore. |
… on Wine DiskHandle::stat() calls GetFileInformationByHandleEx with the FileCompressionInfo class, which Wine does not implement. Wine's NtQueryInformationFile returns STATUS_NOT_IMPLEMENTED (0xC0000002) for unsupported FileInfoClass values, and RtlNtStatusToDosError maps that to ERROR_INVALID_FUNCTION (1) -- not to ERROR_CALL_NOT_IMPLEMENTED (120), which corresponds to STATUS_NOT_SUPPORTED (0xC00000BB) and is not what Wine returns here. The existing fallback (added in 7c32fcc, "Implement filesystem API for Windows.") only matched ERROR_CALL_NOT_IMPLEMENTED with the comment "Probably WINE." This is no longer sufficient, but it's unclear which Wine version introduced the change. The real Wine behavior surfaces as soon as DiskHandle::stat() runs under Wine, e.g. when running cross-compiled capnpc-c++.exe (or any consumer of kj's filesystem API) on a project that uses libmultiprocess's Windows CI. Treat ERROR_INVALID_FUNCTION the same way: skip the sparse-file space-usage query and continue.
523c4af to
f4a31b2
Compare
|
Ok in that case: I pushed a modified commit message. |
|
thanks! |
I tested this patch as part of cross-compile-then-run-in-wine PR: bitcoin-core/libmultiprocess#272.
However, I didn't check the (LLM generated) reasoning in the commit message.