-
Notifications
You must be signed in to change notification settings - Fork 59
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
Add Microsoft.CodeAnalysis/4.9 dependencies #870
Conversation
... necessary to consume new Roslyn dependencies. Roslyn recently updated to 8.0.0 dependencies which makes this change necessary. Unblocks dotnet/runtime#97152
Undoing changes because of issues in SBRP / GenAPI for which I filed the following issues:
cc @ericstj |
How is this intended to resolve dotnet/runtime#97152? Those changes aren't even trying to use 4.10.0. |
Fixed the title. It's roslyn4.9 that brings the 8.0.0 packages in. Remember, you helped with that investigation. |
eng/Build.props
Outdated
<DependencyPackageProjects Include="$(RepoRoot)src\referencePackages\src\**\System.Composition.TypedParts.8.0.0.csproj" /> | ||
<DependencyPackageProjects Include="$(RepoRoot)src\referencePackages\src\**\System.Composition.Runtime.8.0.0.csproj" /> | ||
<DependencyPackageProjects Include="$(RepoRoot)src\referencePackages\src\**\System.Composition.Hosting.8.0.0.csproj" /> | ||
<DependencyPackageProjects Include="$(RepoRoot)src\referencePackages\src\**\System.Composition.Convention.8.0.0.csproj" /> | ||
<DependencyPackageProjects Include="$(RepoRoot)src\referencePackages\src\**\System.Composition.AttributedModel.8.0.0.csproj" /> | ||
<DependencyPackageProjects Include="$(RepoRoot)src\referencePackages\src\**\System.IO.Pipelines.8.0.0.csproj" /> | ||
<DependencyPackageProjects Include="$(RepoRoot)src\referencePackages\src\**\System.Text.Encoding.CodePages.8.0.0.csproj" /> | ||
<DependencyPackageProjects Include="$(RepoRoot)src\referencePackages\src\**\System.Threading.Channels.8.0.0.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.
These need to be sorted in dependency order. For example, I see that System.Composition.TypedParts
depends on System.Composition.Hosting
.
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 thought I added them in sorted order. If the order is wrong, shouldn't I get a build or prebuilt error?
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.
It is documented in the readme
Add DependencyPackageProjects for all new projects in the eng/Build.props in the correct dependency order.
Unfortunately you won't get a build error or prebuilts reported. This is why we need to get project to project references defined so that these DependencyPackageProjects can be removed.
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.
Right, I followed those instructions but looks like I got something in the order wrong. Without build validation or someone telling me that the order is wrong, I would have never noticed that.
... necessary to consume new Roslyn dependencies. Roslyn recently updated to 8.0.0 dependencies which makes this change necessary.
Unblocks dotnet/runtime#97152