dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.67k stars 1.06k forks source link

Define a platform-agnostic NET preprocessor symbol #22788

Open maxkoshevoi opened 2 years ago

maxkoshevoi commented 2 years ago

Is your feature request related to a problem? Please describe.

Before .Net 6 days, if I wanted to multitarget to different platforms and split code between them, I could do it with:

Now, if I target net6.0;net6.0-android;net6.0-ios, I can still do #if ANDROID (or #if NET6_0_ANDROID_OR_GREATER), but there's no symbol to check that current TFM is net6.0 (platform-agnostic) like NETSTANDARD, since both NET and NET6_0 are also defined in platform-spesific TFMs.

Describe the solution you'd like

Define a new preprocessor symbol that would indicate platform-agnostic TMF

Workaround

<PropertyGroup Condition=" !$(TargetFramework.Contains('-')) ">
    <DefineConstants>$(DefineConstants);NETSTANDARD</DefineConstants>
</PropertyGroup>
dsplaisted commented 2 years ago

Does something like the following not work well for you?

#if ANDROID

#elif IOS

#elif WINDOWS

#elif NET
//  Platform agnostic code here
#endif

@terrajobst @mhutch What do you think?

maxkoshevoi commented 2 years ago

@dsplaisted What about macos? Anyway, this could work, but writing this boilerplate every time I need a platform-agnostic symbol sounds a bit cumbersome.

dsplaisted commented 2 years ago

It seems to me that if you want a platform-agnostic #if, then you are probably going to need platform-specific code for each platform too. If that's the case then you're going to have the #if clauses for each platform anyway and it doesn't seem cumbersome to put the platform agnostic code in the #else clause.

But I don't write much code like this, so I'd love to hear if there are code patterns where it doesn't work.

For macos, there should be a define for either MACOS or MACCATALYST depending on what the TargetFramework is.

maxkoshevoi commented 2 years ago

I also don't write much code like this, but thought that having this symbol would be useful.

#else argument does make sense, but only if code for all platforms is in the same file. I came across the need for such symbol when migrating Xamarin.CommunityToolkit to MAUI (to support net.6.0 target): https://github.com/xamarin/XamarinCommunityToolkit/blob/Xamarin.CommunityToolkit.MauiCompat-1.3.0/src/CommunityToolkit/Xamarin.CommunityToolkit.MauiCompat/Views/DrawingView/Service/DrawingViewService.shared.cs#L9

Nirmal4G commented 2 years ago

Adding NETSTANDARD and NETCORE to the platform-agnostic net TFM should solve this issue. BTW I'm also using the same workaround. Instead of adding new ones, I prefer existing symbols, since according to the .NET announcement...

the new net TFM combines and replaces netstandard and netcoreapp TFMs!

maxkoshevoi commented 2 years ago

Lack of such symbol does result in less readable code. Found an example from MAUI Toolkit: https://github.com/CommunityToolkit/Maui/blob/main/src/CommunityToolkit.Maui/Alerts/Snackbar/SnackBar.shared.cs#L95

dsplaisted commented 2 years ago

@maxkoshevoi I don't understand that MAUI code. It looks like it's defining Show and Dismiss methods for the net6.0 TargetFramework but not for any of the operating system specific TargetFrameworks.

What happens if you have net6.0 code that calls these methods, but at runtime the operating system specific implementation is used? Won't you get a MissingMethodException?

This seems to violate the bait and switch pattern

maxkoshevoi commented 2 years ago

@dsplaisted Those methods are present in every platform-spesific file, shared.cs just contains default implementation (in case platform-spesific one is not present).

Hm, guess this example is inapplicable to the issue, since default implementation also should be present for unsupported platforms, and not just for platform-agnostic TFM.

maxkoshevoi commented 2 years ago

Hm, guess this example is inapplicable to the issue

Was wrong again, it is applicable. Let's assume 1) I have platform-spesific code in separate files, and I support Android, iOS, MacCatalyst and Windows (like in example above) 2) I need a default implementation of platform-spesific logic for platform-afnostic TFM 3) I need a default implementation of platform-spesific logic for unsupported platform (let's say Tizen)

This:

public virtual partial Task Show();
public virtual partial Task Dismiss();

#if (NETSTANDARD || TIZEN)
public virtual partial Task Show() { ... }
public virtual partial Task Dismiss() { ... }
#endif

looks waay better (and more readable) than this:

public virtual partial Task Show();
public virtual partial Task Dismiss();

#if !(IOS || ANDROID || MACCATALYST || WINDOWS)
public virtual partial Task Show() { ... }
public virtual partial Task Dismiss() { ... }
#endif
dsplaisted commented 2 years ago

What would you suggest as a name for the preprocessor symbol. We are already using NET, which will be defined for all of the target frameworks in the example.

If we had a good name for it then I would probably be OK with adding a preprocessor symbol.

I think what contributes to the issue in the example is that separate platform-specific source files are being mixed with conditional compilation. Rather than a clunky #if statement, perhaps it would be better to put the Show and Dismiss declarations in a separate source file that is only compiled when not targeting one of the other OS's.

maxkoshevoi commented 2 years ago

What would you suggest as a name for the preprocessor symbol

That is a great question that I don't have a good answer for.. How about

I kinda like NET_PLATFORM_INDEPENDENT, it's long, but really self-explanatory.

I think what contributes to the issue in the example is that separate platform-specific source files are being mixed with conditional compilation.

My understanding is that this is how it's supposed to be done (code for each platform is separated in it's own file).

it would be better to put the Show and Dismiss declarations in a separate source file that is only compiled when not targeting one of the other OS's.

Good pint, but currently MAUI (<SingleProject> functionality) doesn't provide an easy way to do that. It only provides platform-spesific folders. So to do platform-agostic one, user would still need to do some #if gymnastic or ItemGroup with conditional include.

Off-topic: Hm, having a Platforms/Other folder with <SingleProject> out-of-the-box might be a good feature, and it would reduce the need in platform-agostic symbol, but having that symbol is still a good thing.

baronfel commented 2 years ago

As a design note, not all .NET SDK languages support partial classes (and as a result the multiple files with platform-specific code in each pattern), and so they would be using code like the samples @maxkoshevoi put forth. For that reason having a symbol could help cut down on the noise in a file. What about NET_ANY, keeping in line with the NET_<OS Platform> pattern so far?

maxkoshevoi commented 2 years ago

This might work: NET_ANY, NET6_0_ANY, NET6_0_ANY_OR_GREATER

KalleOlaviNiemitalo commented 2 years ago

The app can do #if NET_ANY && NET6_0_OR_GREATER so I don't see a need for NET6_0_ANY_OR_GREATER. Well, perhaps if someone wanted to use it in ConditionalAttribute, but why would one do that?

maxkoshevoi commented 2 years ago

The app can do #if NET_ANY && NET6_0_OR_GREATER

I don't think there is/was any non-versioned symbol (even NETSTANDARD had versions). While #if NET_ANY && NET6_0_OR_GREATER does make sense, #if ANDROID && NET6_0_OR_GREATER also does, but we still have NET6_0_ANDROID_OR_GREATER.

I'm not against it, just don't see a reason for this symbol to be inconsistent with all the others (all other symbols also have symbols with versions and _OR_GREATER).

dsplaisted commented 2 years ago

@Redth @mhutch @jonathanpeppers @terrajobst Any thoughts here on the idea of having a preprocessor symbol that's defined when targeting .NET without any specific target platform?

jonathanpeppers commented 2 years ago

Is there a reason this doesn't work?

<DefineConstants Condition="'$(TargetFramework)' == 'net6.0'">$(DefineConstants);NET_ANY</DefineConstants>

Then the iOS, Android, etc. target frameworks won't get the symbol.

If this was built-in, that might be useful? I'm not sure how many developers would use it, though.

mhutch commented 2 years ago

@jonathanpeppers in version-agnostic form it would be something like this, which isn't the kind of thing I'd expect folks to have to put in their project files:

<DefineConstants Condition="
  $([MSBuild]::IsTargetFrameworkCompatible('net6.0', '$(TargetFramework)')) And 
  '$([MSBuild]::GetTargetPlatformIdentifier($(TargetFramework)))'==''"
>$(DefineConstants);NET_ANY</DefineConstants>

The use case makes sense, but it's not obvious to me what the preprocessor symbol should be called. Ideally the name would make it very clear that it means "when not targeting a specific platform" rather than "when targeting any platform".

KalleOlaviNiemitalo commented 2 years ago

"Neutral" is another word that may be suitable.

Nirmal4G commented 2 years ago

I think "ANY" is short and best suitable but have seen some usages of the word "COMMON" too. Rarely, I found the word "CORE" also being used in this context.

maxkoshevoi commented 2 years ago

Found another place where such symbol would be helpful (I'll stop with examples after this one, promise 😅) https://github.com/jamesmontemagno/InAppBillingPlugin/blob/cfc43ec764aef4575d731e917db8c52595c89312/src/Plugin.InAppBilling/CrossInAppBilling.shared.cs#L36

mhutch commented 2 years ago

I like the idea of prefixing with PLATFORM e.g. PLATFORM_INDEPENDENT or PLATFORM_ANY. This would make it clearer that it's pivoting on the platform component of the TFM, rather than associating it with the other NET prefixed symbols that mostly pivot on framework version or framework "family". The other symbols for platform pivots don't have a NET prefix, or indeed a common prefix at all.

I have been informally calling the TFMs without a platform component the "platform-neutral TFM" so perhaps we could formalize this terminology and go with PLATFORM_NEUTRAL.

I also kinda like PORTABLE but it has some problematic associations (e.g. PCLs) so maybe is best avoided.

A few random thoughts

Nirmal4G commented 2 years ago

NET_PLATFORM_XXX is too long. Many here prefer NET_ANY. If this is not clear, then I vote for NET_COMMON.

mattleibow commented 2 years ago

Maybe the opposite is fine too?

Define NET_PLATFORM only on the platforms so if I wat a net6.0 only, then I do !NET_PLATFORM