Bump BCL references to 9.0 #76890
Bump BCL references to 9.0 #76890
Conversation
dotnet-issue-labeler
bot
added
Area-Infrastructure
untriaged
| @@ -27,6 +27,8 @@ | |||
| The version targeted is only used for build time, since we use MSBuildLocator to discover the proper version at runtime. | |||
| --> | |||
| <_MsbuildVersion>17.3.4</_MsbuildVersion> | |||
| <!-- S.C.I 9.0 no longer supports .NET 6 --> | |||
| <SuppressTfmSupportBuildWarnings>true</SuppressTfmSupportBuildWarnings> | |||
There was a problem hiding this comment.
@jasonmalinowski Would we want to pin the BuildHost at S.C.I 8.0.0 until we are ready to bump the tfm?
There was a problem hiding this comment.
Generally we should be bumping the BuildHost TFM "as late as possible". Once we merge the other PR that makes the library netstandard2.0, somebody could theoretically try to use MSBuildWorkspace in a 6.0 app...but if the BuildHost only runs on 8.0 or later you end up creating an implicit version limitation there.
That is of course not to say that we should bend backwards to support older versions -- but if we can do a minimal amount of work to keep stuff older, that might be easier than dealing the inevitable bugs people file when old versions don't work.
I think setting this property seems easiest; frankly I'm kinda surprised we added such a property in the first place. I'd propose changing the comment though since I'm sure other package upgrades will depend on this than just System.Collections.Immutable.
There was a problem hiding this comment.
And oh: I imagine this is easier to set this than it is to pin versions and deal with the special problems that creates.
There was a problem hiding this comment.
ah ok, I was afraid the package wasn't shipping with netstandard or net6.0 builds but it still has netstardard2.0
| @@ -27,6 +27,8 @@ | |||
| The version targeted is only used for build time, since we use MSBuildLocator to discover the proper version at runtime. | |||
| --> | |||
| <_MsbuildVersion>17.3.4</_MsbuildVersion> | |||
| <!-- S.C.I 9.0 no longer supports .NET 6 --> | |||
There was a problem hiding this comment.
| <!-- S.C.I 9.0 no longer supports .NET 6 --> | |
| <!-- Some of the 9.0 packages no longer have been tested against .NET 6, but we're OK with that --> |
...ools/Source/CompilerGeneratorTools/Source/CSharpSyntaxGenerator/CSharpSyntaxGenerator.csproj
Outdated
Show resolved
Hide resolved
…ence assemblies to the exclusion list.
|
The integration CI failure is expected since the VS image we are using doesn't ship with BCL9 assemblies Here's the combined VS insertion with BCL updates, all the tests results look good |
| <SystemSecurityCryptographyProtectedDataVersion>8.0.0</SystemSecurityCryptographyProtectedDataVersion> | ||
| <SystemSecurityPermissionsVersion>8.0.0</SystemSecurityPermissionsVersion> | ||
| <SystemTextEncodingsWebVersion>8.0.0</SystemTextEncodingsWebVersion> | ||
| <SystemCompositionVersion>9.0.0</SystemCompositionVersion> |
There was a problem hiding this comment.
Is it possible at all to deploy missing dependencies as part of F5/integration tests? IIRC we have this project for it - https://github.com/dotnet/roslyn/blob/main/src/VisualStudio/Setup.Dependencies/Roslyn.VisualStudio.Setup.Dependencies.csproj#L21
genlu
force-pushed
the
UpgradeBcl
branch
2 times, most recently
from
1f8c43b to
aa82706
Compare
| @@ -14,6 +14,9 @@ | |||
| <ItemGroup> | |||
| <!-- Make sure to reference the same version of Microsoft.CodeAnalysis.Analyzers as the rest of the build --> | |||
| <PackageReference Include="Microsoft.CodeAnalysis.Analyzers" PrivateAssets="all" /> | |||
| <!-- Pin SCI and SRM to 8.0, since this generator is built & consumed in our pipeline so it has to work with stock compiler in msbuild (currently shipped with 8.0) --> | |||
There was a problem hiding this comment.
Please file a bug and reference it here so we can come back around and fix this once 17.14 ships.
| For BCL, we want to use the version provided by the runtime in VS, not the ones from the NuGet packages. | ||
| This might not be possible if Roslyn is referencing a higher version than the one shipped as part of runtime in VS. | ||
| For example, we could be referecing S.C.I 7.0 but VS still ships .NET 6. Usually this is a transient state, and the | ||
| two would eventually be in sync. But we'd need to include those binaries during the out-of-sync period. | ||
| Note, for the same reasone, we can't safely exclude shared dependencies from ServiceHub host folder. |
There was a problem hiding this comment.
Wouldn't this be a functional break if we got into this state? If roslyn is reference SCI at version 9.0.0.0 and VS runtime is still using 8.0.0.0 then we open ourselves to all kinds of type / member mismatch issues correct?
There was a problem hiding this comment.
if we ship SCI in our own vsix, then at runtime we'd load our SCI in our own ACL in the servicehub host process and thing would work.
|
@dotnet/source-build this PR requires your team's approval too. Could someone please take a look? Thanks! |
No description provided.