removing $(OS) uses from the libraries

[Anipik]

Working towards #32451

[ViktorHofer]

@Anipik can you please share the motivation for doing this? Can you share your discussion with Eric?

[Anipik]

Earlier we didnt have a good way in msbuild to get the os we are running on. so we started using this shell property to get that.

We currently have 2 properties i.e. OSGroup and OS representing the same thing in our libraries subset. We can calculate the OSGroup without the need of the OS. We are already doing it for non-windows OSES https://github.com/dotnet/runtime/pull/32673/files#diff-d3bedba39ce6da3e37ca0db83d7ef035R30

cc @ericstj

[ViktorHofer]

Why not keep the $(OS) fallback here?

[ViktorHofer]

Are you doing this to switch to the RID layout and not using $(OS) at all anymore?

[Anipik]

Yes that's the final goal

[ViktorHofer]

Thanks, that makes sense. Two questions:

• Are you going to replace the OSGroup usages in dotnet/arcade with ([MSBuild]::IsOSPlatform('X'))? We should not leak that custom property into Arcade.
• We should really rename the OSGroup property name. It still confuses me, BuildOS would really have been a better choice.

[Anipik]

Are you going to replace the OSGroup usages in dotnet/arcade with ([MSBuild]::IsOSPlatform('X'))? We should not leak that custom property into Arcade.

No I am not changing the OS -> OSGroup as OSGroup is defined by runtime repo. However, I will remove OSGroup from the codeAnalysis part.

We should really rename the OSGroup property name. It still confuses me, BuildOS would really have been a better choice.

I can do that as part of this PR

[ViktorHofer]

No I am not changing the OS -> OSGroup as OSGroup is defined by runtime repo.

I think you misunderstood me. I was asking if you remove the OSGroup usages not the OS ones. It seems that OSGroup conditions should be replaceable with the MSBuild IsOSPlatofmr functions.

[Anipik]

I think you misunderstood me. I was asking if you remove the OSGroup usages not the OS ones. It seems that OSGroup conditions should be replaceable with the MSBuild IsOSPlatofmr functions.

yes we can do that in arcade.

[ViktorHofer]

yes we can do that in arcade.

Created dotnet/arcade#4907 to track this.

[Anipik]

@ViktorHofer BuildOS is causing some troubles. I am debugging it and we can make that change in the another pr.

@ViktorHofer @ericstj can you review this one ?

[ericstj]

Keep an eye on the rolling build when merging. Since this changes an entry-point property it's possible it has some differences between official/CI.

[Anipik]

sure I will keep an eye on those.