Code Quality: Replace Vanara with CsWin32
Description
We're working on utilizing Native Aot in the application, however, runtime marshaler is one of the blocker against this. Vanara uses loads of runtime marshaler via DllImport and ComImport and we're working on replacing it with CsWin32, ultimately removing its reference completely.
Concerned code
- Vanara references
- DllImport we explicitly use within the codebase
Gains
- AoT compliant codebase
- Less 3rd party references
Requirements
- Replace DllImport with CsWin32
Comments
CsWin32 requires you to add interfaces, functions, constants, enums, snd etc in NativeMethods.txt placed in Files.App.CsWin32. #define'd constants also need to be written in that text file and use like "PInvoke.CMF_DEFAULT" (They don't get placed in a particular namespace). For more info, download ILSpy and win32metadata.winmd in order to see the full projections.
https://learn.microsoft.com/en-us/dotnet/standard/native-interop/pinvoke-source-generation
Another reference explaining the benefits. I think this is an important issue.
I like this idea. Feel free to work on it with @yaira2’s approval.
Sounds good, it might be worth splitting this into multiple PRs to make it easier to test and review.
Thanks for the initial try. Do you know CsWin32? If you know, you can replace them with API generated by CsWin32. Using DllImport or LibraryImport is deprecation in Files app because we plan to use CsWin32 instead of those code and Vanara.
Just checked about CsWin32, looks promising because of source generator usage like LibraryImportAttribute. Will make a try tonight at NativeFindStorageItemHelper.cs
Don't know if this is expected behavior when migrating: https://github.com/microsoft/CsWin32/issues/1167
I'm not sure, but this function in C language uses void*.
I assume that whether adding in or out and even ref is defined in win32metadata or CsWin32 repo.
#include <fileapifromapp.h>
WINSTORAGEAPI HANDLE FindFirstFileExFromAppW(
LPCWSTR lpFileName,
FINDEX_INFO_LEVELS fInfoLevelId,
LPVOID lpFindFileData, // HERE
FINDEX_SEARCH_OPS fSearchOp,
LPVOID lpSearchFilter,
DWORD dwAdditionalFlags
) noexcept;
You can use Marshal.PtrToStructure:
using System.Runtime.InteropServices;
Marshal.PtrToStructure((nint)lpFindFileData, typeof(WIN32_FIND_DATA));
And you don't need to migrate all of inside of NativeFindStorageItemHelper because it would make us unreviewable so why not migrate step by step.
Also, if you're struggling with some issues you can skip, or you can pick functions up from Win32PInvoke that I made in my PR #15075 after merged because I gathered methods into single file and I think it's a good play ground as well for you.
Now you can see Files.App.Helpers.Win32 Win32PInvoke class. We can replace them with CsWin32 piece by piece (starting from about 10 functions).
[!IMPORTANT]
- Please Use CsWin32 instead of replacing with LibraryImport. This Win32PInvoke class should be going to contain undocumented native functions defined through LibraryImport or DllImport when possible.
- Don't use "*FromApp" functions. They are intended to be used on UWP. Instead, use native ones.
- E is easy, M is medium, D is difficult. If anyone else can take a look, it'd be good to start from ones marked as E.
Documented native methods (targets to convert to CsWin32)
- [ ] M: RmRegisterResources
- [ ] M: RmStartSession
- [ ] E: RmEndSession
- [ ] D: RmGetList
- [ ] E: SetPropW
- [ ] E: CreateEvent
- [ ] E: SetEvent
- [ ] E: GetDpiForWindow
- [ ] M: CoWaitForMultipleObjects
- [x] E: SetWindowLongPtr32
- [x] E: SetWindowLongPtr64
- [ ] E: SHBrowseForFolder
- [ ] E: SHGetPathFromIDList
- [ ] E: CloseHandle
- [x] M: GetOverlappedResult
- [x] E: CancelIo
- [ ] E: CancelIoEx
- [x] M: WaitForMultipleObjectsEx
- [x] E: ResetEvent
- [ ] E: WaitForSingleObjectEx
- [ ] M: ReadDirectoryChangesW
- [ ] E: CreateFileFromAppW
- [ ] M: DeviceIoControl
- [x] M: ToUnicode
- [ ] M: ToUnicodeEx
- [x] E: GetKeyboardState
- [x] E: GetKeyboardLayout
- [x] E: MapVirtualKey
- [x] M: TranslateMessage
- [ ] E: CreateFileFromApp
- [ ] E: GetFileAttributesExFromApp
- [ ] E: SetFileAttributesFromApp
- [x] E: SetFilePointer
- [ ] M: ReadFile
- [ ] M: WriteFile
- [ ] M: WriteFileEx
- [ ] M: GetFileTime
- [ ] M: SetFileTime
- [ ] E: GetFileInformationByHandleEx
- [x] E: GetFileInformationByHandleEx
- [ ] E: FindFirstStreamW
- [ ] E: FindNextStreamW
- [x] E: GetDpiForMonitor
- [x] E: OpenProcessToken
- [x] E: GetCurrentProcess
- [x] E: GetTokenInformation
- [x] E: GetLengthSid
- [x] M: CryptUnprotectData
- [ ] E: IsWow64Process2
- [ ] E: FindNextFile
- [ ] E: FindClose
- [ ] M: FileTimeToSystemTime
- [ ] E: FindFirstFileExFromApp
- [x] E: CompareStringEx
- [ ] E: SHCreateStreamOnFileEx
- [ ] M: SHCreateItemFromParsingName
- [ ] M: CoCreateInstance
- [ ] E: RegisterApplicationRestart
- [ ] M: SHGetKnownFolderPath
Undocumented native functions
- [x] ~~IsElevationRequired~~
- [x] ~~SHUpdateRecycleBinIcon~~
I've replaced all the PInvoke code that doesn't rely on pointers and unsafe in #17118 and #17119 I'm still working on replacing code that does rely on pointers in #17086, #17107, and #17108 (+ later PRs), although since I'm not as experienced with working with pointers and unsafe they'll take quite a bit longer to replace 🙂
The reason I opened #17120 was to demonstrate how close the codebase is to complete removal of hardcoded native functions and I was surprised that there isn't as much work in that area left as I thought there was. Nice work!