CsWin32 follow-up: Fusion (GAC) interop + AgileComPointer infrastructure#13746
Conversation
- CommunicationsUtilities: SetEnvironmentVariable via PInvoke.SetEnvironmentVariable (drops a hand-rolled [DllImport]). - Generate HRESULT codes via CsWin32 (TYPE_E_REGISTRYACCESS, TYPE_E_CANTLOADLIBRARY, REGDB_E_CLASSNOTREG) and consume them from UnregisterAssembly and VisualStudioLocationHelper. - Copy.cs: compare IOException.HResult using the existing CsWin32 HRESULT_FROM_WIN32 conversion (explicit operator HRESULT(WIN32_ERROR)) instead of magic-number constants. Gated by FEATURE_WINDOWSINTEROP. - Drop now-unused constants from Tasks/NativeMethods.cs (NullPtr, ERROR_SUCCESS, ERROR_SHARING_VIOLATION, HRESULT_E_CLASSNOTREGISTERED, ERROR_ACCESS_DENIED, ERROR_INVALID_FILENAME) and the dead PFXImportCertStore comment + CRYPTOAPI_BLOB struct left over from the previous crypt32 cleanup.
Adds AgileComPointer/GlobalInterfaceTable infrastructure (from dotnet/winforms) and migrates the four Fusion APIs (CreateAssemblyCache, CreateAssemblyEnum, CreateAssemblyNameObject, GetCachePath) and three COM interfaces (IAssemblyCache, IAssemblyName, IAssemblyEnum) from [ComImport] to CsWin32-style struct-based COM with blittable signatures. Also: ResourceUpdater ERROR_SHARING_VIOLATION -> WIN32_ERROR; remove dead local IClassFactory [ComImport]; add Remaining-Native-Interop.md inventory; update cswin32 skill docs with HRESULT/PCWSTR/blittability/AgileComPointer/PreserveSig guidance; add AgileComPointerTests covering the new infra on net472 and net10.
🔍 Skill Validator Results
Summary
Full validator output```text Found 2 skill(s) [cswin32-com] 📊 cswin32-com: 2,827 BPE tokens [chars/4: 2,727] (standard ~), 11 sections, 5 code blocks [cswin32-com] ⚠ Skill is 2,827 BPE tokens (chars/4 estimate: 2,727) — approaching "comprehensive" range where gains diminish. [cswin32-interop] 📊 cswin32-interop: 2,552 BPE tokens [chars/4: 2,448] (standard ~), 14 sections, 3 code blocks [cswin32-interop] ⚠ Skill is 2,552 BPE tokens (chars/4 estimate: 2,448) — approaching "comprehensive" range where gains diminish. ✅ All checks passed (2 skill(s)) ``` |
There was a problem hiding this comment.
Pull request overview
This PR continues the CsWin32 migration by replacing Fusion (GAC) [ComImport]/[DllImport] interop with struct-based COM + blittable P/Invokes, and introduces GlobalInterfaceTable/AgileComPointer<T> to safely hold COM pointers in managed fields.
Changes:
- Added
GlobalInterfaceTable+AgileComPointer<T>infrastructure and corresponding unit tests. - Migrated Fusion (GAC) interop to manual struct-based COM under
src/Tasks/AssemblyDependency/Fusion/and updated callers to useComScope<T>/AgileComPointer<T>. - Replaced several hard-coded HRESULT/magic numbers with CsWin32
HRESULT/WIN32_ERRORconstants and removed dead interop declarations.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Tasks/UnregisterAssembly.cs | Uses CsWin32 HRESULT constants for COM error checks. |
| src/Tasks/NativeMethods.cs | Removes Fusion [ComImport]/PInvokes; updates AssemblyCacheEnum to struct-based COM + agile pointer storage. |
| src/Tasks/Microsoft.Build.Tasks.csproj | Adds new Fusion interop sources to Tasks compilation inputs. |
| src/Tasks/Copy.cs | Switches retry HRESULT comparisons to CsWin32 WIN32_ERROR/HRESULT (guarded by FEATURE_WINDOWSINTEROP). |
| src/Tasks/BootstrapperUtil/ResourceUpdater.cs | Replaces sharing violation magic number with CsWin32 WIN32_ERROR/HRESULT. |
| src/Tasks/AssemblyDependency/GlobalAssemblyCache.cs | Updates GAC path/query logic to use new Fusion interop + ComScope<T>. |
| src/Tasks/AssemblyDependency/Fusion/Fusion.cs | Adds Fusion enums/structs and fusion.dll P/Invokes with blittable signatures. |
| src/Tasks/AssemblyDependency/Fusion/IAssemblyCache.cs | Adds struct-based COM definition for IAssemblyCache. |
| src/Tasks/AssemblyDependency/Fusion/IAssemblyEnum.cs | Adds struct-based COM definition for IAssemblyEnum. |
| src/Tasks/AssemblyDependency/Fusion/IAssemblyName.cs | Adds struct-based COM definition for IAssemblyName. |
| src/Framework/Windows/Win32/System/Com/GlobalInterfaceTable.cs | Introduces a wrapper around IGlobalInterfaceTable (GIT). |
| src/Framework/Windows/Win32/System/Com/AgileComPointer.cs | Adds a finalizable managed wrapper for COM pointers stored in fields. |
| src/Framework/Windows/Win32/GeneratedInteropClsCompliance.cs | Adds CLS-compliance partial for IGlobalInterfaceTable. |
| src/Framework/VisualStudioLocationHelper.cs | Replaces magic HRESULT with CsWin32 HRESULT.REGDB_E_CLASSNOTREG. |
| src/Framework/Polyfills/IComIIDPolyfills.cs | Adds net472 IComIID polyfill for IGlobalInterfaceTable. |
| src/Framework/NativeMethods.txt | Adds CsWin32 generation inputs for IGlobalInterfaceTable, PCWSTR, HRESULT constants. |
| src/Framework/Microsoft.Build.Framework.csproj | Excludes new COM helper files when FeatureWindowsInterop is disabled. |
| src/Framework/BackEnd/CommunicationsUtilities.cs | Replaces manual kernel32 P/Invoke with PInvoke.SetEnvironmentVariable. |
| src/Framework.UnitTests/AgileComPointerTests.cs | Adds tests for GIT + AgileComPointer<T> behaviors. |
| documentation/Remaining-Native-Interop.md | New inventory/roadmap for remaining handwritten interop in the repo. |
| .github/skills/cswin32-interop/SKILL.md | Documents pointer/nint/T** conventions for DllImport signatures. |
| .github/skills/cswin32-com/SKILL.md | Documents COM struct patterns incl. dual-target IComIID + agile pointer guidance. |
Comments suppressed due to low confidence (2)
src/Tasks/AssemblyDependency/GlobalAssemblyCache.cs:207
RetrievePathFromFusionNameignores theHRESULTfromQueryAssemblyInfoand then checksassemblyInfo.cbAssemblyInfo == 0(which is caller-initialized tosizeof(ASSEMBLY_INFO)and typically not a failure indicator). If the firstQueryAssemblyInfofails,cchBufcan remain 0, leading to a second call with a zero-length buffer/pointer, which risks incorrect results or native buffer misuse. Capture/check theHRESULTand only perform the second call when the first succeeds andcchBuf > 0; return null/empty on failure.
ASSEMBLY_INFO assemblyInfo = default;
assemblyInfo.cbAssemblyInfo = (uint)sizeof(ASSEMBLY_INFO);
fixed (char* pStrongName = strongName)
{
assemblyCache.Pointer->QueryAssemblyInfo(0, pStrongName, &assemblyInfo);
if (assemblyInfo.cbAssemblyInfo == 0)
{
return null;
}
char[] pathBuffer = new char[assemblyInfo.cchBuf];
fixed (char* pPathBuffer = pathBuffer)
{
assemblyInfo.pszCurrentAssemblyPathBuf = pPathBuffer;
assemblyCache.Pointer->QueryAssemblyInfo(0, pStrongName, &assemblyInfo);
src/Tasks/AssemblyDependency/GlobalAssemblyCache.cs:397
GetGacPathcallsFusion.NativeMethods.GetCachePath(...)twice but never checks the returnedHRESULT. If the sizing call fails (or doesn't updategacPathLength),stackallocmay allocate 0 chars and the finalnew string(..., (int)gacPathLength - 1)can throw due to a negative length. Consider validating theHRESULT(and handling the expected buffer-sizing pattern) before allocating and constructing the string.
uint gacPathLength = 0;
unsafe
{
Fusion.NativeMethods.GetCachePath(AssemblyCacheFlags.GAC, null, &gacPathLength);
char* gacPath = stackalloc char[(int)gacPathLength];
Fusion.NativeMethods.GetCachePath(AssemblyCacheFlags.GAC, gacPath, &gacPathLength);
return new string(gacPath, 0, (int)gacPathLength - 1);
}
There was a problem hiding this comment.
Documentation — two concrete inaccuracies in Remaining-Native-Interop.md
No docs.microsoft.com URLs; skill docs look consistent with code patterns.
Two places in the new roadmap doc describe the wrong facts:
-
Section 1 "Ten hand-written
[ComImport]interfaces" — the actual file has 6[ComImport]entries (confirmed by grep), and the status overview table at the top of the same document correctly says[ComImport] x6. The two numbers contradict each other. -
Section 2
ITypeLib2 / ITypeInfo2 / ICreateTypeInfo / ICreateTypeLib2—src/Tasks/Interop.cscontainsIInternetSecurityManager,IInternetSecurityMgrSite,IEnumString. None of the TypeLib interfaces named in the section description exist in that file.
Note
🔒 Integrity filter blocked 2 items
The following items were blocked because they don't meet the GitHub integrity level.
- #13746
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved". - #13746
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
To allow these resources, lower min-integrity in your GitHub frontmatter:
tools:
github:
min-integrity: approved # merged | approved | unapproved | noneGenerated by Expert Code Review (on open) for issue #13746 · ● 44.4M
Source-build does not generate CsWin32 types (no Windows.Win32.Foundation / System.Com), so the new Fusion blittable COM code cannot compile. Exclude the Fusion/*.cs files from the source-build compile, gate the new using directives and the AssemblyCacheEnum / RetrievePathFromFusionName / GetGacPath Windows-only bodies behind FEATURE_WINDOWSINTEROP, falling back to PlatformNotSupportedException.
…s; GetFullName uses BufferScope with insufficient-buffer retry Remove stray documentation/Remaining-Native-Interop.md from PR. Update cswin32-com skill to document HRESULT.ThrowOnFailure().
Replace tautological ShouldBeLessThanOrEqualTo(uint.MaxValue) with a baseline-relative AddRef/Release check. ROT is a process singleton that reports a fixed refcount, so a hard-coded value (e.g. ShouldBe(0u)) does not hold; comparing to a captured baseline still catches double-release (AV/underflow) and ref-leak regressions in AgileComPointer.
…usionName on FEATURE_WINDOWSINTEROP When DotNetBuildSourceOnly=true (Source-Build (Managed) leg), FEATURE_WINDOWSINTEROP is not defined. The XML doc comment for _agileAssemblyEnum sat outside the #if, dangling when the field was excluded (CS1587). GetNextAssemblyFusionName had only a PlatformNotSupportedException body in the source-build configuration with no callers, tripping IDE0051. Both are now wrapped in #if FEATURE_WINDOWSINTEROP.
75acd80 to
cb6747e
Compare
…nd GetNextAssemblyFusionName on FEATURE_WINDOWSINTEROP
DustinCampbell
left a comment
There was a problem hiding this comment.
I walked through all of the code. Looks good to me!
|
Triaged this PR seems complex in .NET context for why this is desirable so we assigned @rainersigwald and @ViktorHofer as reviewers. |
The roadmap wasn't intended to be in the PR and I removed it.
Continues the CsWin32 struct-based COM migration from dotnet#13705 and dotnet#13746, this time covering the CLR metadata APIs (IMetaDataDispenser / IMetaDataImport2 / IMetaDataAssemblyImport) consumed by AssemblyInformation and ManifestUtil.MetadataReader, plus the ICreateTypeLib path used by RegisterAssembly. New files - src/Tasks/AssemblyDependency/Metadata/ - Metadata.cs folder header / namespace docs - IMetaDataDispenser.cs OpenScope with typed CorOpenFlags - IMetaDataImport2.cs GetCustomAttributeByName + GetPEKind with canonical inherited 3-72 slot map - IMetaDataAssemblyImport.cs GetAssemblyProps / GetAssemblyRefProps / GetFileProps / EnumAssemblyRefs / EnumFiles / GetAssemblyFromScope / CloseEnum - ASSEMBLYMETADATA.cs blittable struct from cor.h - OSINFO.cs blittable struct from cor.h - CorOpenFlags.cs typed [Flags] enum - CorAssemblyFlags.cs typed [Flags] enum - CorTokenType.cs 26 mdt* table-type tags - Tokens.cs MdToken / MdAssembly / MdAssemblyRef / MdFile readonly structs with validating constructors - src/Tasks/TypeLib/ICreateTypeLib.cs struct-based wrapper for RegisterAssembly - src/Tasks.UnitTests/MetadataReader_Tests.cs 12 tests / 24 cross-TFM (PEReader on net10, COM on net472), verifies identity parity for Accessibility.dll Modifications - AssemblyInformation.cs uses MdAssembly / BufferScope<char> / typed CorAssemblyFlags; OpenScope asks for IMetaDataImport2 directly (skips one QI); AllocAsmMeta / FreeAsmMeta eliminated - MetadataReader.cs net472 path migrated from [ComImport] to struct-based COM via AgileComPointer; failure to OpenScope returns null (per-file contract) - RegisterAssembly.cs uses the new ICreateTypeLib struct - Microsoft.Build.Tasks.csproj wildcard <Compile Include> for the Fusion / Metadata / TypeLib folders (FeatureWindowsInterop-gated) - NativeMethods.cs legacy [ComImport] declarations removed; only oleaut32 typelib P/Invokes remain; System.Runtime.Versioning using gated on FEATURE_WINDOWSINTEROP for source-build Skills - .github/skills/cswin32-com/SKILL.md adds 'Strongly-typed handle / token wrappers' with CLR validation table; pair-pointer to cswin32-interop; error- handling-parity guidance; IComIID polyfill heading rename - .github/skills/cswin32-interop/SKILL.md reorganized: Rules / Blittable signatures / Infrastructure / BufferScope<T> / Gotchas; source-build verification note Verified - Normal build: 0 warnings, 0 errors (net472 + net10.0) - Source build: 0 warnings, 0 errors (DotNetBuildSourceOnly=true DotNetBuild=true) - Tests: 24/24 pass on both TFMs
CsWin32 follow-up: CLR metadata + TypeLib interop migration Continues the CsWin32 struct-based COM migration from #13705 and #13746, this time covering the CLR metadata APIs (IMetaDataDispenser / IMetaDataImport2 / IMetaDataAssemblyImport) consumed by AssemblyInformation and ManifestUtil.MetadataReader, plus the ICreateTypeLib path used by RegisterAssembly. New files - src/Tasks/AssemblyDependency/Metadata/ - Metadata.cs folder header / namespace docs - IMetaDataDispenser.cs OpenScope with typed CorOpenFlags - IMetaDataImport2.cs GetCustomAttributeByName + GetPEKind with canonical inherited 3-72 slot map - IMetaDataAssemblyImport.cs GetAssemblyProps / GetAssemblyRefProps / GetFileProps / EnumAssemblyRefs / EnumFiles / GetAssemblyFromScope / CloseEnum - ASSEMBLYMETADATA.cs blittable struct from cor.h - OSINFO.cs blittable struct from cor.h - CorOpenFlags.cs typed [Flags] enum - CorAssemblyFlags.cs typed [Flags] enum - CorTokenType.cs 26 mdt* table-type tags - Tokens.cs MdToken / MdAssembly / MdAssemblyRef / MdFile readonly structs with validating constructors - src/Tasks/TypeLib/ICreateTypeLib.cs struct-based wrapper for RegisterAssembly - src/Tasks.UnitTests/MetadataReader_Tests.cs 12 tests / 24 cross-TFM (PEReader on net10, COM on net472), verifies identity parity for Accessibility.dll Modifications - AssemblyInformation.cs uses MdAssembly / BufferScope<char> / typed CorAssemblyFlags; OpenScope asks for IMetaDataImport2 directly (skips one QI); AllocAsmMeta / FreeAsmMeta eliminated - MetadataReader.cs net472 path migrated from [ComImport] to struct-based COM via AgileComPointer; failure to OpenScope returns null (per-file contract) - RegisterAssembly.cs uses the new ICreateTypeLib struct - Microsoft.Build.Tasks.csproj wildcard <Compile Include> for the Fusion / Metadata / TypeLib folders (FeatureWindowsInterop-gated) - NativeMethods.cs legacy [ComImport] declarations removed; only oleaut32 typelib P/Invokes remain; System.Runtime.Versioning using gated on FEATURE_WINDOWSINTEROP for source-build Skills - .github/skills/cswin32-com/SKILL.md adds 'Strongly-typed handle / token wrappers' with CLR validation table; pair-pointer to cswin32-interop; error- handling-parity guidance; IComIID polyfill heading rename - .github/skills/cswin32-interop/SKILL.md reorganized: Rules / Blittable signatures / Infrastructure / BufferScope<T> / Gotchas; source-build verification note
… hand-rolled interop Continues the CsWin32 struct-based interop migration from dotnet#13746 and dotnet#13853. Removes the legacy Microsoft.VisualStudio.Setup.Configuration.Interop RCW package outright and migrates the directory-enumeration / FileTracker / VS locator / test-fixture call sites to PInvoke + ComScope<T>. New manual COM struct definitions src/Framework/Shared/VisualStudio/ SetupConfiguration.cs CLSID + GetSetupConfiguration fallback DllImport ISetupConfiguration.cs v1 (IUnknown surface only, for QI) ISetupConfiguration2.cs EnumAllInstances (vtable slot 6) IEnumSetupInstances.cs Next (slot 3) ISetupInstance.cs GetInstallationPath/Version/DisplayName (slots 6/7/8) ISetupInstance2.cs GetState (slot 11) InstanceState.cs [Flags] enum InstanceState : uint, Complete = MAXUINT Rewrites src/Framework/VisualStudioLocationHelper.cs Replaces the RCW-based GetInstances() with PInvoke.CoCreateInstance + REGDB_E_CLASSNOTREG fallback to GetSetupConfiguration P/Invoke + QI. Pointer lifetimes managed entirely via ComScope<T>; BSTR out-params via `using BSTR x = default;`. Helper returns `default` ComScope on COM failure rather than throwing a COMException the caller would discard. src/Framework/FileSystem/WindowsNative.cs src/Framework/FileSystem/SafeFileHandle.cs FindFirstFileW / FindNextFileW / FindClose / PathMatchSpecExW migrated to PInvoke.FindFirstFile / FindNextFile / FindClose / PathMatchSpecEx (added to NativeMethods.txt). The legacy hand-shaped Win32FindData (with [MarshalAs(UnmanagedType.ByValTStr)] string fields) is preserved as a caller-facing adapter built from the blittable WIN32_FIND_DATAW. SafeFindFileHandle now wraps a manually-constructed HANDLE and its ReleaseHandle calls PInvoke.FindClose. src/Shared/InprocTrackingNativeMethods.cs FileTracker.dll loader rewritten to use PInvoke.LoadLibrary + PInvoke.GetProcAddress with typed delegate* unmanaged[Stdcall]<...> function pointers matching the actual export signatures from FileTracker.h / FileTracker.def (StartTrackingContext @2 ... SetThreadCount @10). The SafeLibraryHandle subclass and all [UnmanagedFunctionPointer] managed delegate types are deleted; the public API surface gets full XML doc comments. The HMODULE is intentionally never freed (FileTracker detours ExitProcess; unloading mid-shutdown would corrupt the CLR). ANSI export-name encoding uses Encoding.Default + a pooled byte[] (correct for DBCS code pages). Test fixtures migrated to CsWin32 src/UnitTests.Shared/DriveMapping.cs DefineDosDevice / QueryDosDevice -> PInvoke.* with DEFINE_DOS_DEVICE_FLAGS. src/Tasks.UnitTests/AddToWin32Manifest_Tests.cs LoadLibrary / FindResource / LoadResource / LockResource / SizeofResource -> PInvoke.LoadLibraryEx / FindResource / LoadResource / LockResource / SizeofResource consuming HMODULE / HRSRC / HGLOBAL. src/Build.UnitTests/BackEnd/TargetUpToDateChecker_Tests.cs Hand-rolled CreateSymbolicLink / CreateFileForSymlink / SetFileTime replaced with wrappers over PInvoke.* (CreateSymbolicLink already in NativeMethods.txt; SetFileTime added). Deletion src/Build/BackEnd/Node/NativeMethods.cs (~230 lines) The CreateProcess + STARTUP_INFO + PROCESS_INFORMATION + SECURITY_ATTRIBUTES wrapper was consumed by exactly one place -- FileTrackerTests' [Fact(Skip=...)] helper methods. That helper now defines a small private BackEndNativeMethods static class inline, forwarding to PInvoke.CreateProcess and bridging the test-facing struct shapes to Win32 STARTUPINFOW / PROCESS_INFORMATION / SECURITY_ATTRIBUTES at the call. Package reference removed (now unused everywhere) - src/Framework/Microsoft.Build.Framework.csproj - src/Build/Microsoft.Build.csproj - src/Utilities/Microsoft.Build.Utilities.csproj - src/Tasks/Microsoft.Build.Tasks.csproj - src/Framework.UnitTests/Microsoft.Build.Framework.UnitTests.csproj PR dotnet#13853 review feedback (carried in) src/Tasks/ManifestUtil/MetadataReader.cs src/Tasks/ManifestUtil/ApplicationManifest.cs Adds MetadataReader.HasAssemblyAttributes(string[], bool[]) batch overload on both .NETCore and net472 paths; the net472 path acquires IMetaDataAssemblyImport / IMetaDataImport2 from the GIT once and reuses the same GetAssemblyFromScope across all probes. AssemblyAttributeFlags in ApplicationManifest.cs now does one GIT round-trip for all four attribute checks instead of eight (Jan's review comment). src/Tasks.UnitTests/AssemblyInformation_Tests.cs (new) Two real-file tests exercising the full CLR-metadata COM call chain end-to-end on net472 (CoCreateInstance -> OpenScope -> QI -> GetAssemblyProps -> GetCustomAttributeByName x4 -> GetPEKind), with parity assertions against AssemblyName.GetAssemblyName / TargetFrameworkAttribute (Viktor's review comment). Other new tests src/Tasks.UnitTests/MetadataReader_Tests.cs HasAssemblyAttributes_Batch_AgreesWithSingular: cross-validates the new batch overload against the singular overload for both known-true and known-false attributes. src/Framework.UnitTests/FileSystem/WindowsNative_Tests.cs (new) Six tests directly covering the FindFirstFile / FindNextFile / PathMatchSpecEx migration + the Win32FindData adapter + SafeFindFileHandle idempotent-dispose, with WindowsOnlyFact gating. src/Framework.UnitTests/VisualStudioLocationHelper_Tests.cs (new) Six baseline tests asserting the contract that GetInstances() never throws, never returns null, that each returned VisualStudioInstance has valid Name/Path/Version (Major >= 15) shape, paths are unique, and existing paths are absolute directories. Establishes the regression net before the RCW -> struct-based COM migration. NativeMethods.txt additions DefineDosDevice, QueryDosDevice, FindClose, FindFirstFile, FindNextFile, FindResource, LoadResource, LockResource, SizeofResource, LoadLibraryEx, LOAD_LIBRARY_FLAGS, PathMatchSpecEx, SetFileTime, PCSTR. Skill updates .github/skills/cswin32-com/SKILL.md Compressed from ~5,400 -> ~3,520 estimated tokens (-35%). Adds: - "Lifetime: using ComScope<T>" promoted to workflow step 2; raw try/finally Release pattern explicitly banned. - Helper "ownership-transfer" pattern (return ComScope<T> from a helper) with the AcquireSetupConfiguration2 worked example. - BSTR scoping bullet -- `using BSTR x = default;` replaces manual try/finally + SysFreeString. - "Don't throw when the caller swallows it" subsection under Error-Handling Parity, citing AcquireSetupConfiguration2. Verified - Full build: dotnet build MSBuild.Dev.slnf -> 0 warnings, 0 errors. - Tests: Framework.UnitTests 750/750 net472, 721/721 net10.0 Tasks.UnitTests new MetadataReader/AssemblyInformation + AddToWin32Manifest tests pass on both TFMs. - Source build (DotNetBuildSourceOnly=true) unchanged: every new interop file is gated #if FEATURE_WINDOWSINTEROP (and && FEATURE_VISUALSTUDIOSETUP for the VS Setup interfaces).
… hand-rolled interop Continues the CsWin32 struct-based interop migration from dotnet#13746 and dotnet#13853. Removes the legacy Microsoft.VisualStudio.Setup.Configuration.Interop RCW package outright and migrates the directory-enumeration / FileTracker / VS locator / test-fixture call sites to PInvoke + ComScope<T>. New manual COM struct definitions src/Framework/Shared/VisualStudio/ SetupConfiguration.cs CLSID + GetSetupConfiguration fallback DllImport ISetupConfiguration.cs v1 (IUnknown surface only, for QI) ISetupConfiguration2.cs EnumAllInstances (vtable slot 6) IEnumSetupInstances.cs Next (slot 3) ISetupInstance.cs GetInstallationPath/Version/DisplayName (slots 6/7/8) ISetupInstance2.cs GetState (slot 11) InstanceState.cs [Flags] enum InstanceState : uint, Complete = MAXUINT Rewrites src/Framework/VisualStudioLocationHelper.cs Replaces the RCW-based GetInstances() with PInvoke.CoCreateInstance + REGDB_E_CLASSNOTREG fallback to GetSetupConfiguration P/Invoke + QI. Pointer lifetimes managed entirely via ComScope<T>; BSTR out-params via `using BSTR x = default;`. Helper returns `default` ComScope on COM failure rather than throwing a COMException the caller would discard. src/Framework/FileSystem/WindowsNative.cs src/Framework/FileSystem/SafeFileHandle.cs FindFirstFileW / FindNextFileW / FindClose / PathMatchSpecExW migrated to PInvoke.FindFirstFile / FindNextFile / FindClose / PathMatchSpecEx (added to NativeMethods.txt). The legacy hand-shaped Win32FindData (with [MarshalAs(UnmanagedType.ByValTStr)] string fields) is preserved as a caller-facing adapter built from the blittable WIN32_FIND_DATAW. SafeFindFileHandle now wraps a manually-constructed HANDLE and its ReleaseHandle calls PInvoke.FindClose. src/Shared/InprocTrackingNativeMethods.cs FileTracker.dll loader rewritten to use PInvoke.LoadLibrary + PInvoke.GetProcAddress with typed delegate* unmanaged[Stdcall]<...> function pointers matching the actual export signatures from FileTracker.h / FileTracker.def (StartTrackingContext @2 ... SetThreadCount @10). The SafeLibraryHandle subclass and all [UnmanagedFunctionPointer] managed delegate types are deleted; the public API surface gets full XML doc comments. The HMODULE is intentionally never freed (FileTracker detours ExitProcess; unloading mid-shutdown would corrupt the CLR). ANSI export-name encoding uses Encoding.Default + a pooled byte[] (correct for DBCS code pages). Test fixtures migrated to CsWin32 src/UnitTests.Shared/DriveMapping.cs DefineDosDevice / QueryDosDevice -> PInvoke.* with DEFINE_DOS_DEVICE_FLAGS. src/Tasks.UnitTests/AddToWin32Manifest_Tests.cs LoadLibrary / FindResource / LoadResource / LockResource / SizeofResource -> PInvoke.LoadLibraryEx / FindResource / LoadResource / LockResource / SizeofResource consuming HMODULE / HRSRC / HGLOBAL. src/Build.UnitTests/BackEnd/TargetUpToDateChecker_Tests.cs Hand-rolled CreateSymbolicLink / CreateFileForSymlink / SetFileTime replaced with wrappers over PInvoke.* (CreateSymbolicLink already in NativeMethods.txt; SetFileTime added). Deletion src/Build/BackEnd/Node/NativeMethods.cs (~230 lines) The CreateProcess + STARTUP_INFO + PROCESS_INFORMATION + SECURITY_ATTRIBUTES wrapper was consumed by exactly one place -- FileTrackerTests' [Fact(Skip=...)] helper methods. That helper now defines a small private BackEndNativeMethods static class inline, forwarding to PInvoke.CreateProcess and bridging the test-facing struct shapes to Win32 STARTUPINFOW / PROCESS_INFORMATION / SECURITY_ATTRIBUTES at the call. Package reference removed (now unused everywhere) - src/Framework/Microsoft.Build.Framework.csproj - src/Build/Microsoft.Build.csproj - src/Utilities/Microsoft.Build.Utilities.csproj - src/Tasks/Microsoft.Build.Tasks.csproj - src/Framework.UnitTests/Microsoft.Build.Framework.UnitTests.csproj PR dotnet#13853 review feedback (carried in) src/Tasks/ManifestUtil/MetadataReader.cs src/Tasks/ManifestUtil/ApplicationManifest.cs Adds MetadataReader.HasAssemblyAttributes(string[], bool[]) batch overload on both .NETCore and net472 paths; the net472 path acquires IMetaDataAssemblyImport / IMetaDataImport2 from the GIT once and reuses the same GetAssemblyFromScope across all probes. AssemblyAttributeFlags in ApplicationManifest.cs now does one GIT round-trip for all four attribute checks instead of eight (Jan's review comment). src/Tasks.UnitTests/AssemblyInformation_Tests.cs (new) Two real-file tests exercising the full CLR-metadata COM call chain end-to-end on net472 (CoCreateInstance -> OpenScope -> QI -> GetAssemblyProps -> GetCustomAttributeByName x4 -> GetPEKind), with parity assertions against AssemblyName.GetAssemblyName / TargetFrameworkAttribute (Viktor's review comment). Other new tests src/Tasks.UnitTests/MetadataReader_Tests.cs HasAssemblyAttributes_Batch_AgreesWithSingular: cross-validates the new batch overload against the singular overload for both known-true and known-false attributes. src/Framework.UnitTests/FileSystem/WindowsNative_Tests.cs (new) Six tests directly covering the FindFirstFile / FindNextFile / PathMatchSpecEx migration + the Win32FindData adapter + SafeFindFileHandle idempotent-dispose, with WindowsOnlyFact gating. src/Framework.UnitTests/VisualStudioLocationHelper_Tests.cs (new) Six baseline tests asserting the contract that GetInstances() never throws, never returns null, that each returned VisualStudioInstance has valid Name/Path/Version (Major >= 15) shape, paths are unique, and existing paths are absolute directories. Establishes the regression net before the RCW -> struct-based COM migration. NativeMethods.txt additions DefineDosDevice, QueryDosDevice, FindClose, FindFirstFile, FindNextFile, FindResource, LoadResource, LockResource, SizeofResource, LoadLibraryEx, LOAD_LIBRARY_FLAGS, PathMatchSpecEx, SetFileTime, PCSTR. Skill updates .github/skills/cswin32-com/SKILL.md Compressed from ~5,400 -> ~3,520 estimated tokens (-35%). Adds: - "Lifetime: using ComScope<T>" promoted to workflow step 2; raw try/finally Release pattern explicitly banned. - Helper "ownership-transfer" pattern (return ComScope<T> from a helper) with the AcquireSetupConfiguration2 worked example. - BSTR scoping bullet -- `using BSTR x = default;` replaces manual try/finally + SysFreeString. - "Don't throw when the caller swallows it" subsection under Error-Handling Parity, citing AcquireSetupConfiguration2. Verified - Full build: dotnet build MSBuild.Dev.slnf -> 0 warnings, 0 errors. - Tests: Framework.UnitTests 750/750 net472, 721/721 net10.0 Tasks.UnitTests new MetadataReader/AssemblyInformation + AddToWin32Manifest tests pass on both TFMs. - Source build (DotNetBuildSourceOnly=true) unchanged: every new interop file is gated #if FEATURE_WINDOWSINTEROP (and && FEATURE_VISUALSTUDIOSETUP for the VS Setup interfaces).
… hand-rolled interop (#13872) Continues the CsWin32 struct-based interop migration from #13746 and #13853. Removes the legacy `Microsoft.VisualStudio.Setup.Configuration.Interop` RCW package outright and migrates the directory-enumeration / FileTracker / VS locator / test-fixture call sites to `PInvoke` + `ComScope<T>`. ## New manual COM struct definitions Under `src/Framework/Shared/VisualStudio/`: | File | Contents | | --- | --- | | `SetupConfiguration.cs` | `CLSID_SetupConfiguration` + `GetSetupConfiguration` fallback `[DllImport]` | | `ISetupConfiguration.cs` | v1 — `IUnknown` surface only, for QI | | `ISetupConfiguration2.cs` | `EnumAllInstances` (vtable slot 6) | | `IEnumSetupInstances.cs` | `Next` (slot 3) | | `ISetupInstance.cs` | `GetInstallationPath` / `GetInstallationVersion` / `GetDisplayName` (slots 6/7/8) | | `ISetupInstance2.cs` | `GetState` (slot 11) | | `InstanceState.cs` | `[Flags] enum InstanceState : uint`, `Complete = MAXUINT` | ## Rewrites ### `src/Framework/VisualStudioLocationHelper.cs` Replaces the RCW-based `GetInstances()` with `PInvoke.CoCreateInstance` + `REGDB_E_CLASSNOTREG` fallback to the `GetSetupConfiguration` P/Invoke + QI. Pointer lifetimes managed entirely via `ComScope<T>`; `BSTR` out-params via `using BSTR x = default;`. The helper returns a `default` `ComScope` on COM failure rather than throwing a `COMException` the caller would immediately discard. ### `src/Framework/FileSystem/WindowsNative.cs`, `src/Framework/FileSystem/SafeFileHandle.cs` `FindFirstFileW` / `FindNextFileW` / `FindClose` / `PathMatchSpecExW` migrated to `PInvoke.FindFirstFile` / `FindNextFile` / `FindClose` / `PathMatchSpecEx` (added to `NativeMethods.txt`). The legacy hand-shaped `Win32FindData` (with `[MarshalAs(UnmanagedType.ByValTStr)]` string fields) is preserved as a caller-facing adapter built from the blittable `WIN32_FIND_DATAW`. `SafeFindFileHandle` now wraps a manually-constructed `HANDLE` and its `ReleaseHandle` calls `PInvoke.FindClose`. ### `src/Shared/InprocTrackingNativeMethods.cs` FileTracker.dll loader rewritten to use `PInvoke.LoadLibrary` + `PInvoke.GetProcAddress` with typed `delegate* unmanaged[Stdcall]<…>` function pointers matching the actual export signatures from `FileTracker.h` / `FileTracker.def` (`StartTrackingContext` `@2` through `SetThreadCount` `@10`). The `SafeLibraryHandle` subclass and all `[UnmanagedFunctionPointer]` managed delegate types are deleted; the public API surface gets full XML doc comments. The `HMODULE` is intentionally never freed — FileTracker detours `ExitProcess`, so unloading mid-shutdown would corrupt the CLR. ANSI export-name encoding uses `Encoding.Default` + a pooled `byte[]` (correct for DBCS code pages). ## Test fixtures migrated to CsWin32 - `src/UnitTests.Shared/DriveMapping.cs` — `DefineDosDevice` / `QueryDosDevice` → `PInvoke.*` with `DEFINE_DOS_DEVICE_FLAGS`. - `src/Tasks.UnitTests/AddToWin32Manifest_Tests.cs` — `LoadLibrary` / `FindResource` / `LoadResource` / `LockResource` / `SizeofResource` → `PInvoke.LoadLibraryEx` / `FindResource` / `LoadResource` / `LockResource` / `SizeofResource` consuming `HMODULE` / `HRSRC` / `HGLOBAL`. - `src/Build.UnitTests/BackEnd/TargetUpToDateChecker_Tests.cs` — hand-rolled `CreateSymbolicLink` / `CreateFileForSymlink` / `SetFileTime` replaced with wrappers over `PInvoke.*` (`CreateSymbolicLink` already in `NativeMethods.txt`; `SetFileTime` added). ## Deletion - `src/Build/BackEnd/Node/NativeMethods.cs` (~230 lines) — the `CreateProcess` + `STARTUP_INFO` + `PROCESS_INFORMATION` + `SECURITY_ATTRIBUTES` wrapper was consumed by exactly one place: `FileTrackerTests`' `[Fact(Skip=...)]` helper methods. That helper now defines a small private `BackEndNativeMethods` static class inline, forwarding to `PInvoke.CreateProcess` and bridging the test-facing struct shapes to Win32 `STARTUPINFOW` / `PROCESS_INFORMATION` / `SECURITY_ATTRIBUTES` at the call. ## Package reference removed (now unused everywhere) - `src/Framework/Microsoft.Build.Framework.csproj` - `src/Build/Microsoft.Build.csproj` - `src/Utilities/Microsoft.Build.Utilities.csproj` - `src/Tasks/Microsoft.Build.Tasks.csproj` - `src/Framework.UnitTests/Microsoft.Build.Framework.UnitTests.csproj` ## PR #13853 review feedback (carried in) ### `src/Tasks/ManifestUtil/MetadataReader.cs`, `src/Tasks/ManifestUtil/ApplicationManifest.cs` Adds `MetadataReader.HasAssemblyAttributes(string[], bool[])` batch overload on both .NETCore and net472 paths; the net472 path acquires `IMetaDataAssemblyImport` / `IMetaDataImport2` from the GIT once and reuses the same `GetAssemblyFromScope` across all probes. `AssemblyAttributeFlags` in `ApplicationManifest.cs` now does one GIT round-trip for all four attribute checks instead of eight (@JanProvaznik's review comment). ### `src/Tasks.UnitTests/AssemblyInformation_Tests.cs` (new) Two real-file tests exercising the full CLR-metadata COM call chain end-to-end on net472 (`CoCreateInstance` → `OpenScope` → QI → `GetAssemblyProps` → `GetCustomAttributeByName` ×4 → `GetPEKind`), with parity assertions against `AssemblyName.GetAssemblyName` / `TargetFrameworkAttribute` (@ViktorHofer's review comment). ## Other new tests - `src/Tasks.UnitTests/MetadataReader_Tests.cs` — `HasAssemblyAttributes_Batch_AgreesWithSingular`: cross-validates the new batch overload against the singular overload for both known-true and known-false attributes. - `src/Framework.UnitTests/FileSystem/WindowsNative_Tests.cs` (new) — six tests directly covering the `FindFirstFile` / `FindNextFile` / `PathMatchSpecEx` migration + the `Win32FindData` adapter + `SafeFindFileHandle` idempotent-dispose, with `WindowsOnlyFact` gating. - `src/Framework.UnitTests/VisualStudioLocationHelper_Tests.cs` (new) — six baseline tests asserting the contract that `GetInstances()` never throws, never returns null, that each returned `VisualStudioInstance` has valid `Name`/`Path`/`Version` (Major ≥ 15) shape, paths are unique, and existing paths are absolute directories. Establishes the regression net before the RCW → struct-based COM migration. ## `NativeMethods.txt` additions `DefineDosDevice`, `QueryDosDevice`, `FindClose`, `FindFirstFile`, `FindNextFile`, `FindResource`, `LoadResource`, `LockResource`, `SizeofResource`, `LoadLibraryEx`, `LOAD_LIBRARY_FLAGS`, `PathMatchSpecEx`, `SetFileTime`, `PCSTR`. ## Skill updates `.github/skills/cswin32-com/SKILL.md` compressed from ~5,400 → ~3,520 estimated tokens (−35%). Adds: - "Lifetime: `using ComScope<T>`" promoted to workflow step 2; raw `try`/`finally` `Release` pattern explicitly banned. - Helper "ownership-transfer" pattern (return `ComScope<T>` from a helper) with the `AcquireSetupConfiguration2` worked example. - `BSTR` scoping bullet — `using BSTR x = default;` replaces manual `try`/`finally` + `SysFreeString`. - "Don't throw when the caller swallows it" subsection under Error-Handling Parity, citing `AcquireSetupConfiguration2`. ## Verified - **Full build**: `dotnet build MSBuild.Dev.slnf` → 0 warnings, 0 errors. - **Tests**: - `Framework.UnitTests` — 750/750 net472, 721/721 net10.0. - `Tasks.UnitTests` — new `MetadataReader` / `AssemblyInformation` + `AddToWin32Manifest` tests pass on both TFMs. - **Source build** (`DotNetBuildSourceOnly=true`) unchanged: every new interop file is gated `#if FEATURE_WINDOWSINTEROP` (and `&& FEATURE_VISUALSTUDIOSETUP` for the VS Setup interfaces).
Follow-up to the earlier CsWin32 migration. Converts the Fusion (GAC)
[ComImport]interfaces andfusion.dll[DllImport]s to struct-based COM, and adds the missingAgileComPointer<T>/GlobalInterfaceTableinfrastructure to safely hold COM pointers in managed class fields.What changed
Fusion → struct-based COM
The three Fusion interfaces (
IAssemblyCache,IAssemblyName,IAssemblyEnum) and fourfusion.dllP/Invokes (CreateAssemblyCache,CreateAssemblyEnum,CreateAssemblyNameObject,GetCachePath) move from[ComImport]/[DllImport]with managed-marshalled signatures into manual CsWin32-style struct-based COM undersrc/Tasks/AssemblyDependency/Fusion/. Signatures matchCLR\src\inc\fusion.idl.Highlights:
HRESULTreturn type,T**(notout T*),void*(notIntPtr),PCWSTR/PWSTRfrom CsWin32 (added toNativeMethods.txt), no managed reference types in vtables.IComIIDwith dual#if NET(static-abstract) /#else(instance) impls so they work viaComScope<T>on both net472 and net10.RetrievePathFromFusionName,GetGacPath,AssemblyCacheEnum) updated to useusing ComScope<T>blocks throughout. The_assemblyEnumIsNullworkaround for iterator+unsafe-pointer locals is gone.AgileComPointer / GlobalInterfaceTable infrastructure
Adapted from
dotnet/winforms(System.Private.Windows.Core). Two new files insrc/Framework/Windows/Win32/System/Com/:GlobalInterfaceTable.cs— wrapsIGlobalInterfaceTable(CLSIDStdGlobalInterfaceTable). AddedIGlobalInterfaceTabletoNativeMethods.txtplus the IComIID polyfill and CLS-compliance partials.AgileComPointer<T>— finalizable managed wrapper for COM pointers stored in class fields. ProvidesGetInterface()returning aComScope<T>;Dispose()revokes the GIT cookie.Interlocked.Exchangeadapted viaUnsafe.As<uint,int>because net472 lacks theuintoverload.AssemblyCacheEnum._assemblyEnumis now anAgileComPointer<IAssemblyEnum>field instead of a rawIAssemblyEnum*.Smaller cleanups
ResourceUpdaterERROR_SHARING_VIOLATIONmagic number →(int)(HRESULT)WIN32_ERROR.ERROR_SHARING_VIOLATION.[ComImport] IClassFactoryfromTasks/NativeMethods.cs(already provided by CsWin32 /Windows.Win32.System.Com.IClassFactory).crypt32/advapi32declarations + various dead constants fromTasks/NativeMethods.cs(already gone earlier in the branch; relisted for completeness).Tests
New
src/Framework.UnitTests/AgileComPointerTests.cs— 8 tests coveringGlobalInterfaceTableregister/get/revoke,AgileComPointerownership modes,Disposesemantics including idempotency, and cross-cast QI toIUnknown. All 8 pass on bothnet10.0andnet472, exercising both the CsWin32 static-abstractIComIIDpath and the net472 polyfill.Documentation
.github/skills/cswin32-com/SKILL.mdupdated with: HRESULT-vs-int,T**vsout T*,void*vsIntPtr,nint/nuintpreference,PCWSTR/PWSTRvschar*, no managed types in vtables, thePreserveSigdefault difference between[DllImport](true) and[ComImport](false), the dual-target manual struct pattern, and theComScope<T>(locals) vsAgileComPointer<T>(fields) distinction including thetakeOwnership: falsepairing rule..github/skills/cswin32-interop/SKILL.mdgained the same pointer/nintconventions section for[DllImport]signatures.Verification
dotnet msbuild MSBuild.Dev.slnf -v:m— 0 warnings, 0 errors.AgileComPointerTests— 8/8 pass on net10.0; 8/8 pass on net472.FeatureWindowsInterop != true) via existing<Compile Remove>mechanism.