support for adding activatable class entries based off a winmd file#334
support for adding activatable class entries based off a winmd file#334
Conversation
Build Metrics ReportBinary Sizes
Test Results✅ 312 passed, 6 skipped out of 318 tests in 97.3s (+13 tests, +38.8s vs. baseline) CLI Startup Time33ms median (x64, Updated 2026-02-28 01:17:34 UTC · commit |
…/winappcli into nm/activatable-classes
There was a problem hiding this comment.
Pull request overview
Adds automatic WinRT component discovery during packaging by scanning NuGet-delivered .winmd metadata, extracting activatable classes, and emitting the corresponding manifest registrations for both packaged and self-contained deployment scenarios.
Changes:
- Introduces
IWinmdService/WinmdServiceto parse.winmdfiles and discover WinRT components from NuGet packages. - Updates
MsixServicepackaging flow to append third-party activatable class registrations to AppxManifest (framework-dependent) or embedded SxS manifests (self-contained). - Adds tests, sample updates (WPF + Win2D), and documentation describing the new behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/winapp-CLI/WinApp.Cli/Services/WinmdService.cs | New service for reading .winmd metadata and discovering WinRT components in NuGet packages. |
| src/winapp-CLI/WinApp.Cli/Services/IWinmdService.cs | Adds the WinMD service contract and WinRTComponent record. |
| src/winapp-CLI/WinApp.Cli/Services/MsixService.cs | Integrates WinRT discovery/registration into packaging and self-contained manifest embedding. |
| src/winapp-CLI/WinApp.Cli/Helpers/HostBuilderExtensions.cs | Registers IWinmdService in DI. |
| src/winapp-CLI/WinApp.Cli.Tests/WinmdServiceTests.cs | Unit tests for .winmd parsing and component discovery. |
| src/winapp-CLI/WinApp.Cli.Tests/PackageCommandTests.cs | Integration tests validating generated <InProcessServer> entries for Win2D/WebView2 and output naming change. |
| scripts/msix-assets/install-msix.ps1 | Adjusts MSIX lookup to be relative to the script directory (more reliable elevation flow). |
| samples/wpf-app/wpf-app.csproj | Adds Win2D package reference to the WPF sample. |
| samples/wpf-app/README.md | Documents Win2D usage/activation expectations in the sample. |
| samples/wpf-app/MainWindow.xaml.cs | Validates Win2D activation at runtime in the sample app UI. |
| samples/wpf-app/MainWindow.xaml | Adds UI element for displaying Win2D activation status. |
| docs/usage.md | Documents WinRT component discovery behavior during winapp pack. |
Comments suppressed due to low confidence (1)
scripts/msix-assets/install-msix.ps1:116
- The construction of the
$argumentsstring for the elevatedPowerShellprocess embeds$scriptDir,$PSCommandPath, and-PackagePath '$PackagePath'directly inside a-Commandstring using single quotes, without any escaping. Because-PackagePathcomes from untrusted CLI input, an attacker can supply a path that includes a single quote and additional PowerShell code (for example, a file namedgood'; Start-Process calc; '.msix), which will break out of the quoted string and inject arbitrary commands into the elevated session. To mitigate this, avoid manual string concatenation for-Command(e.g., use-Filewith separate-ArgumentListor another structured invocation) or ensure all embedded paths are properly escaped/encoded before being inserted into the command string so they cannot terminate quotes or introduce new statements.
$arguments = "-NoProfile -ExecutionPolicy Bypass -Command `"Set-Location '$scriptDir'; & '$PSCommandPath' -Elevated"
if (-not [string]::IsNullOrEmpty($PackagePath)) {
# Convert to absolute path before passing
$PackagePath = Resolve-Path $PackagePath -ErrorAction SilentlyContinue
if ($PackagePath) {
$arguments += " -PackagePath '$PackagePath'"
}
| using var stream = File.OpenRead(winmdPath.FullName); | ||
| using var peReader = new PEReader(stream); | ||
|
|
||
| if (!peReader.HasMetadata) | ||
| { | ||
| return []; | ||
| } | ||
|
|
||
| var reader = peReader.GetMetadataReader(); | ||
|
|
||
| foreach (var typeDefHandle in reader.TypeDefinitions) | ||
| { | ||
| var typeDef = reader.GetTypeDefinition(typeDefHandle); | ||
|
|
||
| // Skip non-public types | ||
| var visibility = typeDef.Attributes & System.Reflection.TypeAttributes.VisibilityMask; | ||
| if (visibility != System.Reflection.TypeAttributes.Public) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| // Skip nested types (they're activated through their parent) | ||
| if (typeDef.IsNested) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| // Skip interfaces (Abstract + ClassSemanticsMask == Interface) | ||
| if ((typeDef.Attributes & System.Reflection.TypeAttributes.ClassSemanticsMask) == System.Reflection.TypeAttributes.Interface) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| // Skip value types (structs/enums) — not activatable | ||
| var baseTypeHandle = typeDef.BaseType; | ||
| if (!baseTypeHandle.IsNil) | ||
| { | ||
| var baseTypeName = GetFullTypeName(reader, baseTypeHandle); | ||
| if (baseTypeName is "System.ValueType" or "System.Enum" or "System.MulticastDelegate") | ||
| { | ||
| continue; | ||
| } | ||
| } | ||
|
|
||
| // Skip types with no public constructors and no static/activatable attributes | ||
| // In WinRT, a class without activation factories is typically a static class | ||
| // but we still include it — the runtime needs entries for static classes with | ||
| // factory interfaces too. Only skip if it looks like a pure attribute. | ||
| var baseType = !baseTypeHandle.IsNil ? GetFullTypeName(reader, baseTypeHandle) : null; | ||
| if (baseType == "System.Attribute") | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| var namespaceName = reader.GetString(typeDef.Namespace); | ||
| var typeName = reader.GetString(typeDef.Name); | ||
|
|
||
| // Skip the synthetic <Module> type | ||
| if (string.IsNullOrEmpty(namespaceName) && typeName == "<Module>") | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| // Skip types in implementation-detail namespaces | ||
| if (string.IsNullOrEmpty(namespaceName)) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| var fullName = $"{namespaceName}.{typeName}"; | ||
| classes.Add(fullName); | ||
| } |
There was a problem hiding this comment.
GetActivatableClasses can throw (e.g., BadImageFormatException/IOException) if a .winmd is corrupt or not a valid PE. Since this is driven by third-party NuGet contents, an exception here would fail packaging; consider catching these exceptions and returning an empty list (or a diagnostic) instead of propagating.
| using var stream = File.OpenRead(winmdPath.FullName); | |
| using var peReader = new PEReader(stream); | |
| if (!peReader.HasMetadata) | |
| { | |
| return []; | |
| } | |
| var reader = peReader.GetMetadataReader(); | |
| foreach (var typeDefHandle in reader.TypeDefinitions) | |
| { | |
| var typeDef = reader.GetTypeDefinition(typeDefHandle); | |
| // Skip non-public types | |
| var visibility = typeDef.Attributes & System.Reflection.TypeAttributes.VisibilityMask; | |
| if (visibility != System.Reflection.TypeAttributes.Public) | |
| { | |
| continue; | |
| } | |
| // Skip nested types (they're activated through their parent) | |
| if (typeDef.IsNested) | |
| { | |
| continue; | |
| } | |
| // Skip interfaces (Abstract + ClassSemanticsMask == Interface) | |
| if ((typeDef.Attributes & System.Reflection.TypeAttributes.ClassSemanticsMask) == System.Reflection.TypeAttributes.Interface) | |
| { | |
| continue; | |
| } | |
| // Skip value types (structs/enums) — not activatable | |
| var baseTypeHandle = typeDef.BaseType; | |
| if (!baseTypeHandle.IsNil) | |
| { | |
| var baseTypeName = GetFullTypeName(reader, baseTypeHandle); | |
| if (baseTypeName is "System.ValueType" or "System.Enum" or "System.MulticastDelegate") | |
| { | |
| continue; | |
| } | |
| } | |
| // Skip types with no public constructors and no static/activatable attributes | |
| // In WinRT, a class without activation factories is typically a static class | |
| // but we still include it — the runtime needs entries for static classes with | |
| // factory interfaces too. Only skip if it looks like a pure attribute. | |
| var baseType = !baseTypeHandle.IsNil ? GetFullTypeName(reader, baseTypeHandle) : null; | |
| if (baseType == "System.Attribute") | |
| { | |
| continue; | |
| } | |
| var namespaceName = reader.GetString(typeDef.Namespace); | |
| var typeName = reader.GetString(typeDef.Name); | |
| // Skip the synthetic <Module> type | |
| if (string.IsNullOrEmpty(namespaceName) && typeName == "<Module>") | |
| { | |
| continue; | |
| } | |
| // Skip types in implementation-detail namespaces | |
| if (string.IsNullOrEmpty(namespaceName)) | |
| { | |
| continue; | |
| } | |
| var fullName = $"{namespaceName}.{typeName}"; | |
| classes.Add(fullName); | |
| } | |
| try | |
| { | |
| using var stream = File.OpenRead(winmdPath.FullName); | |
| using var peReader = new PEReader(stream); | |
| if (!peReader.HasMetadata) | |
| { | |
| return []; | |
| } | |
| var reader = peReader.GetMetadataReader(); | |
| foreach (var typeDefHandle in reader.TypeDefinitions) | |
| { | |
| var typeDef = reader.GetTypeDefinition(typeDefHandle); | |
| // Skip non-public types | |
| var visibility = typeDef.Attributes & System.Reflection.TypeAttributes.VisibilityMask; | |
| if (visibility != System.Reflection.TypeAttributes.Public) | |
| { | |
| continue; | |
| } | |
| // Skip nested types (they're activated through their parent) | |
| if (typeDef.IsNested) | |
| { | |
| continue; | |
| } | |
| // Skip interfaces (Abstract + ClassSemanticsMask == Interface) | |
| if ((typeDef.Attributes & System.Reflection.TypeAttributes.ClassSemanticsMask) == System.Reflection.TypeAttributes.Interface) | |
| { | |
| continue; | |
| } | |
| // Skip value types (structs/enums) — not activatable | |
| var baseTypeHandle = typeDef.BaseType; | |
| if (!baseTypeHandle.IsNil) | |
| { | |
| var baseTypeName = GetFullTypeName(reader, baseTypeHandle); | |
| if (baseTypeName is "System.ValueType" or "System.Enum" or "System.MulticastDelegate") | |
| { | |
| continue; | |
| } | |
| } | |
| // Skip types with no public constructors and no static/activatable attributes | |
| // In WinRT, a class without activation factories is typically a static class | |
| // but we still include it — the runtime needs entries for static classes with | |
| // factory interfaces too. Only skip if it looks like a pure attribute. | |
| var baseType = !baseTypeHandle.IsNil ? GetFullTypeName(reader, baseTypeHandle) : null; | |
| if (baseType == "System.Attribute") | |
| { | |
| continue; | |
| } | |
| var namespaceName = reader.GetString(typeDef.Namespace); | |
| var typeName = reader.GetString(typeDef.Name); | |
| // Skip the synthetic <Module> type | |
| if (string.IsNullOrEmpty(namespaceName) && typeName == "<Module>") | |
| { | |
| continue; | |
| } | |
| // Skip types in implementation-detail namespaces | |
| if (string.IsNullOrEmpty(namespaceName)) | |
| { | |
| continue; | |
| } | |
| var fullName = $"{namespaceName}.{typeName}"; | |
| classes.Add(fullName); | |
| } | |
| } | |
| catch (System.BadImageFormatException) | |
| { | |
| // Corrupt or invalid .winmd; treat as having no activatable classes. | |
| return []; | |
| } | |
| catch (System.IO.IOException) | |
| { | |
| // I/O issues reading the .winmd; treat as having no activatable classes. | |
| return []; | |
| } | |
| catch (System.Exception) | |
| { | |
| // Any other unexpected issue interpreting the .winmd; be resilient and continue. | |
| return []; | |
| } |
| if (string.IsNullOrWhiteSpace(finalPackageName) || string.IsNullOrWhiteSpace(extractedPublisher)) | ||
| { | ||
| try | ||
| { | ||
| if (string.IsNullOrWhiteSpace(finalPackageName)) | ||
| { | ||
| var nameMatch = AppxPackageIdentityNameRegex().Match(manifestContent); | ||
| finalPackageName = nameMatch.Success ? nameMatch.Groups[1].Value : "Package"; | ||
| } | ||
|
|
||
| if (string.IsNullOrWhiteSpace(extractedPublisher)) | ||
| { | ||
| var publisherMatch = AppxPackageIdentityPublisherRegex().Match(manifestContent); | ||
| extractedPublisher = publisherMatch.Success ? publisherMatch.Groups[1].Value : null; | ||
| } | ||
|
|
||
| if (string.IsNullOrWhiteSpace(extractedVersion)) | ||
| { | ||
| var versionMatch = AppxPackageIdentityVersionRegex().Match(manifestContent); | ||
| extractedVersion = versionMatch.Success ? versionMatch.Groups[1].Value : null; | ||
| } | ||
| } |
There was a problem hiding this comment.
extractedVersion is only parsed inside the if (string.IsNullOrWhiteSpace(finalPackageName) || string.IsNullOrWhiteSpace(extractedPublisher)) block. If callers provide both packageName and publisher, extractedVersion remains null and the default output filename will not include the manifest version, which is inconsistent with the new behavior/tests. Consider extracting the version independently of whether name/publisher were provided.
| private async Task<Dictionary<string, string>> GetAllUserPackagesAsync(TaskContext taskContext, CancellationToken cancellationToken) | ||
| { | ||
| var packages = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase); | ||
|
|
||
| // Path 1: Try winapp.yaml | ||
| if (configService.Exists()) | ||
| { | ||
| var config = configService.Load(); | ||
| foreach (var pkg in config.Packages) | ||
| { | ||
| packages.TryAdd(pkg.Name, pkg.Version); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| // Path 2: Try .csproj via `dotnet list package --format json` | ||
| var cwd = new DirectoryInfo(currentDirectoryProvider.GetCurrentDirectory()); | ||
| var csprojFiles = dotNetService.FindCsproj(cwd); | ||
| var csproj = csprojFiles.Count > 0 ? csprojFiles[0] : null; | ||
| if (csproj != null) | ||
| { | ||
| try | ||
| { | ||
| var packageList = await dotNetService.GetPackageListAsync(csproj, cancellationToken); | ||
| var allPackages = packageList?.Projects? | ||
| .SelectMany(p => p.Frameworks ?? []) | ||
| .SelectMany(f => (f.TopLevelPackages ?? []).Concat(f.TransitivePackages ?? [])); | ||
|
|
||
| if (allPackages != null) | ||
| { | ||
| foreach (var pkg in allPackages) | ||
| { | ||
| if (!string.IsNullOrEmpty(pkg.Id) && !string.IsNullOrEmpty(pkg.ResolvedVersion)) | ||
| { | ||
| packages.TryAdd(pkg.Id, pkg.ResolvedVersion); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| taskContext.AddDebugMessage($"{UiSymbols.Warning} Could not retrieve package list from .csproj: {ex.Message}"); | ||
| } | ||
| } |
There was a problem hiding this comment.
GetAllUserPackagesAsync falls back to dotnet list package --format json, but CreateMsixPackageAsync already invokes similar logic (e.g., GetWinAppSDKPackageDependenciesAsync / GetWindowsAppSdkDependencyInfoAsync) that can also call dotnet list package. This can result in multiple expensive dotnet invocations per packaging operation. Consider caching the package list for the duration of the pack operation (or passing it down) to avoid duplicate process executions.
| private async Task AppendThirdPartyWinRTManifestEntriesAsync( | ||
| FileInfo manifestPath, | ||
| string architecture, | ||
| Dictionary<string, string>? winAppSDKPackages, | ||
| TaskContext taskContext, | ||
| CancellationToken cancellationToken) |
There was a problem hiding this comment.
AppendThirdPartyWinRTManifestEntriesAsync has a winAppSDKPackages parameter that isn't used. If it's intentionally unused now, consider removing it from the signature/callers to reduce confusion (or wire it up if it was meant for exclusions).
| var extensionsSb = new StringBuilder(); | ||
| foreach (var component in components) | ||
| { | ||
| var classes = winmdService.GetActivatableClasses(component.WinmdPath); | ||
| if (classes.Count == 0) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| // Check if entries for this DLL already exist in the manifest or in entries we've already generated | ||
| if (manifestContent.Contains(component.ImplementationDll, StringComparison.OrdinalIgnoreCase) | ||
| || extensionsSb.ToString().Contains(component.ImplementationDll, StringComparison.OrdinalIgnoreCase)) | ||
| { |
There was a problem hiding this comment.
Inside the loop, extensionsSb.ToString().Contains(...) re-materializes the entire StringBuilder contents each iteration, which is avoidable overhead. Consider tracking already-added implementation DLLs in a HashSet instead (and/or using manifestContent.Contains + a set) to keep this O(n) without repeated allocations.
| **WinRT component discovery:** | ||
|
|
||
| When packaging, `winapp pack` automatically scans NuGet packages in the `.winapp/packages` cache for third-party WinRT components (e.g., Win2D). It parses `.winmd` files to extract activatable class names and locates their implementation DLLs. The discovered entries are registered as follows: | ||
|
|
||
| - **Framework-dependent** (default): Activatable classes are added as `<InProcessServer>` entries in the `AppxManifest.xml` | ||
| - **Self-contained** (`--self-contained`): Activatable classes are embedded in side-by-side (SxS) manifests within the executable | ||
|
|
||
| Infrastructure packages (e.g., `Microsoft.Windows.SDK.CPP`, `Microsoft.Windows.SDK.BuildTools`) are automatically excluded. |
There was a problem hiding this comment.
This new section says winapp pack scans NuGet packages in the .winapp/packages cache, but the implementation uses nugetService.GetNuGetGlobalPackagesDir() (typically ~/.nuget/packages, or NUGET_PACKAGES if set). Also, earlier in this doc --output is described as defaulting to <name>.msix, but the code now defaults to <name>_<version>.msix when a Version can be extracted. Please update the documentation wording to match the actual discovery location and default output filename behavior.
|
|
||
| private static IEnumerable<DirectoryInfo> SafeEnumDirs(DirectoryInfo root, string searchPattern) | ||
| { | ||
| try { return root.Exists ? root.EnumerateDirectories(searchPattern, SearchOption.AllDirectories) : []; } |
There was a problem hiding this comment.
SafeEnumDirs always enumerates with SearchOption.AllDirectories. When called per package for multiple roots (metadata/lib/References), this forces a full recursive traversal of each package folder and can make packaging noticeably slower on large dependency graphs. Consider probing expected top-level paths directly (e.g., packageDir/metadata, packageDir/lib, packageDir/References) and only recursing within those known folders when necessary.
| try { return root.Exists ? root.EnumerateDirectories(searchPattern, SearchOption.AllDirectories) : []; } | |
| try { return root.Exists ? root.EnumerateDirectories(searchPattern, SearchOption.TopDirectoryOnly) : []; } |
| // Read the existing manifest content and strip the closing </assembly> tag | ||
| // so we can append new entries before re-closing it | ||
| var existingContent = await File.ReadAllTextAsync(manifestPath.FullName, cancellationToken); | ||
| var closingTag = "</assembly>"; | ||
| var closingIndex = existingContent.LastIndexOf(closingTag, StringComparison.OrdinalIgnoreCase); | ||
|
|
||
| var sb = new StringBuilder(); | ||
| if (closingIndex >= 0) | ||
| { | ||
| sb.Append(existingContent, 0, closingIndex); | ||
| } | ||
| else | ||
| { | ||
| sb.Append(existingContent); | ||
| } | ||
|
|
||
| foreach (var component in components) | ||
| { | ||
| var classes = winmdService.GetActivatableClasses(component.WinmdPath); | ||
| if (classes.Count == 0) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| taskContext.AddDebugMessage($"{UiSymbols.Note} Registering {classes.Count} activatable class(es) from {component.ImplementationDll}"); | ||
|
|
||
| sb.AppendLine($" <asmv3:file name='{component.ImplementationDll}'>"); | ||
| foreach (var className in classes) | ||
| { | ||
| sb.AppendLine($" <winrtv1:activatableClass name='{className}' threadingModel='both'/>"); | ||
| } | ||
| sb.AppendLine(" </asmv3:file>"); | ||
| } |
There was a problem hiding this comment.
AppendThirdPartyWinRTManifestEntriesAsync appends asmv3:file blocks unconditionally and doesn't check whether the generated SxS manifest already contains entries for the same DLL/class. If the input manifest already has those registrations (or the method is invoked multiple times), this can produce duplicate activatableClass entries and an invalid/ambiguous manifest. Consider skipping components already present in existingContent (similar to the packaged-Appx manifest path).
Description
During packaging, we now automatically discovers third-party WinRT components (e.g., Win2D, WebView2) from NuGet packages and registers their activatable classes in the appropriate manifest. Works with packages referenced in winapp.yaml or .csproj:
Related Issue
Closes: #325
Type of Change
Checklist