Adding target platform preprocessor symbols#11635
Conversation
|
@dsplaisted this is my first pass at adding preprocessor symbols. I haven't included backwards compatibility for target platforms yet, since a way to check for supported target platforms hasn't been implemented yet (afaik). |
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
Outdated
Show resolved
Hide resolved
| <_NetCoreAppFrameworkVersions Include="@(SupportedNETCoreAppTargetFramework->'%(Identity)'->TrimStart('.NETCoreApp,Version=v'))" /> | ||
| <_NetCoreAppCompatibleVersions Include="@(_NetCoreAppFrameworkVersions)" Condition=" $([MSBuild]::VersionGreaterThan(%(Identity), 3.1)) and $([MSBuild]::VersionLessThan(%(Identity), $(TargetFrameworkVersion))) " /> |
There was a problem hiding this comment.
This is a nice approach to this problem! But since it has to happen on every build, it's making me think about having some precomputation at SDK build time for faster execution time. We can keep that in our back pocket for the future.
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
Outdated
Show resolved
Hide resolved
|
@dsplaisted do you have any more feedback here? I've updated the issue to track the other half of this work, which I'll make in a separate PR. |
dsplaisted
left a comment
There was a problem hiding this comment.
I would kind of like one test that tests the compilation constants end to end. IE compile an app with the following code and verify that you get the expected output when you run it:
#if NETCOREAPP
Console.WriteLine("NETCOREAPP");
#endif
#if NETCOREAPP3_1
Console.WriteLine("NETCOREAPP3_1");
#endif
#if NET
Console.WriteLine("NET");
#endif
#if NET5_0
Console.WriteLine("NET5_0");
#endif
// etc.The tests that expect WINDOWS to be defined will need to be updated once we fix that issue, though we don't need to block this PR on that.
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
Outdated
Show resolved
Hide resolved
src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.BeforeCommon.targets
Show resolved
Hide resolved
| itemGroup.Add(supportedFramework); | ||
| }); | ||
|
|
||
| AssertDefinedConstantsOutput(testAsset, targetFramework, new[] { "NETCOREAPP", "NET", "WINDOWS", "WINDOWS7_0" }.Concat(expectedDefines).ToArray()); |
There was a problem hiding this comment.
This is a problem we didn't realize: The common targets set the TargetPlatformIdentifier to Windows if it's not already set. This is not what we planned for, so we'll have to figure out how to address this.
There was a problem hiding this comment.
Yup, I noticed that while implementing this. Once we add the condition for including those defaults I'll have to come back and change this test (remove the WINDOWS assertions in this line).
|
What about the "OS flavored" preprocessors like NET6_0_WINDOWS. I don´t get these to work. When will these implemented? |
@Maxyeah There are separate symbols defined for the OS. So you can do something like |
|
Ok thanks, but what are the combined prepocessors for? |
I'm not sure what you mean. If you're talking about something like |
Adds the first half of #11236
Fixes #11544