Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Code Quality: Replace Vanara with CsWin32 #15000

Open
gumbarros opened this issue Mar 19, 2024 · 9 comments · May be fixed by #17002
Open

Code Quality: Replace Vanara with CsWin32 #15000

gumbarros opened this issue Mar 19, 2024 · 9 comments · May be fixed by #17002
Labels
help wanted Extra attention is needed
Milestone
Files v4

Comments

@gumbarros
Copy link
Contributor

gumbarros commented Mar 19, 2024 •
edited by yaira2
Loading

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.
@gumbarros
Copy link
Contributor Author

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.

@gumbarros gumbarros mentioned this issue Mar 26, 2024
Closed
@0x5bfa
Copy link
Member

0x5bfa commented Mar 26, 2024

I like this idea. Feel free to work on it with @yaira2’s approval.

@yaira2
Copy link
Member

yaira2 commented Mar 26, 2024

Sounds good, it might be worth splitting this into multiple PRs to make it easier to test and review.

@yaira2 yaira2 moved this from 🆕 New to 📋 Planning stage in Files task board Mar 26, 2024
@yaira2 yaira2 moved this from 📋 Planning stage to 🔖 Ready to build in Files task board Mar 26, 2024
@gumbarros gumbarros mentioned this issue Mar 26, 2024
Closed
8 tasks
@0x5bfa

This comment was marked as outdated.

Sign in to view
@gumbarros
Copy link
Contributor Author

Just checked about CsWin32, looks promising because of source generator usage like LibraryImportAttribute. Will make a try tonight at NativeFindStorageItemHelper.cs

@gumbarros gumbarros changed the title Code Quality: SYSLIB1054 - Use LibraryImportAttribute instead of DllImportAttribute to generate p/invoke marshalling code at compile time. Code Quality: SYSLIB1054 - Use CsWin32 instead of DllImportAttribute to generate p/invoke marshalling code at compile time. Mar 28, 2024
@gumbarros
Copy link
Contributor Author

Don't know if this is expected behavior when migrating:
microsoft/CsWin32#1167

@0x5bfa

This comment was marked as outdated.

Sign in to view
@0x5bfa

This comment was marked as outdated.

Sign in to view
@gumbarros gumbarros mentioned this issue Apr 7, 2024
Merged
8 tasks
@Josh65-2201 Josh65-2201 moved this from 🔖 Ready to build to 🏗 In progress in Files task board Apr 7, 2024
This was referenced Apr 7, 2024
Merged
Merged
Merged
@gumbarros gumbarros mentioned this issue Apr 14, 2024
Merged
8 tasks
@gumbarros gumbarros mentioned this issue May 4, 2024
Merged
8 tasks
@0x5bfa 0x5bfa mentioned this issue May 19, 2024
Merged
@0x5bfa 0x5bfa mentioned this issue Jul 5, 2024
Merged
6 tasks
@Josh65-2201 Josh65-2201 changed the title Code Quality: SYSLIB1054 - Use CsWin32 instead of DllImportAttribute to generate p/invoke marshalling code at compile time. Code Quality: Replace Vanara with CsWin32 Jul 6, 2024
@0x5bfa 0x5bfa mentioned this issue Jul 7, 2024
Merged
@0x5bfa 0x5bfa mentioned this issue Jul 17, 2024
Merged
2 tasks
This was referenced Jul 30, 2024
Closed
Merged
@0x5bfa 0x5bfa mentioned this issue Aug 3, 2024
Closed
@0x5bfa 0x5bfa mentioned this issue Aug 15, 2024
Merged
@0x5bfa 0x5bfa mentioned this issue Aug 27, 2024
Merged
This was referenced Sep 8, 2024
Merged
Closed
This was referenced Sep 21, 2024
Closed
Merged
Merged
Merged
@0x5bfa 0x5bfa mentioned this issue Oct 5, 2024
Closed
@0x5bfa
Copy link
Member

0x5bfa commented Oct 10, 2024 •
edited
Loading

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
  • E: SetWindowLongPtr32
  • E: SetWindowLongPtr64
  • E: SHBrowseForFolder
  • E: SHGetPathFromIDList
  • E: CloseHandle
  • M: GetOverlappedResult
  • E: CancelIo
  • E: CancelIoEx
  • M: WaitForMultipleObjectsEx
  • E: ResetEvent
  • E: WaitForSingleObjectEx
  • M: ReadDirectoryChangesW
  • E: CreateFileFromAppW
  • M: DeviceIoControl
  • M: ToUnicode
  • M: ToUnicodeEx
  • E: GetKeyboardState
  • E: GetKeyboardLayout
  • E: MapVirtualKey
  • M: TranslateMessage
  • E: CreateFileFromApp
  • E: GetFileAttributesExFromApp
  • E: SetFileAttributesFromApp
  • E: SetFilePointer
  • M: ReadFile
  • M: WriteFile
  • M: WriteFileEx
  • M: GetFileTime
  • M: SetFileTime
  • E: GetFileInformationByHandleEx
  • E: GetFileInformationByHandleEx
  • E: FindFirstStreamW
  • E: FindNextStreamW
  • E: GetDpiForMonitor
  • E: OpenProcessToken
  • E: GetCurrentProcess
  • E: GetTokenInformation
  • E: GetLengthSid
  • M: CryptUnprotectData
  • E: IsWow64Process2
  • E: FindNextFile
  • E: FindClose
  • M: FileTimeToSystemTime
  • E: FindFirstFileExFromApp
  • E: CompareStringEx
  • E: SHCreateStreamOnFileEx
  • M: SHCreateItemFromParsingName
  • M: CoCreateInstance
  • E: RegisterApplicationRestart
  • M: SHGetKnownFolderPath

Undocumented native functions

  • IsElevationRequired
  • SHUpdateRecycleBinIcon

Sorry, something went wrong.

@0x5bfa 0x5bfa mentioned this issue Oct 24, 2024
Merged
@0x5bfa 0x5bfa mentioned this issue Nov 19, 2024
Closed
5 tasks
@yaira2 yaira2 mentioned this issue Nov 22, 2024
Merged
@0x5bfa 0x5bfa mentioned this issue Nov 25, 2024
Closed
@yaira2 yaira2 mentioned this issue Nov 26, 2024
Merged
@yaira2 yaira2 added the help wanted Extra attention is needed label Nov 26, 2024
@yaira2 yaira2 added this to the Files v4 milestone Feb 10, 2025
@0x5bfa 0x5bfa mentioned this issue Mar 16, 2025
Merged
This was referenced Mar 29, 2025
Open
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Extra attention is needed
Projects
Files task board
Status: 🏗 In progress
Milestone
Files v4
Development

Successfully merging a pull request may close this issue.

Code Quality: Introduced WindowsBulkOperations in lieu of ShellFileOperations2 0x5bfa/Files
3 participants
@yaira2 @gumbarros @0x5bfa