Win8 baseline: Simplify <filesystem> API calls
#5434
Merged
Conversation
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
+ [`CopyFile2`](https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-copyfile2) is "Windows 8 \[desktop apps | UWP apps\]". + [`CopyFileW`](https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-copyfilew) is "Windows XP \[desktop apps | UWP apps\]". + We were inconsistently guarding this with `defined(_ONECORE)` versus `defined(_CRT_APP)`.
+ [`CreateFile2`](https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfile2) is "Windows 8 \[desktop apps | UWP apps\]". + [`CreateFileW`](https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilew) is "Windows XP \[desktop apps only\]".
…ad of `GetFileInformationByHandle`. + [`GetFileInformationByHandleEx`](https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-getfileinformationbyhandleex) is "Windows Vista \[desktop apps | UWP apps\]". + [`GetFileInformationByHandle`](https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfileinformationbyhandle) is "Windows XP \[desktop apps only\]". + The documentation around `FileIdInfo`/[`FILE_ID_INFO`](https://learn.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_id_info) is confusing. It claims "Minimum supported client: None supported" and "Minimum supported server: Windows Server 2012 \[desktop apps only\]", but in modern `filesystem.cpp` it is our only codepath when `defined(_CRT_APP)`. That suggests that it works on Client and works for UWP apps, so I think it might actually be "Windows 8 \[desktop apps | UWP apps\]".
StephanTLavavej
added
enhancement
|
I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
🗺️ Overview
<filesystem>implementation (and older<experimental/filesystem>) contains several codepaths of the form "if we're a UWP app, call a modern API, otherwise call a legacy API". This is not only twice as much code to maintain, but we don't have automated test coverage for UWP apps, which greatly increases risk. Now that our baseline OS is Win8, we can send everything through the modern APIs.filesystem.cpp, where_Get_file_id_by_handle()has the unusual pattern of calling modernGetFileInformationByHandleEx()and then falling back to legacyGetFileInformationByHandle(). See the commit notes below for why I suspect this is unnecessary. This code was moved around by [copy_file] allowFILE_SHARE_*#2718 but the original logic dates back to the initial GitHub commit.⚙️ Commits
CopyFile2instead ofCopyFileW.CopyFile2is "Windows 8 [desktop apps | UWP apps]".CopyFileWis "Windows XP [desktop apps | UWP apps]".defined(_ONECORE)versusdefined(_CRT_APP).CreateFile2instead ofCreateFileW.CreateFile2is "Windows 8 [desktop apps | UWP apps]".CreateFileWis "Windows XP [desktop apps only]".GetFileInformationByHandleExinstead ofGetFileInformationByHandle.GetFileInformationByHandleExis "Windows Vista [desktop apps | UWP apps]".GetFileInformationByHandleis "Windows XP [desktop apps only]".FileIdInfo/FILE_ID_INFOis confusing. It claims "Minimum supported client: None supported" and "Minimum supported server: Windows Server 2012 [desktop apps only]", but in modernfilesystem.cppit is our only codepath whendefined(_CRT_APP). That suggests that it works on Client and works for UWP apps, so I think it might actually be "Windows 8 [desktop apps | UWP apps]". This PR certainly appears to work on my client Windows 11 machine.📜 Changelog
<filesystem>implementation by unconditionally calling APIs that were added in Windows 8.