-
Notifications
You must be signed in to change notification settings - Fork 96
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.
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
IncludeAssets=all advice causes problems with NuGet packages #1158
Comments
The missing dependencies seems like a valid report. Thanks for sharing. |
The documentation in the README suggests setting IncludeAssets=all (by removing the auto-generated metadata). This suggestion can lead to broken NuGet packages. Something should probably be done about this suggestion to make it less problematic, either by amending it, or adding build targets to warn on this specific situation, or perhaps something else (I'm not really sure). |
I don't yet understand how removing IncludeAssets metadata breaks anything. The specific break you speak of in the description relates to missing references to System.Memory and System.Runtime.CompilerServices.Unsafe. Those don't propagate to your own consumers automatically when you set PrivateAssets=all. That's not the same thing as IncludeAssets=all. |
Yes, but without setting IncludeAssets=All, you cannot use those dependencies, so it does not matter that they don't flow. You don't get access to System.Memory so you can't introduce an assembly reference to it. The source generator would, of course, like to use these dependencies--that's why you suggest setting it in the first place. But it causes problems in some instances (like this one). |
Ah, OK I get it now. And at least in some cases, the source generator is smart enough to avoid emitting APIs that depend on these when they aren't referenced. Thank you for your patience in helping me understand. |
I also ran into this issue. This is the error I was getting:
As a work-around I had to add a reference to the Microsoft.Windows.CsWin32 v0.3.49-beta references System.Runtime.CompilerServices.Unsafe v6.0.0, so I also referenced that specific version. I also had to add an assembly binding redirection in
Thank you. |
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 #1158
I'd love to get you folks to validate the effectiveness of my fix in #1172. |
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 #1158
The documentation in the README suggests that the default
<IncludeAssets>
boilerplate generated by .NET tooling is removed:Though this usually causes no ill effects, this advice is dangerous for projects which produce a NuGet package. Since CsWin32 is marked as PrivateAssets=all,
System.Memory
andSystem.Runtime.CompilerServices.Unsafe
will not be marked as dependencies of the built package, even though the built DLL depends on their assemblies. In other words, using this technique on a project which produces a NuGet package will break the package.Easy reproduction:
netstandard2.0
project and add the following package reference:System.Runtime.CompilerServices.Unsafe
so DLL has an assembly reference to it.Note that, according to the nuspec, this package does not have any dependencies, but the DLL in the package has an assembly reference to
System.Runtime.CompilerServices.Unsafe
6.0.0. This will cause errors for consumers of this package.I'm not sure how actionable this is, but this is a pretty bad bug that's easy to miss. One way of addressing it to emit an error from the CsWin32 package when this situation is detected. I don't know if anything further is possible without altering the behavior of NuGet.
The text was updated successfully, but these errors were encountered: