Skip to content

Commit

Permalink
Drop the implicit System.Memory reference from the package
Browse files Browse the repository at this point in the history
Because we advise users to set `PrivateAssets=all` on the CsWin32 package itself, its transitive dependencies don't pass through to the CsWin32 user's own consumers, leading to compilation or binding redirect issues because System.Memory is missing.
By dropping it from the package itself and requiring CsWin32 users to reference it directly, we put a bit more responsibility on them but solve the problem of the missing reference.

To help users get this right, we report a new warning when System.Memory hasn't been referenced.

Fixes microsoft#1158
  • Loading branch information
AArnott committed May 3, 2024
1 parent 200857d commit 6d6fc60
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 24 deletions.
13 changes: 7 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,16 @@ Install the `Microsoft.Windows.CsWin32` package:
dotnet add package Microsoft.Windows.CsWin32 --prerelease
```

**Tip**: Remove the `IncludeAssets` metadata from the package reference so that you get better code generation by allowing nuget to bring in the `System.Memory` package as a transitive dependency.
You should also install the `System.Memory` package when targeting .NET Framework 4.5+ or .NET Standard 2.0,
as that adds APIs that significantly improve much of the code generated by CsWin32:

```diff
<PackageReference Include="Microsoft.Windows.CsWin32" Version="0.1.647-beta">
<PrivateAssets>all</PrivateAssets>
- <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
```ps1
dotnet add package System.Memory
```

Projects targeting .NET Core 2.1+ or .NET 5+ do *not* need to add the `System.Memory` package reference,
although it is harmless to do so.

Your project must allow unsafe code to support the generated code that will likely use pointers.
This does *not* automatically make all your code *unsafe*.
Use of the `unsafe` keyword is required anywhere you use pointers.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,29 +24,21 @@
<dependency id="Microsoft.Windows.SDK.Win32Metadata" version="$MetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.WDK.Win32Metadata" version="$WDKMetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.SDK.Win32Docs" version="$ApiDocsVersion$" include="buildTransitive" />
<dependency id="System.Memory" version="4.5.5" include="All" />
<dependency id="System.Runtime.CompilerServices.Unsafe" version="5.0.0" include="All" />
</group>
<group targetFramework="net461">
<dependency id="Microsoft.Windows.SDK.Win32Metadata" version="$MetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.WDK.Win32Metadata" version="$WDKMetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.SDK.Win32Docs" version="$ApiDocsVersion$" include="buildTransitive" />
<dependency id="System.Memory" version="4.5.5" include="All" />
<dependency id="System.Runtime.CompilerServices.Unsafe" version="6.0.0" include="All" />
</group>
<group targetFramework=".NETStandard1.1">
<dependency id="Microsoft.Windows.SDK.Win32Metadata" version="$MetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.WDK.Win32Metadata" version="$WDKMetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.SDK.Win32Docs" version="$ApiDocsVersion$" include="buildTransitive" />
<dependency id="System.Memory" version="4.5.5" include="All" />
<dependency id="System.Runtime.CompilerServices.Unsafe" version="5.0.0" include="All" />
</group>
<group targetFramework=".NETStandard2.0">
<dependency id="Microsoft.Windows.SDK.Win32Metadata" version="$MetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.WDK.Win32Metadata" version="$WDKMetadataVersion$" include="buildTransitive" />
<dependency id="Microsoft.Windows.SDK.Win32Docs" version="$ApiDocsVersion$" include="buildTransitive" />
<dependency id="System.Memory" version="4.5.5" include="All" />
<dependency id="System.Runtime.CompilerServices.Unsafe" version="6.0.0" include="All" />
</group>
</dependencies>
</metadata>
Expand Down
13 changes: 13 additions & 0 deletions src/Microsoft.Windows.CsWin32/SourceGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,14 @@ public class SourceGenerator : ISourceGenerator
"Configuration",
DiagnosticSeverity.Error,
isEnabledByDefault: true);

public static readonly DiagnosticDescriptor MissingRecommendedReference = new(
"PInvoke009",
"Missing package reference",
"Missing reference to recommended package: \"{0}\"",
"Configuration",
DiagnosticSeverity.Warning,
isEnabledByDefault: true);
#pragma warning restore CS1591 // Missing XML comment for publicly visible type or member

private const string NativeMethodsTxtAdditionalFileName = "NativeMethods.txt";
Expand Down Expand Up @@ -185,6 +193,11 @@ public void Execute(GeneratorExecutionContext context)
context.ReportDiagnostic(Diagnostic.Create(UnsafeCodeRequired, location: null));
}

if (compilation.GetTypeByMetadataName("System.Memory`1") is null)
{
context.ReportDiagnostic(Diagnostic.Create(MissingRecommendedReference, location: null, "System.Memory"));
}

Docs? docs = ParseDocs(context);
SuperGenerator superGenerator = SuperGenerator.Combine(CollectMetadataPaths(context).Select(path => new Generator(path, docs, options, compilation, parseOptions)));
try
Expand Down
17 changes: 7 additions & 10 deletions src/Microsoft.Windows.CsWin32/readme.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,16 @@ methods and supporting types to a C# project.
To get started, create a "NativeMethods.txt" file in your project directory
that lists the names of Win32 APIs for which you need to have generated, one per line.

Tips
----
You should also install the `System.Memory` package when targeting .NET Framework 4.5+ or .NET Standard 2.0,
as that adds APIs that significantly improve much of the code generated by CsWin32:

Remove the `IncludeAssets` metadata from the package reference so that you get better code generation
by allowing nuget to bring in the `System.Memory` package as a transitive dependency.

```diff
<PackageReference Include="Microsoft.Windows.CsWin32" Version="0.1.647-beta">
<PrivateAssets>all</PrivateAssets>
- <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
```ps1
dotnet add package System.Memory
```

Projects targeting .NET Core 2.1+ or .NET 5+ do *not* need to add the `System.Memory` package reference,
although it is harmless to do so.

Your project must allow unsafe code to support the generated code that will likely use pointers.

Learn more from our README on GitHub: https://github.com/microsoft/CsWin32#readme
42 changes: 42 additions & 0 deletions test/Microsoft.Windows.CsWin32.Tests/SourceGeneratorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,46 @@ public async Task UnparseableNativeMethodsJson()
},
}.RunAsync();
}

/// <summary>
/// Asserts that no warning is produced even without the required reference, when no source is being generated anyway.
/// </summary>
[Fact]
public async Task MissingSystemMemoryReference_NoGeneratedCode()
{
await new VerifyCS.Test
{
ReferenceAssemblies = ReferenceAssemblies.NetFramework.Net472.Default,
}.RunAsync();
}

/// <summary>
/// Asserts that a warning is produced when targeting a framework that our generated code requires the System.Memory reference for, but the reference is missing.
/// </summary>
[Fact]
public async Task MissingSystemMemoryReference_WithGeneratedCode_NetFx472()
{
await new VerifyCS.Test
{
ReferenceAssemblies = ReferenceAssemblies.NetFramework.Net472.Default,
NativeMethodsTxt = "CreateFile",
ExpectedDiagnostics =
{
new DiagnosticResult(SourceGenerator.MissingRecommendedReference.Id, DiagnosticSeverity.Warning),
},
}.RunAsync();
}

/// <summary>
/// Asserts that when targeting a framework that implicitly includes the references we need, no warning is generated.
/// </summary>
[Fact]
public async Task MissingSystemMemoryReference_WithGeneratedCode_Net60()
{
await new VerifyCS.Test
{
ReferenceAssemblies = ReferenceAssemblies.Net.Net60,
NativeMethodsTxt = "CreateFile",
}.RunAsync();
}
}

0 comments on commit 6d6fc60

Please sign in to comment.