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.

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

Exclude inboxed analyzers from VerifyClosure task #7624

Merged
merged 1 commit into from
Jul 15, 2021

Conversation

RussKie
Copy link
Member

@RussKie RussKie commented Jul 15, 2021

As per https://github.com/dotnet/designs/blob/main/accepted/2021/InboxSourceGenerators.md inboxed source generators are placed under ref/ folder, and need to be excluded from RuntimePack reference check.

Fixes broken build dotnet/windowsdesktop#1803

Tested locally manually modifying C:\Development\windowsdesktop.packages\microsoft.dotnet.sharedframework.sdk\6.0.0-beta.21363.2\targets\sharedfx.targets and getting the build to pass:
image

As per https://github.com/dotnet/designs/blob/main/accepted/2021/InboxSourceGenerators.md
inboxed source generators are placed under ref/ folder, and need to be
excluded from RuntimePack reference check.

Fixes broken build dotnet/windowsdesktop#1803
@RussKie RussKie requested review from ericstj and ryalanms July 15, 2021 08:01
@RussKie
Copy link
Member Author

RussKie commented Jul 15, 2021

Tagged as critical as we're unable to build Windows Desktop, and thus won't be able to snap for Preview7.

@@ -457,6 +457,9 @@

<ItemGroup Condition="'$(PlatformPackageType)' == 'RuntimePack'">
<IgnoredReference Include="@(RuntimePackAsset->'%(FileName)')" />
<!-- Ignore inboxed analyzers -->
<IgnoredReference Include="@(Reference->'%(FileName)')"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Analyzers shouldn’t be raised as references. They are not listed as references in the framework list. This is only happening because you are making an old SDK use a new ref pack. This can go away once you pick up a Preview7 SDK with dotnet/sdk@07768a0#diff-1b35791f7e51f6b95005b2fb7aa5e65ceca0d11578158af081003c91d97b4373R80

This workaround Is fine for now, but please delete it once you update the SDK.

@ericstj
Copy link
Member

ericstj commented Jul 15, 2021

inboxed source generators are placed under ref/ folder, and need to be excluded from RuntimePack reference check

They are not, see comment above.

Copy link
Member

@ericstj ericstj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for now but please remove after Preview7

@ryalanms
Copy link
Member

@lukas-lansky @MattGal: Can this be merged now? Thanks.

@ryalanms
Copy link
Member

/cc @dotnet/wpf-developers

@MattGal MattGal merged commit a311590 into main Jul 15, 2021
@ericstj
Copy link
Member

ericstj commented Jul 15, 2021

Actually, I hadn't realized this was in Arcade. You could have done this in the WindowsDesktop repo itself and save yourself having to wait for Arcade update. No need to do this in the shared framework SDK. The place to do so is here: https://github.com/dotnet/windowsdesktop/blob/2f830140b7df74ebcf518b73b9252534e751a7ef/pkg/windowsdesktop/sfx/Directory.Build.targets#L29-L40

You could simply list the single assembly name that's causing trouble: System.Text.Json.SourceGeneration

@ericstj
Copy link
Member

ericstj commented Jul 15, 2021

Upon closer inspection, this isn't just a validation problem. Those references are actually appearing in product binaries:

C:\src\dotnet\windowsdesktop\.packages\microsoft.dotnet.sharedframework.sdk\6.0.0-beta.21363.2\targets\sharedfx.targets(467,5): error : Assembly 'DirectWriteForwarder' is missing dependency 'System.Text.Json.SourceGeneration' [C:\src\dotnet\windowsdesktop\pkg\windowsdesktop\sfx\Microsoft.WindowsDesktop.App.Runtime.sfxproj]
C:\src\dotnet\windowsdesktop\.packages\microsoft.dotnet.sharedframework.sdk\6.0.0-beta.21363.2\targets\sharedfx.targets(467,5): error : Assembly 'System.Printing' is missing dependency 'System.Text.Json.SourceGeneration' [C:\src\dotnet\windowsdesktop\pkg\windowsdesktop\sfx\Microsoft.WindowsDesktop.App.Runtime.sfxproj]

This is because C++/CLI doesn't behave like CSC WRT to only emitting references it uses. C++/CLI will produce assemblies with assembly refs regardless of whether or not the assembly is used. We need to prevent C++/CLI from seeing those references.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants