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

Update LSIF generator to latest bin log reader #67085

Merged

Conversation

gundermanc
Copy link
Member

@gundermanc gundermanc commented Feb 28, 2023

Roslyn LSIF generator chokes on 'version 16' bin logs. This PR updates to v16 so that it can properly generate LSIF for codebases/build-environments using newer SDKs.

Fixes AB#1743537

@gundermanc gundermanc requested a review from a team as a code owner February 28, 2023 04:58
@ghost ghost added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Feb 28, 2023
@gundermanc
Copy link
Member Author

Tagging @jasonmalinowski for a review.

Do we expect the binlog version to change frequently? Is there a way that we can make this process a bit more automated and/or coordinated with the MSBuild team to minimize the period of time that a new toolset is unsupported?

@gundermanc gundermanc force-pushed the dev/chgund/UpdateStructuredBinLog branch 2 times, most recently from 97d699c to 5095f5f Compare February 28, 2023 17:18
@jasonmalinowski jasonmalinowski self-assigned this Feb 28, 2023
@@ -11,6 +11,7 @@
<ItemGroup>
<Reference Include="System" />
<Reference Include="System.Core" />
<Reference Include="System.IO.Compression" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😕 Was this change required?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it seems that ZipArchive was moved from one DLL to another and this generates a new compiler warning without the reference.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might indicate a packaging issue with whatever had the demand -- that could have been listed in the NuGet package itself. Not a big deal though.

@gundermanc gundermanc force-pushed the dev/chgund/UpdateStructuredBinLog branch from 5095f5f to 3d7fca6 Compare February 28, 2023 19:32
@gundermanc gundermanc requested review from sharwell and jasonmalinowski and removed request for sharwell February 28, 2023 19:33
@jasonmalinowski jasonmalinowski merged commit 1a642d9 into dotnet:main Feb 28, 2023
@ghost ghost added this to the Next milestone Feb 28, 2023
@RikkiGibson RikkiGibson modified the milestones: Next, 17.6 P2 Mar 2, 2023
@genlu genlu mentioned this pull request Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Infrastructure Community The pull request was submitted by a contributor who is not a Microsoft employee.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants