Visual Studio restore is broken as projects are missing from the dependency closure

[ViktorHofer]

Since I applied various fixes to the product over the last weeks that should make the overall IDE experience better I wanted to try them out and had to realize with dismay that the integrated Visual Studio restore is still broken if an upfront restore via the CLI (dotnet restore) isn't performed. The impact of that is that the solution is unusable.

The root cause for that lies in this code path:

runtime/eng/slngen.targets

Lines 8 to 11 in cc649ac

	<!-- Don't include reference projects which compose the microsoft.netcore.app targeting pack (except the current leaf's reference project) as those are referenced by the source project via named references
	and hence don't need to be part of the solution file (only P2Ps need to).
	Include the reference project in the solution file if it targets more than just NetCoreAppCurrent as other frameworks like .NETFramework, .NETStandard or older .NETCoreApp ones require it. -->
	<IncludeInSolutionFile Condition="'$(IsNETCoreAppRef)' == 'true' and '$(MSBuildProjectName)' != '$(SlnGenMainProject)' and '$(TargetFramework)' == '$(NetCoreAppCurrent)' and ('$(TargetFrameworks)' == '' or '$(TargetFrameworks)' == '$(NetCoreAppCurrent)')">false</IncludeInSolutionFile>

This causes slngen to not include reference projects which are part of the shared framework and aren't shipping out-of-band. Even though, shared framework reference assemblies are only referenced via "Reference" items by source projects, the reference projects themselves reference each other via ProjectReferences. Note that VS restore requires all dependencies expressed via ProjectReference items to be present in the solution file and ignores Reference items as those point to the compiled assembly and there's nothing for restore to do on those.

As an example, ref/System.Drawing.Common.csproj inside System.Runtime.sln is depending on ref/System.Collections.NonGeneric.csproj but as it is missing, the restore inside Visual Studio fails.

If I remove the condition in the slngen.targets file and re-generate all solution files (dotnet.cmd build src\libraries\slngen.proj), the solution files get larger by about 50 ref projects (as the solution now includes all dependencies) but the solution restore finally works and no up-front restore is necessary anymore. Note that because the reference projects are very small and fast to evaluate, the solution load time doesn't increase noticeably and the solution doesn't look overloaded as all the added projects are grouped inside the "ref" folder (tested with VS 2022 17.2 Preview 1).

Because of that I propose that we remove the condition in the slngen.targets file with the caveat that solutions get bigger but with the huge win that up-front restores aren't necessary anymore. That means instead of:

build.cmd libs.sfx /p:RefOnly=true
build.cmd -restore libs
build.cmd -vs System.Runtime

I can now directly open VS after the libs.ref subset is built:

build.cmd libs.sfx /p:RefOnly=true
start src\libraries\System.Runtime\System.Runtime.sln

or just continue to use the VS switch for convenience (which also uses the repo local SDK which is useful when a matching one isn't globally installed):

build.cmd libs.sfx /p:RefOnly=true
build.cmd -vs System.Runtime

Ultimately at some point in the future when source projects use ProjectReference items instead of Reference items to dynamically express their dependencies, the build.cmd libs.ref step won't be necessary anymore either and a dev will be able to directly open a VS solution file from a fresh clone.

@ericstj @safern @joperezr @carlossanlop any objections to what I'm proposing?

[ghost]

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Since I applied various fixes to the product over the last weeks that should make the overall IDE experience better I wanted to try them out and had to realize with dismay that the integrated Visual Studio restore is still broken if an upfront restore via the CLI (dotnet restore) isn't performed. The impact of that is that the solution is unusable.

The root cause for that lies in this code path:

runtime/eng/slngen.targets

Lines 8 to 11 in cc649ac

	<!-- Don't include reference projects which compose the microsoft.netcore.app targeting pack (except the current leaf's reference project) as those are referenced by the source project via named references
	and hence don't need to be part of the solution file (only P2Ps need to).
	Include the reference project in the solution file if it targets more than just NetCoreAppCurrent as other frameworks like .NETFramework, .NETStandard or older .NETCoreApp ones require it. -->
	<IncludeInSolutionFile Condition="'$(IsNETCoreAppRef)' == 'true' and '$(MSBuildProjectName)' != '$(SlnGenMainProject)' and '$(TargetFramework)' == '$(NetCoreAppCurrent)' and ('$(TargetFrameworks)' == '' or '$(TargetFrameworks)' == '$(NetCoreAppCurrent)')">false</IncludeInSolutionFile>

This causes slngen to not include reference projects which are part of the shared framework and aren't shipping out-of-band. Even though, shared framework reference assemblies are only referenced via "Reference" items by source projects, the reference projects themselves reference each other via ProjectReferences. Note that VS restore requires all dependencies expressed via ProjectReference items to be present in the solution file and ignores Reference items as those point to the compiled assembly and there's nothing for restore to do on those.

As an example, ref/System.Drawing.Common.csproj inside System.Runtime.sln is depending on ref/System.Collections.NonGeneric.csproj but as it is missing, the restore inside Visual Studio fails.

If I remove the condition in the slngen.targets file and re-generate all solution files (dotnet.cmd build src\libraries\slngen.proj), the solution files get larger by about 50 ref projects (as the solution now includes all dependencies) but the solution restore finally works and no up-front restore is necessary anymore. Note that because the reference projects are very small and fast to evaluate, the solution load time doesn't increase noticeably and the solution doesn't look overloaded as all the added projects are grouped inside the "ref" folder (tested with VS 2022 17.2 Preview 1).

Because of that I propose that we remove the condition in the slngen.targets file with the caveat that solutions get bigger but with the huge win that up-front restores aren't necessary anymore. That means instead of:

build.cmd libs.ref
build.cmd -restore libs
build.cmd -vs System.Runtime

I can now directly open VS after the libs.ref subset is built:

build.cmd libs.ref
start src\libraries\System.Runtime\System.Runtime.sln

or just continue to use the VS switch for convenience (which also uses the repo local SDK which is useful when a matching one isn't globally installed):

build.cmd libs.ref
build.cmd -vs System.Runtime

Ultimately at some point in the future when source projects use ProjectReference items instead of Reference items to dynamically express their dependencies, the build.cmd libs.ref step won't be necessary anymore either and a dev will be able to directly open a VS solution file from a fresh clone.

@ericstj @safern @joperezr @carlossanlop any objections to what I'm proposing?

Author:	ViktorHofer
Assignees:	-
Labels:	area-Infrastructure-libraries
Milestone:	-

[ViktorHofer]

Would love to hear people's opinion about the above described issue. What I heard from the recent community survey is that one of the top pain points is that solution files in our repositories can't be opened without ceremony involved. Fixing the VS restore by making sure that the dependency closure is satisfied by the solution files would get us closer to the community's wish.

[joperezr]

I'm not opposed to what you are proposing, but I think that is still not what the community wants. The main ask here would be to be able to open a solution in VS after a clean git clone and even with your proposal, this wouldn't be possible. In fact, I'm not even sure that is something we can ever achieve given the fact that a lot of our infrastructure for building lives in Arcade which needs to be restored at least once before opening a solution, so even though with your proposal the "ceremony" is being shorter, it is still there. Perhaps something we can consider instead of making the more expensive changes proposed, is to consolidate the current ceremony into a single command.

The current upfront restore in a traditional machine with a decent network, doesn't really take that much (around 2 minutes). I am sure that with your proposed changes this is reduced greatly, but have you measured that and if so can you share some of the perf impact?

I guess my uber point is, before we go about and make some changes that will have notable differences to all (people will start seeing 50+ projects inside the ref/ folder in the solution) we should measure and make sure that the cost is worth the benefit, especially since the end result is not meeting the desired community result.

[ericstj]

cc @stephentoub @eerhardt

I guess the tradeoff for some here is the sln size and build performance. You've also done more work recently to improve that.
Can you measure the difference for incremental build after a small change in the src library in some common SLNs before and after and share it?

For example System.Runtime, System.Text.Json, and Microsoft.Extensions.Logging? I think those are somewhat representative of the developer workflows for bottom, middle, and top of stack.

[eerhardt]

and Microsoft.Extensions.Logging

Can you change that to Microsoft.Extensions.Hosting? That one is the most "top of the stack" since it references almost everything (including Logging).

My opinion is roughly inline with @joperezr above - if I need to run something up front, I'd rather that "up front" cost be more and my incremental cost (building constantly in VS, running tests, etc) be lower. After pulling new code, I kick off a repo libs+clr -rc Release build. I need to do that in order to run unit tests anyway. So having VS load more projects and become slower to load/build/etc isn't worth the tradeoff IMO.

[ViktorHofer]

Can you change that to Microsoft.Extensions.Hosting? That one is the most "top of the stack" since it references almost everything (including Logging).

I spent some more time thinking about how to fix the current broken behavior without increasing the build graph. Out-of-band projects already build on-top of the shared framework since we split the libs build into "libs.sfx" and "libs.oob". As these projects already sit on the top, we can just use the default targeting pack references for OOBs and remove the ProjectReferences that point to inbox projects. Note that this is already the behavior for most OOB source projects. Overall that will actually make the build graph and by that, solution files smaller for OOBs.

If I don't hear any objections I would like to give that a try and prepare a PR that will better demonstrate what I'm proposing.

[ViktorHofer]

#68543 accidentally closed this issue. Reopening.

[ViktorHofer]

from @joperezr

In fact, I'm not even sure that is something we can ever achieve given the fact that a lot of our infrastructure for building lives in Arcade which needs to be restored at least once before opening a solution, so even though with your proposal the "ceremony" is being shorter, it is still there. Perhaps something we can consider instead of making the more expensive changes proposed, is to consolidate the current ceremony into a single command.

Infrastructure that lives in Arcade is built and packaged there and we consume those libraries the usual way via PackageReferences, which are part of a project's restore. Your statement that these need to be up-front restored before a solution can be opened isn't right. NuGet is part of Visual Studio and makes sure that all dependencies (PackageReferences, PackageDownload, FrameworkReferences, msbuild sdks, ...) are restored on the fly. You can test this by opening i.e. System.Runtime.sln (or any other solution where P2Ps are in sync between ref and src, like System.Runtime.ComilerServices.Unsafe) from a fresh clone and building the ref and the source assembly in it. If I'm not mistaken, everything should build just fine without an up-front restore.

The reason why most src/libraries solutions can't be opened from a fresh clone is because dependencies inside the shared framework aren't defined via ProjectReference items. As an example, System.Collections.NonGeneric depends on System.Threading but because that dependency isn't expressed via a ProjectReference, System.Threading isn't available when System.Collections.NonGeneric is built.

from @ericstj

I guess the tradeoff for some here is the sln size and build performance. You've also done more work recently to improve that.

With recent improvements (#68488, #68543 and #68603), a project's build graph is now as short as possible. Before these changes were merged, a project's build graph contained many unnecessary dependencies, which impacted both the evaluation and restore/build performance.

Can you measure the difference for incremental build after a small change in the src library in some common SLNs before and after and share it?

I can finally share some data based on #68668 which builds on top of the improvements listed above.

Project	No. of Projects Before	No. of Projects After
System.CodeDom	9	6
System.Net.Quic	7	18
System.Runtime	25	25
System.Runtime.Serialization.Formatters	55	58
System.Text.Json	23	27
Microsoft.Extensions.Hosting	74	75

In some cases the number of projects in the solution file drops, in most cases it increases between 1-5 projects and in a few cases it increases between 10-15 projects. The time it takes to load and evaluate the solution file shouldn't change much by adding this number of projects. Visual Studio invested a lot in evaluation, Intellisense and restore performance over the last years.

@ericstj regarding incremental build performance, I'm not sure what you ask me to verify. AFAIK Visual Studio's fast up-to-date check doesn't incrementally build projects that haven't changed (based on inputs & outputs).
If I would change the sln's main source library, I would not see a difference at all because the libraries being added with my change aren't depending on the source library and hence don't build at all.

from @eerhardt

if I need to run something up front, I'd rather that "up front" cost be more and my incremental cost (building constantly in VS, running tests, etc) be lower.

You probably mean that as a general statement and I fully agree with that. Just to clarify, I don't expect #68668 to increase the incremental cost in any noticeable way (based on VS's fast up-to-date check).

[ericstj]

I'm not sure what you ask me to verify.

My suggestion was to show others that the impact of this change on their daily work is minimal and that the tradeoff is a good one.

AFAIK Visual Studio's fast up-to-date check doesn't incrementally build projects that haven't changed (based on inputs & outputs).

Very cool. Testing that out in these cases and confirming that we don't do anything funny in dotnet/runtime to break that would be a good piece of data to share here.

Maybe share the time to do a build for these sln's once they are loaded with no changes over what was already built up front? Also the time for build after a comment change in the src library?
You can use the Project System Tools Build Logging window to capture the "Elapsed" time of the build.
If both are very small (like under a couple seconds) then its probably sufficient to demonstrate that we have a good workflow. If they aren't small then you can share the diff before/after a change to show that it didn't get any worse.

I think these small checks and the new data you've shared around the actual number of projects (which is much less than 50 per sln) address the concerns raised by @joperezr and @eerhardt.

[ViktorHofer]

Let me explain how Visual Studio actually performs fast up-to-date checks:

1. When a build is triggered inside the IDE, it initially populates its cache by triggering (incremental) builds.
2. After that, whenever a new build is triggered and the inputs and outputs didn't change, the incremental build is skipped entirely (I would love msbuild to do that on the CLI as well).
3. When a project's input or output changes, it gets rebuilt but its dependencies won't because their inputs & outputs didn't change.

Here's an example of a fast up-to-date check from VS's output window after I made a comment change to the source assembly. You can see that VS doesn't even attempt to incrementally build dependencies that aren't impacted:

Build started...
1>------ Build started: Project: System.Net.Quic (src\System.Net.Quic), Configuration: Debug Any CPU ------
1>System.Net.Quic -> C:\git\runtime5\artifacts\bin\System.Net.Quic\Debug\net7.0\System.Net.Quic.dll
1>System.Net.Quic -> C:\git\runtime5\artifacts\bin\System.Net.Quic\Debug\net7.0-freebsd\System.Net.Quic.dll
1>System.Net.Quic -> C:\git\runtime5\artifacts\bin\System.Net.Quic\Debug\net7.0-windows\System.Net.Quic.dll
1>System.Net.Quic -> C:\git\runtime5\artifacts\bin\System.Net.Quic\Debug\net7.0-osx\System.Net.Quic.dll
1>System.Net.Quic -> C:\git\runtime5\artifacts\bin\System.Net.Quic\Debug\net7.0-linux\System.Net.Quic.dll
========== Build: 1 succeeded, 0 failed, 14 up-to-date, 0 skipped ==========

I noticed that System.Private.CoreLib always incrementally builds as fast up-to-date checks seem to not work for it. I filed #68698 to track that.

You can use the Project System Tools Build Logging window to capture the "Elapsed" time of the build.

I ❤️ open source so I filed a bug that I found while working with the project system tools: dotnet/project-system-tools#439.

The time for build after a comment change in the src library?

Numbers are from my Surface Book 2 which in contrast to most devs on the team is a quite slow machine. Visual Studio 17.2 Preview 5. As already mentioned before, the numbers below are exactly the same on main as builds of dependencies are skipped when they are up-to-date.

I made to sure wait until background tasks completed and triggered multiple builds to get the right average.

Project	Build time before change in src	Build time after change in src	Notes
System.CodeDom	5.898s	4.511s
System.Net.Quic	9.988s	7.560s
System.Runtime	4.087	3.784	ApiCompat bug inside VS: #68699
System.Runtime.Serialization.Formatters	12.641	9.800
System.Text.Json	13.319		Not measure-able because fast up-to-date file checks of System.Text.Json are broken. Contacted @drewnoakes offline
Microsoft.Extensions.Hosting	17.451		Not measure-able because fast up-to-date file checks of System.Text.Json are broken. Contacted @drewnoakes offline

My assumption is that VS uses a fallback mechanism when dependencies are missing from the solution which makes the build actually slower.

Maybe share the time to do a build for these sln's once they are loaded with no changes over what was already built up front?

That's hard to measure as the libs root build only builds the current vertical (ie Windows on a Windows machine) in contrast to VS which builds all configurations. I could measure this with an all-configurations build but that wouldn't be representative as devs rarely build in that mode from the root. I believe that the the shared data already demonstrates that the changes doesn't regress the performance inside VS except for initially evaluating and populating the fast-up-date check cache. These two steps are naturally slower but as said, based on the low number of projects being added to the solutions I couldn't measure any real impact.