Transparently add the \\?\ prefix to Win32 calls for extended length path handling #226
Conversation
jakepetroules
requested review from
glessard,
lorentey and
milseman
as code owners
|
I anticipate there might be some contention about this patch given the general spirit of SwiftSystem not incorporating higher level cross platform translations/behaviors (which should go in Foundation) but wanted to send this up to prompt some discussion of this idea. I'd like to point out that we already convert UTF-8 strings to UTF-16 and normalize slashes in file paths, so I feel that adding the extended length prefix is still within the spirit of the level of platform translations that SwiftSystem should be doing. Without doing this, a lot of programs will simply fail to access files whose paths are over the length limit; it's very unrealistic to assume that developers (even those relatively familiar with Windows) know about \?\ or will properly propagate it everywhere it's needed. Do other folks agree? |
|
This approach doesn't seem unreasonable to me. It's about setting up the syscalls for success, just as we've gone to significant lengths not to clobber the errno state prematurely. |
|
@swift-ci test |
jakepetroules
force-pushed
the
windows-extended-length-path-handling
branch
from
a2cd411 to
f008ab0
Compare
|
@swift-ci test |
jakepetroules
force-pushed
the
windows-extended-length-path-handling
branch
from
f008ab0 to
eec0886
Compare
|
@swift-ci test |
jakepetroules
force-pushed
the
windows-extended-length-path-handling
branch
from
eec0886 to
208a08e
Compare
|
@swift-ci test |
| // 2. Canonicalize the path. | ||
| // This will add the \\?\ prefix if needed based on the path's length. | ||
| let capacity = Int16.max | ||
| return try withUnsafeTemporaryAllocation(of: WCHAR.self, capacity: numericCast(capacity)) { outBuffer in |
There was a problem hiding this comment.
This is allocating 32767 bytes, and I expect that will be executed as a heap allocation, repeated every time this is called. Usually swift-system tries to avoid doing implicit allocations like that, as clients sometimes get infuriated when they find out that a low-level syscall wrapper library engages in those.
We may have already lost those clients on Windows; I worry that this may make it harder to try winning them back.
There was a problem hiding this comment.
I changed this to use PathAllocCanonicalize, which has Windows allocate the buffer and can theoretically be smaller, but it's always going to be a heap allocation.
That said, Win32 APIs are not syscalls; they are higher level than the Unix "equivalents". Windows does not have a stable syscall interface and thus it's undocumented. So I think we can get away with some heap allocations where necessary to use the APIs correctly, since many Win32 functions are already doing so internally.
There was a problem hiding this comment.
I think the length of the result of the Path*Canonicalize* functions is always less than or equal to the length of the input string plus one (plus two if you've got a trailing NUL), so we don't need to allocate 32KB here.
I also wonder whether we could have a canonicalize-in-place function that mutates the underlying SystemString as a more efficient option.
| /// to ensure long paths greater than 260 characters are handled correctly. | ||
| /// | ||
| /// - seealso: https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation | ||
| internal func withCanonicalPathRepresentation<Result>(_ body: (Self) throws -> Result) throws -> Result { |
There was a problem hiding this comment.
Should FilePath offer this as a public operation?
There was a problem hiding this comment.
It's tempting, but I'd rather leave that to a future enhancement, I'm not sure yet that this should be a public API or what the right shape would be for it, since we'd have to commit to supporting it if we were to make that change.
jakepetroules
force-pushed
the
windows-extended-length-path-handling
branch
from
208a08e to
a706fe6
Compare
|
@swift-ci test |
jakepetroules
force-pushed
the
windows-extended-length-path-handling
branch
from
a706fe6 to
f376587
Compare
|
@swift-ci test |
jakepetroules
force-pushed
the
windows-extended-length-path-handling
branch
from
f376587 to
23fece6
Compare
|
@swift-ci test |
…path handling On Windows, there is a built-in maximum path limitation of 260 characters under most conditions. This can be extended to 32767 characters under either of the following two conditions: - Adding the longPathAware attribute to the executable's manifest AND enabling the LongPathsEnabled system-wide registry key or group policy. - Ensuring fully qualified paths passed to Win32 APIs are prefixed with \\?\ Unfortunately, the former is not realistic for the Swift ecosystem, since it requires developers to have awareness of this specific Windows limitation, AND set longPathAware in their apps' manifest AND expect end users of those apps to change their system configuration. Instead, this patch transparently prefixes all eligible paths in calls to Win32 APIs with the \\?\ prefix to allow them to work with paths longer than 260 characters without requiring the caller of System to manually prefix the paths. See https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation for more info.
jakepetroules
force-pushed
the
windows-extended-length-path-handling
branch
from
23fece6 to
bc02471
Compare
|
@swift-ci test |
On Windows, there is a built-in maximum path limitation of 260 characters under most conditions. This can be extended to 32767 characters under either of the following two conditions:
Unfortunately, the former is not realistic for the Swift ecosystem, since it requires developers to have awareness of this specific Windows limitation, AND set longPathAware in their apps' manifest AND expect end users of those apps to change their system configuration.
Instead, this patch transparently prefixes all eligible paths in calls to Win32 APIs with the \?\ prefix to allow them to work with paths longer than 260 characters without requiring the caller of System to manually prefix the paths.
See https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation for more info.