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

Replace net6.0 TargetFramework Equality Conditions with msbuild intrinsic functions #43857

Merged
merged 4 commits into from
Oct 28, 2020
Merged

Replace net6.0 TargetFramework Equality Conditions with msbuild intrinsic functions #43857

merged 4 commits into from
Oct 28, 2020

Conversation

Anipik
Copy link
Contributor

@Anipik Anipik commented Oct 26, 2020

Working Towards #43646

The '$(TargetFramework)' == 'net6.0' conditions are meant for all platforms. This currently works because we remove the platform string from the tfm.
We will be no longer doing that hence we are changes the condition to $(TargetFramework.StartsWith('net6.0'))

I have done this for the != as well.

@ghost
Copy link

ghost commented Oct 26, 2020

Tagging subscribers to this area: @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

@safern
Copy link
Member

safern commented Oct 26, 2020

Are there any implications on not removing the platform part of the tfm from it and passing it down to the SDK?

Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should instead use these intrinsic functions: dotnet/msbuild#5171 (comment).

@Anipik
Copy link
Contributor Author

Anipik commented Oct 26, 2020

Are there any implications on not removing the platform part of the tfm from it and passing it down to the SDK?

There shouldnt be any, i have a blueprint which almost 90% of things going right.

@ViktorHofer
Copy link
Member

ViktorHofer commented Oct 26, 2020

Are there any implications on not removing the platform part of the tfm from it and passing it down to the SDK?

Not the SDK but one impact: we can finally revert dotnet/arcade@bd2a2b0 as we will then have distinct identities.

That said, if TargetPlatformMoniker won't be supported for older TFMs we might not be able to revert it yet.

@Anipik
Copy link
Contributor Author

Anipik commented Oct 26, 2020

I believe we should instead use these intrinsic functions: dotnet/msbuild#5171 (comment).

We should also remove the TargetsX properties as well, as we have now this support. any thoughts on moving towards that ?

TargetsX properties are wrong because they rely on the fact the tfm should be passed as global property. Removing this will help us to use property in projects and also use innerbuilds directly.

@ViktorHofer
Copy link
Member

ViktorHofer commented Oct 26, 2020

We should also remove the TargetsX properties as well, as we have now this support. any thoughts on moving towards that ?

Yes we should do that as well (either in this PR or a follow-up one) but only for >= net5.0 configurations. For older ones we probably need to use EndsWith(...) or TargetFrameworkSuffix.

@Anipik
Copy link
Contributor Author

Anipik commented Oct 26, 2020

Yes we should do that as well (either in this PR or a follow-up one) but only for >= net5.0 configurations. For older ones we probably need to use EndsWith(...) or TargetFrameworkSuffix.

I will throw up follow up for this one.

@Anipik Anipik changed the title Replace net6.0 TargetFramework Equality Conditions conditions with startsWith Replace net6.0 TargetFramework Equality Conditions conditions with msbuild intrinsic functions Oct 26, 2020
Anipik and others added 3 commits October 28, 2020 07:27
Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ViktorHofer ViktorHofer changed the title Replace net6.0 TargetFramework Equality Conditions conditions with msbuild intrinsic functions Replace net6.0 TargetFramework Equality Conditions with msbuild intrinsic functions Oct 28, 2020
@Anipik Anipik merged commit 3fca083 into dotnet:master Oct 28, 2020
@Anipik Anipik deleted the net6Condition branch October 28, 2020 18:09
Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we update any docs with this? I would imagine people to still use '$(TargetFramework)' != '$(NetCoreAppCurrent)'

@ViktorHofer
Copy link
Member

I would imagine people to still use '$(TargetFramework)' != '$(NetCoreAppCurrent)'

Which is fine as long as the TargetFramework isn't overloaded. There is room for improvement here as there are several projects that condition on "'$(TargetFramework)' == '$(NetCoreAppCurrent)' or $(TargetFramework.StartsWith('netcoreapp'))". We should clean those projects up. I'm unsure if we want documentation for this in our repo as it's feature that will be used in customer applications as well. Documentation of the intrinsic functions will be released with VS16.8.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants