[release/6.0] Fix stable builds for some transport packages

[mmitche]

Some non-shipping transport and sources packages were getting incorrectly getting stable versions. This didn't show up in the servicing excercise, though they probably did the right thing for unrelated reasons. My guess is that a number of the .proj files that should not be stabilized used to be pkgproj files. The conditional only covered pkgproj.

[ghost]

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

Issue Details

---

Some non-shipping transport and sources packages were getting incorrectly getting stable versions. This didn't show up in the servicing excercise, though they probably did the right thing for unrelated reasons. My guess is that a number of the .proj files that should not be stabilized used to be pkgproj files. The conditional only covered pkgproj.

Author:	mmitche
Assignees:	-
Labels:	area-Infrastructure-libraries
Milestone:	-

[mmitche]

@Anipik Any idea why this didn't show up in the servicing exercise?

[Anipik]

@Anipik Any idea why this didn't show up in the servicing exercise?

taking a look

[mmitche]

@Anipik This doesn't fix the issue: https://dev.azure.com/dnceng/internal/_build/results?buildId=1430080&view=results. Can I hand this off to you?

[Anipik]

@Anipik This doesn't fix the issue: https://dev.azure.com/dnceng/internal/_build/results?buildId=1430080&view=results. Can I hand this off to you?

currentlt i dont have access to the machine, i will be able to take look in 30-45 mins

[ericstj]

Yeah, this props file is only used by coreclr subset, you were seeing issue with transport packages from the libs subset. The problem observed is that non-shipping packages should not get stable versions. So we need to see why these are getting stable versions and change that.

[safern]

I can help as well if needed. Which are the transport packages getting stable versions?

[mmitche]

D:\a\1\s\.packages\microsoft.dotnet.arcade.sdk\7.0.0-beta.21518.6\tools\SdkTasks\PublishArtifactsInManifest.proj(139,5): error : Package 'Microsoft.Extensions.HostFactoryResolver.Sources' has stable version '6.0.0' but is targeted at a non-isolated feed 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet6-transport/nuget/v3/index.json'
D:\a\1\s\.packages\microsoft.dotnet.arcade.sdk\7.0.0-beta.21518.6\tools\SdkTasks\PublishArtifactsInManifest.proj(139,5): error : Package 'Microsoft.Internal.Runtime.AspNetCore.Transport' has stable version '6.0.0' but is targeted at a non-isolated feed 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet6-transport/nuget/v3/index.json'
D:\a\1\s\.packages\microsoft.dotnet.arcade.sdk\7.0.0-beta.21518.6\tools\SdkTasks\PublishArtifactsInManifest.proj(139,5): error : Package 'Microsoft.Internal.Runtime.WindowsDesktop.Transport' has stable version '6.0.0' but is targeted at a non-isolated feed 'https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet6-transport/nuget/v3/index.json'

[safern]

I see that those packages set IsShipping=false in their project file, so let me look at a binlog and see why they are getting stable versions.

[Anipik]

found the issue. Fixing it, the problem was introduced here in stable rtm pr https://github.com/dotnet/runtime/pull/60294/files#diff-10fcb9beea2f7da7fa953442dab4573d0a144be5adb61f7c750ceed9041424f9R3

this is causing issues

[safern]

I'm pushing a fix to this PR as Anirudh is still away from his computer.

[safern]

I've fixed it with: ca7bb1b

@Anipik @ViktorHofer @lewing @radical please take a look.

I don't think we should include workloads-testing.targets on all of src\libraries\Directory.Build.props tree as those are only intended to install workloads for testing when sending to helix. Also these are only intended/needed for mobile testing, so I moved the import to tests.mobile.targets and also cleaned up a little bit how we set some properties.

Changed how we calculate the path to the aot package to use a private property rather than affecting the PackageVersion property that defines the final version of the produced package in case the targets are included in a package evaluation.

PS. I think we should port this cleanup to main.

[ViktorHofer]

I agree with your assessment and the clean-up looks right but at this point of the 6.0 release I would prefer a more targeted fix to reduce risk. Is that possible?

[safern]

I agree with your assessment and the clean-up looks right but at this point of the 6.0 release I would prefer a more targeted fix to reduce risk. Is that possible?

Sure. I can just rename the property to be a "private" property and then do this cleanup on main.

[safern]

Sure. I can just rename the property to be a "private" property and then do this cleanup on main.

Done. Thanks 😄

[radical]

I don't think we should include workloads-testing.targets on all of src\libraries\Directory.Build.props tree as those are only intended to install workloads for testing when sending to helix. Also these are only intended/needed for mobile testing, so I moved the import to tests.mobile.targets and also cleaned up a little bit how we set some properties.

Just fyi, we do use this when testing locally too, eg when running Wasm.Build.Tests. And currently this is used only by wasm, but it will likely get used for other mobile cases too - iOS/android etc.

[safern]

Just fyi, we do use this when testing locally too, eg when running Wasm.Build.Tests. And currently this is used only by wasm, but it will likely get used for other mobile cases too - iOS/android etc.

Right, but only on test projects, so moving it to tests.mobile.targets make sense IMO.