Skip to content
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

Upgrade System.Memory to fully fix another MSBuild loading issue #63545

Merged
merged 1 commit into from
Sep 19, 2022

Conversation

jasonmalinowski
Copy link
Member

@jasonmalinowski jasonmalinowski commented Aug 23, 2022

This works around dotnet/msbuild#7873

Fixes #63409 (along with #63527)

@jasonmalinowski jasonmalinowski requested review from a team as code owners August 23, 2022 21:41
@jasonmalinowski jasonmalinowski self-assigned this Aug 23, 2022
@jaredpar
Copy link
Member

Has mantis signed off on this since it can impact VS load?

@chsienki

@chsienki
Copy link
Contributor

Presumably it's already being loaded, so shouldn't make a difference, but might want to do a val build just to be sure.

@jasonmalinowski
Copy link
Member Author

I see System.Memory being referenced by Roslyn.VisualStudio.Setup.Dependencies.csproj which implies we just pick up whatever VS has inserted. I also see it being added to the ExpressionEvaluatior ResultProvider, but I don't know what that means.

I suspect that means this can't break much, which is a great reason to run a validation build. 😄

@jasonmalinowski
Copy link
Member Author

@chsienki So looking at the VS build I triggered for this, it appears VS is already using the newer version. Naturally, this means this upgrade still breaks the VS build itself for reasons I need to investigate further.

@JoeRobich
Copy link
Member

Stinks that projects which reference both M.CA and NuGet libs would now end up with a version mismatch. A lot of System.Memory 4.5.4 deps out there. =(

@dotnet dotnet deleted a comment from azure-pipelines bot Sep 19, 2022
@jasonmalinowski
Copy link
Member Author

The internal build is now passing, so @dotnet/roslyn-infrastructure or @dotnet/roslyn-ide could I get a signoff on this?

@jasonmalinowski jasonmalinowski merged commit 4b85339 into dotnet:main Sep 19, 2022
@jasonmalinowski jasonmalinowski deleted the fix-lsif-indexing branch September 19, 2022 22:10
@ghost ghost added this to the Next milestone Sep 19, 2022
@Cosifne Cosifne modified the milestones: Next, 17.4 P3 Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LSIF tool backward incompatibility
6 participants