-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[release/6.0] Fix stable builds for some transport packages #60635
Conversation
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.
Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer Issue DetailsSome 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.
|
@Anipik Any idea why this didn't show up in the servicing exercise? |
taking a look |
@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 |
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. |
I can help as well if needed. Which are the transport packages getting stable versions? |
|
I see that those packages set |
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 |
I'm pushing a fix to this PR as Anirudh is still away from his computer. |
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 Changed how we calculate the path to the aot package to use a private property rather than affecting the PS. I think we should port this cleanup to main. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. |
… workloads-testing.targets
ca7bb1b
to
015738c
Compare
Done. Thanks 😄 |
Just fyi, we do use this when testing locally too, eg when running |
Right, but only on test projects, so moving it to |
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.