-
Notifications
You must be signed in to change notification settings - Fork 57
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
Update runtime dependency packages to latest from maintenance-packages #1068
Update runtime dependency packages to latest from maintenance-packages #1068
Conversation
Per the instructions for adding new packages: Add DependencyPackageProjects for all new projects in the eng/Build.props in the correct dependency order. |
Done. Sorry, I missed that line in the instructions. |
eng/Build.props
Outdated
@@ -36,6 +37,11 @@ | |||
<DependencyPackageProjects Include="$(RepoRoot)src\referencePackages\src\**\Nuget.Commands.6.11.0.csproj" /> | |||
<DependencyPackageProjects Include="$(RepoRoot)src\referencePackages\src\**\System.Text.Json.8.0.5.csproj" /> | |||
<DependencyPackageProjects Include="$(RepoRoot)src\textOnlyPackages\src\**\Microsoft.NetCore.Platforms.3.1.0.csproj" /> | |||
<DependencyPackageProjects Include="$(RepoRoot)src\referencePackages\src\**\System.Buffers.4.6.0-preview.1.24529.4.csproj" /> | |||
<DependencyPackageProjects Include="$(RepoRoot)src\referencePackages\src\**\System.Memory.4.6.0-preview.1.24529.4.csproj" /> |
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.
System.Memory depends on Buffers, Vectors and Unsafe and therefore must be listed after those.
I noticed Bcl.AsyncInterfaces is not added to Build.props, is that expected? (I did not add that one, it was already there). It depends on System.Threading.Tasks.Extensions, which I did add (although it depends on an older version). |
Oh - We only add to DependencyPackageProjects those projects that are dependency of others. We don't add all that we add. There are many other packages in the referencePackages folder that are not in Build.props. |
This is a long story which I won't go into details here. dotnet/source-build#1690 tracks the work to which will eliminate the need for DependencyPackageProjects. This is currently blocked on dotnet/source-build#4482. We hope to have this work done in the 10.0 timeframe. The requirement is to add new projects to the DependencyPackageProjects. The projects must be defined in the correct dependency order. All new projects should be appended at the end of the list. Once a package is added to the previous source build artifacts the DependencyPackageProjects can be cleaned up which is why it is a partial list. |
Oh ok got it. Then I'll re-add the one I just removed, Bcl.HashCode. |
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.
LGTM.
The only thing that caught my eye was the preview versions, but I see that using previous versions here was approved by @MichaelSimons (see dotnet/maintenance-packages#140 (comment)) and also mentioned in the PR description.
This is all going into main, we want to test these preview versions in runtime. I am currently in the process of stabilizing the m-p repo to generate the official GA versions of these packages. If needed, I'll be happy to generate the reference packages for GA too. |
Unblocks dotnet/runtime#108806
We have generated new preview versions of some OOB packages that have been recently migrated to the maintenance-packages repo. We need to include them in sbrp so that we can update runtime's
main
dependencies to those preview versions without breaking source build.The packages that we want to update in runtime are:
I used the generate.sh script to generate these files, per the instructions.
For additional details, see the decided plan here: dotnet/maintenance-packages#140 (comment)
cc @ericstj @ViktorHofer @MichaelSimons