-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Issues switching from .NET SDK 6.0.x to 7.0.100 #29164
Comments
Example of code analysis rules that do not cause errors in Visual Studio 17.5.0 Preview 1.0 but does when running |
@mavasani Is this related to the issue you fixed recently? |
This is getting out of hand. We are now seeing .NET SDK 6.0.xyz builds failing on CI machines after .NET SDK 7.0 has been installed in e.g. |
Forget the last part that appears to have been our own mistake because we defined |
cc @vitek-karas @agocke. @nietras - Maybe that issue should be spun off into a separate issue in dotnet/runtime? We want people to set the |
Nope, that was the reverse problem where code analysis rules are not getting enabled. @nietras Can you try to set the |
This is a known issue (now, we figure it out really late unfortunately) - there's a discussion on the problem and potential fixes here: #29031. Your analysis is spot on - the SDK changed to default RID pretty much always. As @eerhardt noted - we want you to set I'm interested in how this worked for you on 6 though. If the app targets |
@mavasani setting <AnalysisLevel>latest</AnalysisLevel>
<AnalysisMode>none</AnalysisMode>
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
<RunAnalyzersDuringBuild>true</RunAnalyzersDuringBuild>
<EnableNETAnalyzers>true</EnableNETAnalyzers>
<CodeAnalysisTreatWarningsAsErrors>true</CodeAnalysisTreatWarningsAsErrors> using compound
Yes, this definitely seems to be the case, as I am working through each CA and updating an # CA1501: Avoid excessive inheritance
dotnet_diagnostic.CA1501.severity = none
# CA1506: Avoid excessive class coupling
dotnet_diagnostic.CA1506.severity = none
# CA1724: Type names should not match namespaces
dotnet_diagnostic.CA1724.severity = none
# CA1810: Initialize reference type static fields inline
dotnet_diagnostic.CA1810.severity = none Additionally, I noticed settings To try to understand I tried to look at how the analyzers severity was defined packaged
@vitek-karas it does produce an However the
I may have given a confusing description in issue. We don't rely on running the exe-directly but:
and hence the below is probably wrong on my part:
As that claims something that assumes the exe is involved in the runs from VS and running test projects that reference the exe-project using mstest for both x86 and x64 like we do. This worked in .NET 6 but not .NET 7 with |
A little side note also discovered a rule https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1852
and I hit the top-level statement issue here too: dotnet/runtime#78270 "Type 'Program' can be sealed because it has no subtypes in its containing assembly and is not externally visible" |
I don't think the Re why |
@nietras Can you also try and add an explicit NuGet package reference to https://dev.azure.com/dnceng/public/_artifacts/feed/dotnet7/NuGet/Microsoft.CodeAnalysis.NetAnalyzers/versions and see if fixes the Code analysis issue? Otherwise, it would make sense to split apart the Code analysis part of this issue into a separate issue in roslyn-analyzers repo. Thanks. |
@vitek-karas thanks for the explanation that fits with my understanding after all then, sorry about the confusing description.
That would would seem likely, but could this not be due to
@mavasani added below to bottom of <ItemGroup>
<PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="7.0.0-preview1.22559.1" />
</ItemGroup> added
I am not privy to where this might be best resolved just let me know what you think and open issues accordingly. |
It could be that PlatformTarget of the dll - but I doubt it. For example the missing runtime start event error - the runtime start event should fire before the runtime even tries to load the dll. But again, I haven't played with it. The real fix is what is described in the referred PR, it sort of doesn't matter why it fails exactly as it should never get into that situation. |
I believe roslyn-analyzers/issues/6281 already has this issue covered? Hoping for feedback on this issue and whether it's a regression between .NET 6 and 7. |
The additional information is included above. We didn't expect RID Inference to be a breaking change but there were after-effects, like here: dotnet/sdk#29164 or here dotnet/sdk#29177
That is correct, dotnet/docs#32784 will add it to the breaking change list. Thanks! |
I just bumped into the CA1852 false-positive (#29164 (comment)) upgrading from 7.0.100 to 7.0.101 in an app with a top-level program:
|
@martincostello This is fixed in dotnet/roslyn-analyzers#6278 |
@nietras We are wrapping up the work to fix the aforementioned Also, this has been documented here due to this issue: https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/7.0/automatic-runtimeidentifier |
I expect the other component with publishing to be fixed in 7.0.200. Will close this soon if there's no more correspondence. |
As a first step in migrating a .NET desktop (WPF,WinForms) application from .NET 6.0 to .NET 7.0 we have been simply switching to the new .NET SDK 7.0.100 by e.g. defining:
where
version
before was say6.0.403
. No other changes were made. However, this caused two sets of issues and we are wondering if these were expected, why and how to handle them properly. The two issues are:<PublishSingleFile>true</PublishSingleFile>
incsproj
PublishSingleFile
This was a head scratcher. The solution in question will build an
AnyCPU
exe that works and runs on both x86 and x64. However, when switching to .NET SDK 7.0.100 this would not work. Either tests would simply not run with issues around bad format for theexe
assembly or similar. Or if trying to run theexe
project e.g. using F5 in visual studio this would simply crash upon start before anything with something likeThe target process exited without raising a CoreCLR started event. Ensure that the target process is configured to use .NET Core."... code 2147516546 (0x80008082)
. As far as I can tell it turns out behavior has changed (silently) forPublishSingleFile
so this now forces aRuntimeIdentifier
(RID) to be set without any warning or similar. This causes the "format" of the output assembly to become platform specific e.g.win-x64
and hence trying to run x86 tests based on it will fail. Changing this definition to:resolved the issue. Guessing you should simply not define these options in csproj in the future if you have project that you want to build as Any CPU but instead do that only on command line when actually doing publish. Which means these often become redundantly defined.
Code Analysis Changes
This we don't understand. We have the following defined in
Directory.Build.props
with an accompanying.editorconfig
next tosln
at root of git repo.This compiled fine with zero errors using .NET 6.0 SDK. Switching to .NET SDK 7.0.100 and we had to define the following
NoWarn
to the solution building without errors. Note that the set of CAxxxx errors reported in VS and on command line are different this is the union of both to get both building.that's a lot of new code analysis rules suddenly causing errors and this seems off, is this expected?
We would also the have expected to be able to set
AnalysisLevel
to6.0
and get the 6.0 SDK behavior but this had no effect at all. Was this not the intended function of this property?Further technical details
dotnet --info
Visual Studio 17.5.0 Preview 1.0
The text was updated successfully, but these errors were encountered: