Remove handling of certain BCL binaries from our build

[genlu]

VS is taking over insertion of the following BCL packages.

• System.Collections.Immutable (not done in VS yet)
• System.Reflection.Metadata (not done in VS yet)
• System.Runtime.CompilerServices.Unsafe
• System.Threading.Tasks.Extensions

See https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/365249 and
https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/364153 for the change on VS side

We have an insertion to validate this change:
https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/365416

[jaredpar]

Do we still control the versions of these assemblies that are inserted into the roslyn directory of MSBuild? If so how do we keep those in sync with what VS is inserting?

[genlu]

@jaredpar We do. And we don't keep them in sync (roslyn is referecing 5.0, while VS is moving to 6.0). csc, vbcscompiler, etc all have their own exe.config, is keeping them in sync necessary? they will just use the version shipped by us, right? When roslyn is running in devenv, we will be redirected to whatever version VS provides.

[jaredpar]

is keeping them in sync necessary?

It's not necessary but I'd say it's desirable. Consider that when in sync then we're only carrying one copy in the installer, only NGEN'ing one version, analyzers / generators using all the same versions inside VS as well as on the command line. It is just overall more streamline.

Consider that now that VS has moved to 6.0 the compiler will likely do so as well in short order. In general that should be the pattern. If VS moves forward we should move to match. It doesn't have to be in lock step with synchronized check ins but we should be trying to be on same versions at RTM time.

[genlu]

I guess for now, we will stay out-of -sync until compiler move to 6.0 as well. In longer term, is it possible that we don't include them in compiler vsix, instead redirect to the ones in VS?

[genlu]

@AArnott Are you gonna merge this? https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/364765 We'd prefer to have it done separately to limit the scope of our next insertion.

Created a new PR for SCI and SRM
https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/365249

[jaredpar]

In longer term, is it possible that we don't include them in compiler vsix, instead redirect to the ones in VS?

That has a number of downsides I'd not like to go down.

Really I don't think it's much harder than whenever VS moves their binaries forward we move the compilers forward too. If there was an announcement list, etc ... where we saw the verisons changing we could file a bug and update a week later. It's not much work.

[RikkiGibson]

In my opinion, the most reasonable way for sync to work is for us to check via some automation. We can manually open the PRs if that automation notifies us that something needs to change, but ideally it would be difficult or impossible for someone to bump the package versions on the VS side and forget to notify us.

[genlu]

We might be able to let insertion tool checks for the version of certain packages in VS, and create a bug when there's mismatch in major version

[AArnott]

If there was an announcement list, etc ... where we saw the verisons changing we could file a bug and update a week later. It's not much work.

I wonder if the Impactful Change DL is too heavy-handed for this. Probably so.
I usually include Tomas on PRs that update those versions in VS. I didn't this time around because I didn't update any packages that Roslyn was inserting optimizing builds of so I didn't think it mattered.
Roslyn team members could add themselves as optional reviewers for any changes to the corefx.props and corefx.packageconfig files in order to be notified when these versions are upgrading.

[genlu]

Roslyn team members could add themselves as optional reviewers for any changes to the corefx.props and corefx.packageconfig files in order to be notified when these versions are upgrading.

I think this would be sufficient, let's add infraswat to the subscription then. @JoeRobich Do you know how to do that?

Also @jaredpar what's the plan for moving roslyn to 6.0 libraries?

[jaredpar]

Also @jaredpar what's the plan for moving roslyn to 6.0 libraries?

Once this change has settled and merged into VS then we should just upgrade pretty much immediately after. Both because it matches the "keep in sync" guideline and testing out immediately that we can indeed insert independently.

We should file an issue, track it to 17.1 and assign it to the compiler team. I can assign it out from there

[JoeRobich]

Merging to unblock signed build.

[genlu]

@jaredpar Well, this is actually merged 🎊
I filed an issue for moving roslyn to 6.0 packages #57875. Pls note that the upgrade isn't completed in VS yet, most was done in this PR, but SCI and SRM is handled here

[RikkiGibson]

@genlu @dotnet/roslyn-infrastructure 17.0 insertions currently require manual editing to work properly. Could this change be backported to the 17.0 branch?

[genlu]

Is rel/d17.0 already using stock BCL packages instead of the ones from us?

[RikkiGibson]

It seems we do in corefx.props but not in default.config.