-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Move package publishing out of Roslyn official build #80009
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
base: main
Are you sure you want to change the base?
Conversation
71d8ef8 to
5568e5a
Compare
9582cdf to
e13f253
Compare
9c6ce75 to
fb7e019
Compare
| "Microsoft.CodeAnalysis.LanguageServer.win-x64": "vs-impl", | ||
| "Microsoft.CodeAnalysis.LanguageServer.win-x86": "vs-impl" | ||
| }, | ||
| "packages": { |
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.
servicing branches still rely on this existing in main. Post this change, I plan on backporting the part of this change to pull from the current branch, and we can delete this from main.
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.
Extracting out the branch specific publish data to #80308 so its easier to backport.
| "vs": "https://devdiv.pkgs.visualstudio.com/_packaging/VS/nuget/v3/index.json" | ||
| }, | ||
| "packageFeedOverride": { | ||
| "Microsoft.CodeAnalysis.LanguageServer.alpine-arm64": "vs-impl", |
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.
We want to be able to directly insert the language server into vscode from the official build, so we keep publishing these to vs-impl (can't easily pull from the devdiv/VS feed).
| </ItemGroup> | ||
|
|
||
| <ItemGroup Condition=" '$(PreReleaseVersionLabel)' == 'pr-validation' "> | ||
| <ItemGroup Condition=" '$(DotNetBuildFromVMR)' != 'true' "> |
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.
disables darc publishing of packages, but keeps publishing symbols.
c2ab0a4 to
aba8f6f
Compare
docs/wiki/NuGet-packages.md
Outdated
|
|
||
| SDK GA releases align with VS Minor versions. So going from `11.0.100` to `11.0.200` means a new VS Minor version, hence incrementing the Roslyn package minor version. | ||
|
|
||
| SDK prereleases will not always align with a new VS Minor version however. So while `11.0.100-preview.4` might produce Roslyn `5.6.0-preview.4`, `11.0.100-preview.5` could produce Roslyn `5.6.0-preview5` with the same minor version. |
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.
This is particularly true between November and February when there are no prereleases.
b66640b to
3cf8602
Compare
|
|
||
| Prior to this change, a Roslyn package was matched exactly to a VS version, and did not exactly match SDK versions. Now, Roslyn packages will exactly align with an SDK version, but will no longer exactly match a VS version. While generally the SDK releases ship alongside new VS versions, there may be drift in the exact commit of Roslyn when compared to VS (especially for prereleases). | ||
|
|
||
| ### Version Schema |
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.
PTAL at these version changes. The versions will mostly be the same for stable packages. Preview packages will have build numbers. I've tried to explain the logic below but let me know if it needs more info.
docs/wiki/NuGet-packages.md
Outdated
|
|
||
| SDK GA releases align with VS Minor versions. So going from `11.0.100` to `11.0.200` means a new VS Minor version, hence incrementing the Roslyn package minor version. | ||
|
|
||
| However, SDK prereleases do not always correspond to new VS Minor or VS preview versions. For example, while `11.0.100-preview.4` might produce Roslyn `5.6.0-2-25465.101`, the subsequent `11.0.100-preview.5` could produce Roslyn `5.6.0-2-25503.114` without any change to the VS minor or preview version. In these cases, we need to include the VMR build number to differentiate between packages. This typically occurs with early SDK versions or during extended VS preview cycles (such as preview 1 for a new VS major version). |
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.
alternative here is setting our prerelease label to the SDK prerelease and manually incrementing for SDK releases. But that seems a bit more complicated due to us branching for VS, and not the SDK.
don't produce release packages in roslyn build.
3cf8602 to
5d5a50a
Compare
| <UsingToolVSSDK Condition="$([MSBuild]::IsOSPlatform('Windows'))">true</UsingToolVSSDK> | ||
| <UsingToolPdbConverter>false</UsingToolPdbConverter> | ||
| <UsingToolSymbolUploader>true</UsingToolSymbolUploader> | ||
| <UsingToolNuGetRepack>true</UsingToolNuGetRepack> |
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.
this was creating the 3 copies of all our packages in our official build. we don't need it now that vmr is producig them.
|
@JoeRobich @333fred @jaredpar @mmitche would appreciate some of your eyes on this |
| * C = Our own patch version if we need to service the package for any reason, generally `0`. | ||
| * D = The VS prerelease version `-` VMR build version (if any). | ||
|
|
||
| We still use VS versions in our version scheme as Roslyn currently branches for releases with VS in mind. |
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'm worried about this, given that we don't do the shipping any more. Since SDK ship dates do not line up with VS, I'm concerned that some 5.X.Y version will be shipped as release, but then VS 18.X will release at a later date and contain a breaking change from when the supposed corresponding Roslyn package was released.
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'm concerned that some 5.X.Y version will be shipped as release, but then VS 18.X will release at a later date and contain a breaking change from when the supposed corresponding Roslyn package was released.
Yeah it is definitely possible to have a breaking change ship in a VS releases that isn't present in the SDK we released before (since the SDK ships a bit sooner than VS). But a couple considerations here
- The reverse is true today - VS and our packages could ship a breaking change that is not compatible with the most recent SDK. But I don't think we have hit that recently.
- There is a limited amount of time where we can ship a breaking change that will not be picked up in the SDK. For example we're snapping for VS 18.0 next week on Monday, at the same time as the SDK for RC2. There's very little time to get a change into VS that wouldn't be in the SDK. Any other breaking change would need to go into the Roslyn release branch which should have extra scrutiny.
- As long as we haven't shipped a binary breaking change, VS will still load the VS versions of the packages (like what happens today)
We definitely should be careful about making breaking changes very late into the SDK release though, as it isn't impossible. Need to do our best to make sure VS either won't break or has the change.
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.
The reverse is true today - VS and our packages could ship a breaking change that is not compatible with the most recent SDK. But I don't think we have hit that recently.
Sure, but we're not versioned according to SDK releases. We're versioned according to VS releases. IMO, it's a worse break in this direction.
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.
Sure, but we're not versioned according to SDK releases. We're versioned according to VS releases
This is true - I'd prefer if our package version was tied to the SDK version when it comes out of the VMR. But a couple of thoughts
a. This is something we can always change later if we decide that aligning our version number to the SDK version is important.
b. If we are to do something, I'd prefer to investigate that with the .NET 11 release as it'll give us more time to figure out how we can fit SDK versioning into our VS branch structure (which is also kind of unknown, see below). For 10 GA, we keep a similar version format and VS compatibility story.
IMO, it's a worse break in this direction.
Think it depends on exactly what the break is, but I could see that. But to hit this
a) we have to actually manage to insert a breaking change (/ new API) into VS but not the SDK (as above)
b) be using the tip of Roslyn and not referencing older versions for older VS/SDK compatibility
c) we are unable to service the package in the SDK to resolve the breaking change.
I will say if the VS ship schedule changes to ship faster (and therefore not align with SDK releases), we may have to revisit this somewhat in order to ensure GA packages are available that contain recent enough VS changes. I'm not entirely convinced that would be necessary though (see below). If we do though, it should be possible to do out of the VMR (if we accept minor commit drift).
| ## Versioning | ||
| Starting on Roslyn versions `5.0` and above, Roslyn NuGet packages are shipped with the .NET SDK instead of VS. New Roslyn packages will be published on every SDK preview and GA release. | ||
|
|
||
| Prior to this change, a Roslyn package was matched exactly to a VS version, and did not exactly match SDK versions. Now, Roslyn packages will exactly align with an SDK version, but will no longer exactly match a VS version. While generally the SDK releases ship alongside new VS versions, there may be drift in the exact commit of Roslyn when compared to VS (especially for prereleases). |
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.
Historically this seems to be ok as new feature band SDK releases corresponded with VS minor version releases (see chart). If the VS ship schedule accelerates, we may find ourselves needing to ship new release packages between when new feature band SDKs are released.
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 possible that will be necessary, but unless the SDK feature band schedule slows down dramatically, I don't necessarily think we need to do that.
The main issue would be if we add or break an API in a new version of VS, to use it there you have to wait for new GA packages. But breaking changes at least should be rare, and new APIs are not that frequent plus someone needs to need the API (and not be referencing old versions of Roslyn to keep things working in old VS/SDKs).
But yes - that could definitely mean we need to GA faster, but I can't work against an unknown.
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.
@marcpopMSFT this is an interesting case to think about with the SDK ship strategy. Don't think it changes plans but it is something to keep in mind.
The main issue would be if we add or break an API in a new version of VS, to use it there you have to wait for new GA packages.
If we add a new API then yes it could delay a 3rd party extension taking advantage of that in VS. How often does that happen today though? Usually at least for analyzers there is a bit of a delay in new API adoption because if you adopted it immediately then you would break in 90+% of SDK that didn't have it.
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.
Here's a table of how often our public API files have changed. It only includes our changes, and not say a VS dependency API upadating requiring our package to update, but hopefully at least gives an idea.
https://gist.github.com/dibarbet/f836c80e03935b65c230440e11104c9d
Most frequent are changes to the compiler public APIs. VS/Workspaces/Features/Editor features do not change frequently.
And agreed, generally targeting a lower version of Roslyn is ideal so you can work in older VS and SDK versions anyway.
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.
filtered out anything that hasn't changed this year and also external access, leaves this set
| File Path | Last Updated | Updates Last 3 Months | Updates Last 6 Months | Avg Updates Per Month |
|---|---|---|---|---|
| src/CodeStyle/Core/Analyzers/PublicAPI.Unshipped.txt | 2025-06-17 | 0 | 2 | 0.31 |
| src/Compilers/Core/Portable/PublicAPI.Shipped.txt | 2025-02-20 | 0 | 0 | 0.46 |
| src/Compilers/Core/Portable/PublicAPI.Unshipped.txt | 2025-09-17 | 9 | 19 | 9.89 |
| src/Compilers/CSharp/Portable/PublicAPI.Shipped.txt | 2025-07-24 | 1 | 1 | 0.34 |
| src/Compilers/CSharp/Portable/PublicAPI.Unshipped.txt | 2025-08-21 | 3 | 9 | 4.99 |
| src/Compilers/VisualBasic/Portable/PublicAPI.Shipped.txt | 2025-02-20 | 0 | 0 | 0.25 |
| src/Compilers/VisualBasic/Portable/PublicAPI.Unshipped.txt | 2025-02-20 | 0 | 0 | 0.93 |
| src/Features/Core/Portable/PublicAPI.Unshipped.txt | 2025-04-02 | 0 | 1 | 0.55 |
| src/RoslynAnalyzers/Microsoft.CodeAnalysis.AnalyzerUtilities/PublicAPI.Shipped.txt | 2025-02-07 | 0 | 0 | 0 |
| src/RoslynAnalyzers/Microsoft.CodeAnalysis.AnalyzerUtilities/PublicAPI.Unshipped.txt | 2025-03-14 | 0 | 0 | 2 |
| src/Workspaces/Core/Portable/PublicAPI.Shipped.txt | 2025-02-20 | 0 | 0 | 0.28 |
| src/Workspaces/Core/Portable/PublicAPI.Unshipped.txt | 2025-09-10 | 3 | 6 | 2.75 |
| src/Workspaces/MSBuild/BuildHost/PublicAPI.Shipped.txt | 2025-01-20 | 0 | 0 | 0 |
| src/Workspaces/MSBuild/BuildHost/PublicAPI.Unshipped.txt | 2025-01-20 | 0 | 0 | 0 |
| src/Workspaces/MSBuild/Core/PublicAPI.Shipped.txt | 2025-01-20 | 0 | 0 | 0 |
| src/Workspaces/MSBuild/Core/PublicAPI.Unshipped.txt | 2025-01-20 | 0 | 0 | 0 |
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 looks like there are failures in the linked VMR builds, is that expected?
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.
yeah the errors are unrelated - the windows job passed and created the correct artifacts
| Note: this package is intended as a replacement for Microsoft.Net.Compilers (which is a Windows-only package) and Microsoft.NETCore.Compilers. Those packages are now deprecated and will be deleted in the future. | ||
|
|
||
| ## Versioning | ||
| Starting on Roslyn versions `5.0` and above, Roslyn NuGet packages are shipped with the .NET SDK instead of VS. New Roslyn packages will be published on every SDK preview and GA release. |
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.
The title of the PR says "move package publishing out of Roslyn official builds" but as far as I understand, we already publish Roslyn in the VMR today (resulting in .1xx versions). We also publish it from Roslyn official builds (.x versions). So the PR is more about just removing the latter, right?
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.
Generally yes - and adjusting the version slightly to ensure VMR can produce non-conflicting stable packages.
eng/Versions.props
Outdated
| <MinorVersion>0</MinorVersion> | ||
| <PatchVersion>0</PatchVersion> | ||
| <PreReleaseVersionLabel>2</PreReleaseVersionLabel> | ||
| <PreReleaseVersionLabel>2$(_RoslynPackageSuffix)</PreReleaseVersionLabel> |
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.
Maybe pull the 2 out into a separate property (_RoslynPrereleaseVersion) so we are not trying to update it here.
| |---|---|---|---|---| | ||
| | Roslyn version | 5.6.0-2.25465.10 | 5.8.0-3.25501.82 | 5.9.0 | 5.10.0 | | ||
|
|
||
| SDK GA releases align with VS Minor versions. So going from `11.0.100` to `11.0.200` means a new VS Minor version, hence incrementing the Roslyn package minor version. |
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.
Just to sanity check: this is an absolutely guarantee, not just something that has happened to work in practice?
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.
from what I have been told, yes, feature bands intentionally line up with VS versions. though I cannot say what the future holds
| <!-- All packages produced by the Roslyn official build should be suffixed with '-vs' to distinguish them from VMR produced packages --> | ||
| <_RoslynPackageSuffix Condition="'$(DotNetBuildFromVMR)' != 'true'">-vs</_RoslynPackageSuffix> | ||
|
|
||
| <MajorVersion>5</MajorVersion> | ||
| <MinorVersion>0</MinorVersion> | ||
| <PatchVersion>0</PatchVersion> | ||
| <PreReleaseVersionLabel>2</PreReleaseVersionLabel> | ||
| <_RoslynPreReleaseVersion>2</_RoslynPreReleaseVersion> | ||
| <PreReleaseVersionLabel>$(_RoslynPreReleaseVersion)$(_RoslynPackageSuffix)</PreReleaseVersionLabel> |
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.
You may want to adjust the naming and comments here to make it clear this is just the version being suffixed, not the package itself.
| Note: this package is intended as a replacement for Microsoft.Net.Compilers (which is a Windows-only package) and Microsoft.NETCore.Compilers. Those packages are now deprecated and will be deleted in the future. | ||
|
|
||
| ## Versioning | ||
| Starting on Roslyn versions `5.0` and above, Roslyn NuGet packages are shipped with the .NET SDK instead of VS. New Roslyn packages will be published on every SDK preview and GA release. |
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.
In practice we've had challenges when we consume new VS/editor packages, since those sometimes aren't available until a VS release, and those can take a few days to get posted. Does moving this to SDK releases make anything more challenging there?
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.
Yes and no. We still need to publish manually because we may have to update for new VS dependencies. If VS releases don't align with the SDK and release slower the SDK there is the possibility of us having to wait for a VS release to publish packages. Can happen today for prereleases for new major versions
| <!-- All packages produced by the Roslyn official build should be suffixed with '-vs' to distinguish them from VMR produced packages --> | ||
| <_RoslynPackageSuffix Condition="'$(DotNetBuildFromVMR)' != 'true'">-vs</_RoslynPackageSuffix> |
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.
And the -vs versions are still what are being inserted into VS? Do we have any cases where other packages are produced in the VS tree, and will end up with dependencies depending on these VS versions?
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.
And the -vs versions are still what are being inserted into VS?
Yes
Do we have any cases where other packages are produced in the VS tree, and will end up with dependencies depending on these VS versions?
Not public packages as far as I have been able to tell. If they are, they would switch to reference the VMR produced version.
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.
Not public packages as far as I have been able to tell. If they are, they would switch to reference the VMR produced version.
Is that something we somehow need to add validation to ensure? Because I imagine we'll discover that only after some -vs package got published and folks come to us wondering where it is.
Roslyn side of the changes to publish out of the VMR instead of our official build. This part essentially stops the package publishing (and creation of shipping packages), as well as adjusts versioning slightly to produce packages that we can ship on SDK releases.
VMR side change - dotnet/dotnet#2449
Resolves #78299
Resolves #78300
VMR builds with these changes
stable prerelease - https://dev.azure.com/dnceng/internal/_build/results?buildId=2794951&view=results
stable release - https://dev.azure.com/dnceng/internal/_build/results?buildId=2794953&view=results
Roslyn validation
https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=12364522&view=results
https://dnceng.visualstudio.com/internal/_build/results?buildId=2790770&view=results