Skip to content

SDK reports a number of new errors when multi-targeting and enabling AOT/Trim analyzers #35528

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

Closed
tannergooding opened this issue Sep 19, 2023 · 14 comments · Fixed by #35767
Closed
Labels
Area-NetSDK untriaged Request triage from a team member

Comments

@tannergooding
Copy link
Member

As per the title, the .NET 8 RC1 SDK is reporting a number of new errors when setting properties such as:

  • EnableAotAnalyzer
  • EnableSingleFileAnalyzer
  • EnableTrimAnalyzer
  • IsTrimmable

These errors surface if the properties are set to true and the TFM does not support the functionality.

This is unfortunate because it puts a responsibility on the end user to understand the inner complexities and limitations of the tooling and what TFMs they support in a multi-targeted project.

The SDK and tooling itself already knows this information and it would, in my opinion, be significantly better if the SDK simply did the right thing and ignored the property on TFMs where the analyzer don't work.

It might be desirable to still surface a warning so users are aware that netstandard2.0 won't be analyzed or trimmed, but I wholeheartedly believe that erroring out here is the wrong thing and that if a warning is surfaced, the user should be able to suppress it.

@ghost ghost added Area-NetSDK untriaged Request triage from a team member labels Sep 19, 2023
@tannergooding
Copy link
Member Author

This looks to be caused by #34077 and has already been hit by various dotnet repos.

@eerhardt
Copy link
Member

@agocke @sbomer @vitek-karas - FYI.

I'm not sure where we landed is a good place for typical library authors.

@tannergooding
Copy link
Member Author

tannergooding commented Sep 19, 2023

I've just seen a lot of people basically avoid anything MSBuild related, because they view it as spooky/complex

My expectation is that a typical developer won't know they can do something like '$(TargetFrameworkIdentifier)' == '.NETCoreApp or $([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', '...')) or any of these other conditionals that may or may not work depending on their setup

So I expect they'll try to enable trimming/AOT, get an error that they don't know how to handle, not be able to find a solid solution for it because it's MSBuild, and then will simply not go through with enabling the functionality.

The tooling knows what it does and doesn't support and so it's fully capable of doing the right thing by default and surfacing a warning or informational diagnostic that the user could suppress. That helps, in my opinion, lead users towards a pit of success around the feature.

@sbomer
Copy link
Member

sbomer commented Sep 19, 2023

The breaking change is described here: https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/8.0/trimming-unsupported-targetframework.

The SDK and tooling itself already knows this information and it would, in my opinion, be significantly better if the SDK simply did the right thing and ignored the property on TFMs where the analyzer don't work.

It might be desirable to still surface a warning so users are aware that netstandard2.0 won't be analyzed or trimmed

That's the intention for the properties you listed - they should have no effect, except that they produce a warning (not an error). Are you seeing something different? Maybe we need to adjust the wording of the message if that's not clear - happy to take any suggestions.

Note that the publish settings (PublishAot, PublishTrimmed) produce hard errors instead - we discussed this and came to the conclusion that the publish settings in particular warranted an error on unsupported TFMs, even if the above properties were treated as a warning.

@tannergooding
Copy link
Member Author

tannergooding commented Sep 19, 2023

I saw a number of different build failures related to these properties but didn't check exactly which reported what kind.

It simply represented a break from the previous preview/release and one where I had to go dig up uncommon MSBuild knowledge to resolve.

It makes the entire experience with multi-targeting and supporting AOT painful and much harder to get people to adopt if they don't have the knowledge of how to resolve MSBuild issues.

@sbomer
Copy link
Member

sbomer commented Sep 19, 2023

Yeah, it's definitely a break and will cause some pain, but we hope it makes the experience more predictable in the long run. Before this, we allowed turning on these analyzers for netstandard2.0 apps even though they produced incomplete results (since the netstandard ref assemblies aren't annotated for trim compatibility).

If it helps at all, the warnings include the right MSBuild magic so that people don't have to figure it out themselves:

 warning NETSDK1210: IsAotCompatible and EnableAotAnalyzer are not supported for the target framework. Consider multi-targeting to a supported framework to enable ahead-of-time compilation analysis, and set IsAotCompatible only for the supported frameworks. For example:
 warning NETSDK1210: <IsAotCompatible Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))">true</IsAotCompatible> 

@tannergooding
Copy link
Member Author

Why not just have the analyzer be a no-op for those TFMs? They don't support trimming, so there's nothing for a dev to do, the tooling can simply decide to not run the analyzer for unsupported TFMs and the user will see nothing different.

I can somewhat understand PublishTrimmed since they may expect trimming to occur and then find it didn't happen. But even that seems like something that could be a warning since the tool is functionally doing a no-op there. It can't trim it, so it produces the same output.

@sbomer
Copy link
Member

sbomer commented Sep 19, 2023

Libraries built for a TFM that's not supported by the tooling can still be included as a dependency of an app, where it might produce trim warnings when the app gets published. If a library author sets IsTrimmable and it produces no warnings, for the supported TFMs this is supposed to mean that the library is trim-compatible. For the unsupported TFMs, we want to avoid giving the false impression that the library is trim-compatible.

@vitek-karas
Copy link
Member

For the motivation behind this: We've seen people run into the "False sense of security" - they set IsTrimmable=true and didn't get any warnings, so thought they were fine (which was not the case obviously).

We've also seen cases where project has some net6.0 libraries but it also has some netstandard2.0 libraries in the mix. Currently we don't have a good way to run analyzers on all of those, because we don't know what it means to run the analyzer on netstandard2.0 (the analyzer would run, but would produce cca 50% of the warnings it should, because the ref assemblies from framework are missing trim annotations).

Finally, we really want people to multi-target to NET 6+, since otherwise they will have a problem with annotating the library (the attributes only exist in NET 6, so they would have to backfill their implementations).

As for hard error when trying to PublishTrimmed/PublishAot something which is not net6/net7+:

  • For AOT this is actually important - in 7+ dotnet publish /PublishAot=true on a classlib project is actually a valid scenario and will produce a native library in the output. On 6- this doesn't work, so I think it's reasonable to fail.
  • For Trimming, currently trimming libraries is not supported in any case, but again, I think it's important to error since we're not going to do what the user asked for. In addition, it's basically undefined what does it mean to dotnet publish a classlib project.

Overall - our main design goal behind trimming/AOT is to make the experience predictable - no surprises. Large part of how we achieve that is to provide built-time feedback about possible incompatibilities. We actually started with publish-time feedback only, but got reactions that it's cumbersome, so we added the analyzer for IDE/build-time feedback. Recently we've seen people (even our own - ASP.NET) basically ask if they can rely solely on the analyzer, as it's an easier experience. All this combined: if the analyzer is unreliable because it sometimes doesn't run without any feedback to the user, then developers will lose trust in it and we're back in publish-time feedback land (which is really not a great UX for library authors).

I definitely appreciate that the suggested solution to make the IsTrimmably conditioned on TFM is not very nice, so if there are better ideas, we'd love to hear them. Maybe suppressing the warnings (although that's potential dangerous)? What we really wanted was:

  • netstandard2.0 library should produce a warning - to get rid of the false sense of security
  • guide library authors toward multi-targeting with net6+
  • make it possible, but not simple, to annotate netstandard2.0 library

@MichalPetryka
Copy link

MichalPetryka commented Sep 20, 2023

  • guide library authors toward multi-targeting with net6+

Why not suppress the warning automatically when multi-targetting then? Or use the supported TFM for checking of the unsupported one then?

@vitek-karas
Copy link
Member

Why not suppress the warning automatically when multi-targetting then?

We were looking into that... and I'm revisiting that right now as well. It was rather difficult, but we'll do more investigation if there's some better way.

@lewing
Copy link
Member

lewing commented Sep 28, 2023

Xamarin and wasm apps have been trimming netstandard assemblies since pre NET5 as part of every build, this appears to be a massive breaking change for these targets first introduced in rc1??

@sbomer
Copy link
Member

sbomer commented Sep 28, 2023

Pieces of the breaking change were introduced much earlier, starting with #29441, but the last piece of the break specifically for netstandard2.0 projects unfortunately came in quite late with #34077, when I discovered a bug specific to netstandard2.0 (as opposed to netstandard2.1 for example).

For what it's worth, we're re-evaluating in response to the feedback here, and are limiting the scope of the break to apps that don't multitarget to a supported TFM.

sbomer added a commit that referenced this issue Oct 3, 2023
…#35767)


The warnings introduced by
 #34077 and described in
 https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/8.0/trimming-unsupported-targetframework
 are unnecessary noise for projects that multitarget to include a
 TargetFramework that is compatible with
 trimming/AOT/single-file, and is a low enough version to ensure
 that it will be picked over any other TFM's assets when the
 library is consumed in a trimmed/AOT'd/single-file app.

This fixes the issue by detecting correctly multi-targeted
libraries and suppressing the warnings in these cases.

Note that prior to the .NET 8 SDK, netstandard libraries would
still get the `IsTrimmable` attribute embedded in the assembly
when setting `IsTrimmable`, and could use trim analysis despite
incomplete warnings due to unannotated ref assemblies. With the
.NET 8 SDK this is no longer the case. Even for correctly
multi-targeted libraries, the netstandard output assembly will no
longer contain the `IsTrimmable` attribute. We debated whether it
was worth warning in this case, but decided that the negative
impact of the warning on library developers following the "golden
path" was too large to justify keeping this as a warning.

This solution addresses a correctness problem that we wanted to
avoid (the correctness problem: allowing an inadvertently
non-"trimmable" netstandard asset to be consumed in a trimmed app
that uses a supported TFM).

It's still possible to get into that situation in an app that
explicitly references the netstandard assembly, or that targets
an EOL TFM (causing it to consume the netstandard asset from a
library that multitargets `netstandard2.0;net6.0` for example).

Fixes #35528
sbomer added a commit to sbomer/sdk that referenced this issue Oct 4, 2023
…dotnet#35767)

The warnings introduced by
 dotnet#34077 and described in
 https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/8.0/trimming-unsupported-targetframework
 are unnecessary noise for projects that multitarget to include a
 TargetFramework that is compatible with
 trimming/AOT/single-file, and is a low enough version to ensure
 that it will be picked over any other TFM's assets when the
 library is consumed in a trimmed/AOT'd/single-file app.

This fixes the issue by detecting correctly multi-targeted
libraries and suppressing the warnings in these cases.

Note that prior to the .NET 8 SDK, netstandard libraries would
still get the `IsTrimmable` attribute embedded in the assembly
when setting `IsTrimmable`, and could use trim analysis despite
incomplete warnings due to unannotated ref assemblies. With the
.NET 8 SDK this is no longer the case. Even for correctly
multi-targeted libraries, the netstandard output assembly will no
longer contain the `IsTrimmable` attribute. We debated whether it
was worth warning in this case, but decided that the negative
impact of the warning on library developers following the "golden
path" was too large to justify keeping this as a warning.

This solution addresses a correctness problem that we wanted to
avoid (the correctness problem: allowing an inadvertently
non-"trimmable" netstandard asset to be consumed in a trimmed app
that uses a supported TFM).

It's still possible to get into that situation in an app that
explicitly references the netstandard assembly, or that targets
an EOL TFM (causing it to consume the netstandard asset from a
library that multitargets `netstandard2.0;net6.0` for example).

Fixes dotnet#35528
sbomer added a commit that referenced this issue Oct 6, 2023
…ngle-file warnings (#35851)

Backport of #35767.

## Customer Impact
In .NET 8 we made a breaking change that introduces a warning when setting `IsTrimmable` or related properties in a .NET Standard library. The warning was introduced because we changed the behavior (to not reference the trim analyzers, and to not mark the assembly with `[assembly: AssemblyMetadat("IsTrimmable", "True")]`.

The intended way to fix this in a library was by multi-targeting the library to include a TFM supported by trimming, and to set `IsTrimmable` only on the supported TFMs. We got feedback (see #35528) that the warning was a painful break for multitargeted libraries, and that the warning shouldn't be produced in the first place for libraries following our guidance.

This change addresses it by silencing the warning for correctly multitargeted projects. The warning is silenced only when the multitargeting set includes a low enough supported TFM to ensure that the `IsTrimmable` assets will be consumed (instead of non-trimmable .NET Standard assets) in any supported app that references the library.

## Testing

Existing unit tests passed without changes. Added tests to validate the behavior for various multitargeted projects. Added tests to ensure that the changes don't break the behavior of projects that use custom `TargetFramework` values as aliases for `TargetFrameworkIdentifier` and `TargetFrameworkVersion`.

## Risk

Low to medium. This change reduces overall impact of the breaking change by limiting the scope of the warning, but adds complexity to the behavior. The complexity is unlikely to be surfaced to the developer, but is observable (it would be confusing to any developer who tried to understand the specific circumstances under which the warning was produced).

## Original description


The warnings introduced by
 #34077 and described in
 https://learn.microsoft.com/en-us/dotnet/core/compatibility/sdk/8.0/trimming-unsupported-targetframework
 are unnecessary noise for projects that multitarget to include a
 TargetFramework that is compatible with
 trimming/AOT/single-file, and is a low enough version to ensure
 that it will be picked over any other TFM's assets when the
 library is consumed in a trimmed/AOT'd/single-file app.

This fixes the issue by detecting correctly multi-targeted libraries and suppressing the warnings in these cases.

Note that prior to the .NET 8 SDK, netstandard libraries would still get the `IsTrimmable` attribute embedded in the assembly when setting `IsTrimmable`, and could use trim analysis despite incomplete warnings due to unannotated ref assemblies. With the .NET 8 SDK this is no longer the case. Even for correctly multi-targeted libraries, the netstandard output assembly will no longer contain the `IsTrimmable` attribute. We debated whether it was worth warning in this case, but decided that the negative impact of the warning on library developers following the "golden path" was too large to justify keeping this as a warning.

This solution addresses a correctness problem that we wanted to avoid (the correctness problem: allowing an inadvertently non-"trimmable" netstandard asset to be consumed in a trimmed app that uses a supported TFM).

It's still possible to get into that situation in an app that explicitly references the netstandard assembly, or that targets an EOL TFM (causing it to consume the netstandard asset from a library that multitargets `netstandard2.0;net6.0` for example).

Fixes #35528
@YoshiRulz
Copy link

YoshiRulz commented Jun 27, 2024

edit: Solve-after-post strikes again. I found the props were also being set in Directory.Build.targets. I couldn't tell you why there were things other than targets in there.

original post

I thought the warning was erroneously triggering, but it seems the suggested fix is just misleading. My project file looked like

<PropertyGroup>
	<TargetFrameworks>netstandard2.0;netcoreapp3.0;net8.0</TargetFrameworks>
	<IsAotCompatible Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))">true</IsAotCompatible>
</PropertyGroup>

exactly as suggested, yet

$> rm -fr bin obj /tmp/.dotnet; dotnet build
  Determining projects to restore...
.../8.0.300/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(90,5): warning NETSDK1210: IsAotCompatible and EnableAotAnalyzer are not supported for the target framework. Consider multi-targeting to a supported framework to enable ahead-of-time compilation analysis, and set IsAotCompatible only for the supported frameworks. For example: [.../myProj/myProj.csproj::TargetFramework=netstandard2.0]
.../8.0.300/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(90,5): warning NETSDK1210: <IsAotCompatible Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))">true</IsAotCompatible> [.../myProj/myProj.csproj::TargetFramework=netstandard2.0]
  Restored .../myProj/myProj.csproj (in 429 ms).
.../8.0.300/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(90,5): warning NETSDK1210: IsAotCompatible and EnableAotAnalyzer are not supported for the target framework. Consider multi-targeting to a supported framework to enable ahead-of-time compilation analysis, and set IsAotCompatible only for the supported frameworks. For example: [.../myProj/myProj.csproj::TargetFramework=netstandard2.0]
.../8.0.300/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(90,5): warning NETSDK1210: <IsAotCompatible Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))">true</IsAotCompatible> [.../myProj/myProj.csproj::TargetFramework=netstandard2.0]
  myProj -> .../myProj/bin/Debug/netstandard2.0/myProj.dll
  myProj -> .../myProj/bin/Debug/netcoreapp3.0/myProj.dll
  myProj -> .../myProj/bin/Debug/net8.0/myProj.dll

This appears to work:

<IsAotCompatible>false</IsAotCompatible>
<IsAotCompatible Condition=" $([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0')) ">true</IsAotCompatible>
<!-- or simply -->
<IsAotCompatible>$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))</IsAotCompatible>

However, I'm now getting the similar NETSDK1212 warning, and I can't figure out how to resolve it. Not even a blanket <IsTrimmable>false</IsTrimmable> works—it gets overridden to true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-NetSDK untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants