-
Notifications
You must be signed in to change notification settings - Fork 7
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
Extensions samples should not set SuppressTfmSupportBuildWarnings #217
Comments
Thanks for logging this, @ViktorHofer. In our case, this is actually intentional given though packages from dotnet-extensions do work on net462, and we want our samples to also cross-compile for that TFM. Do note that while dotnet/extensions can work in net462, it is not an officially supported configuration, as dotnet/extensions are only officially supported in net6.0 and above. We set the property globally to acknowledge that we understand that those TFMs are unsupported by at least some of the packages, but we still want to provide samples for those. We could add a comment above the setting of this property to explain this, if you think that would be valuable. |
Are those extension samples customer facing? |
Yes, they are the ones in this repo. Essentially all of the samples in this repo work in both net6.0+, as well as net462. But for net462, they throw build warnings due to unsupported configurations in some packages, which is why we set the property. |
I'm not sure why we need this setting on officially supported versions of .NET Framework? Our packages only emit that warning on unsupported TFMs (runtimes): < net6.0 and < net462. |
This applies to packages from dotnet/runtime, but in dotnet/extensions we use the same property to control out of support frameworks, and in there, net462 is not officially supported |
I wasn't aware of that but that worries me, actually a lot. These samples promote this setting to enable experimental .NET Framework support for extensions libraries. At the same time this disables the important dotnet/runtime package warning that is emitted when using our packages on out-of-support runtimes. This is mixing concerns and from our perspective harmful as this disables a part of our packaging infrastructure that we intentionally put in place. |
Discussed this offline with @ViktorHofer and @ericstj. The suggestion is mainly to not use the same property name that is used by the packages from dotnet/runtime, so the proposal is to introduce a new property for packages from dotnet/extensions that allow users to suppress the warning of dotnet/extensions packages only, and we will also make sure that we are backwards compatible by setting it to true if a project is already using the previous property. This issue will now be used to track that work. |
dotnet/extensions#4952 (reply in thread) also requests a |
extensions-samples/Directory.Build.props
Line 10 in c33a8bb
The
SuppressTfmSupportBuildWarnings
switch shouldn't be used in samples as it's a workaround to temporarily allow runtimes to consume a package that doesn't support that runtime anymore by falling back to the netstandard2.0 implementation. For System.* packages this switch might not work and we clearly document that this is an unsupported and untested scenario: https://github.com/dotnet/runtime/blob/28a069017673a03473fdd06e8d693a91e3eb9fbb/eng/packaging.targets#L222The text was updated successfully, but these errors were encountered: