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

Extensions samples should not set SuppressTfmSupportBuildWarnings #217

Open
ViktorHofer opened this issue Jan 11, 2024 · 9 comments
Open

Comments

@ViktorHofer
Copy link
Member

ViktorHofer commented Jan 11, 2024

<SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings>

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#L222

@ViktorHofer
Copy link
Member Author

cc @joperezr @RussKie @ericstj

@joperezr
Copy link
Member

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.

@ViktorHofer
Copy link
Member Author

Are those extension samples customer facing?

@joperezr
Copy link
Member

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.

@ViktorHofer
Copy link
Member Author

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.

@joperezr
Copy link
Member

Our packages

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

@ViktorHofer
Copy link
Member Author

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.

@joperezr
Copy link
Member

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.

@KalleOlaviNiemitalo
Copy link

dotnet/extensions#4952 (reply in thread) also requests a SuppressTfmSupportBuildWarnings-like property for dotnet/extensions packages only.

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

No branches or pull requests

3 participants