Remove workarounds for bugs in Microsoft.VisualStudio.Extensibility.JsonGenerators.Sdk #79182
Conversation
This allows us to remove our workaround for https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_workitems/edit/2471514/
This seems to build fine now, I'm guessing it was fixed?
| <PackageVersion Include="Microsoft.VisualStudio.Extensibility.JsonGenerators.Sdk" Version="17.12.2037-preview3" /> | ||
| <PackageVersion Include="Microsoft.VisualStudio.Extensibility.Sdk" Version="18.0.52-preview1" /> | ||
| <PackageVersion Include="Microsoft.VisualStudio.Extensibility" Version="18.0.52-preview1" /> | ||
| <PackageVersion Include="Microsoft.VisualStudio.Extensibility.JsonGenerators.Sdk" Version="18.0.52-preview1" /> |
There was a problem hiding this comment.
My initial hope was to just be able to bump this specific package version, but the generator itself has an assertion that the version in this must match the of the Extensibility contracts being used. This means that we can't take a new version of the generator to workaround a bug without also implicitly bumping which versions of VS we're targeting. So I'm putting this in main-vs-deps which can take the bump just fine.
@matteo-prosperi just an FYI but this might become a problem at some point. It's been very common over the years that Roslyn needs to take bumps to the build tools to get fixes there, but can't necessarily take a bump on what version we're targeting. I imagine customers wanting to target older versions might also run into challenges here. I can absolutely understand having the SDK tooling older than the targeted contracts is bad and doesn't make sense, but it'd seem the other way around should be safe to do?
There was a problem hiding this comment.
I can see us removing the strict requirement of versions matching and using instead a major+minor version match only.
This would only change things for internal teams since we only publish one version of these packages to nuget.org for each VS minor version. But it would allow us to patch a minor version of Microsoft.VisualStudio.Extensibility.JsonGenerators.Sdk if some issue is found.
Some sort of alignment between Microsoft.VisualStudio.Extensibility.JsonGenerators.Sdk and Microsoft.VisualStudio.Extensibility.Contracts is required since the generator must be able to interpret the configuration types defined in .Contracts.
There was a problem hiding this comment.
Some sort of alignment between Microsoft.VisualStudio.Extensibility.JsonGenerators.Sdk and Microsoft.VisualStudio.Extensibility.Contracts is required since the generator must be able to interpret the configuration types defined in .Contracts
So I can absolutely imagine that the generator needs to be "newer" than the contracts, since otherwise it won't know how to interpret new types of things. But as long as the generator is newer than the contracts, I'd imagine you'd be fine, unless over time how an expression is generated might change for newer VS targets. (But I'd assume that'd be rare.)
There was a problem hiding this comment.
I discovered this file while deleting the other workaround. I was able to build just fine, so probably safe to delete? 🤷
There was a problem hiding this comment.
Seems like it was added just for that workaround in the Json generators nuget package, seems safe to get rid off from history at least.
No description provided.