Skip to content

Honor .slnf solution filters when reading the solution's project set - #422

Merged
razzmatazz merged 1 commit into
razzmatazz:mainfrom
pbednarcik:fix/slnf-project-count
Sep 6, 2026
Merged

razzmatazz merged 1 commit into
razzmatazz:mainfrom
pbednarcik:fix/slnf-project-count

Conversation

@pbednarcik

@pbednarcik pbednarcik commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

solutionLoadProjectFilenames reads ProjectsInOrder from SolutionFile.Parse, and for a .slnf that lists every project of the parent solution, not the filtered set. MSBuild applies the filter internally when building, but the project set we derive from it is wrong, with two visible effects on a filtered solution:

  • the load progress denominator counts the whole parent solution. On dotnet/roslyn's Compilers.slnf the reports read (74/358) and the percentage stopped at 20 (this is the counter from feat: report per-project progress while loading a solution #417, so the bug is mine to fix)
  • the workspace-global TargetFramework intersection runs over projects the filter excludes, which can disengage the multi-TFM support for no reason

SolutionFile.ProjectShouldBuild answers exactly this question but it is internal, so it is resolved by reflection, the way this server already reaches Roslyn internals. It answers true for every project when the solution carries no filter, so it needs no .slnf check of its own. Microsoft.Build comes from the SDK that MSBuildLocator picked rather than from a pinned package reference, so a member that stops existing logs a warning and degrades to loading every declared project instead of failing the solution load.

Test: a new solutionWithFilter fixture (three-project Full.sln, Partial.slnf keeping two of them) loaded through solutionPathOverride. Before the fix the progress reports read ProjectA (1/3), ProjectC (2/3) and stopped at 66; with the fix they read (1/2), (2/2) and reach 100. The test harness did not copy .slnf files into the temp fixture dir, so that is added to the copy filter, and the progress-report collection shared by the load-progress tests moved into a small helper.

@razzmatazz razzmatazz left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved with suggestion

Comment thread src/CSharpLanguageServer/Roslyn/Solution.fs Outdated
solutionLoadProjectFilenames read ProjectsInOrder from SolutionFile.Parse,
which for a .slnf lists every project of the PARENT solution. Two visible
effects on filtered solutions: the load-progress denominator counted the
whole parent solution (dotnet/roslyn Compilers.slnf reported "(74/358)" and
finished with the percentage stuck at 20), and the workspace-global
TargetFramework intersection ran over projects the filter excludes, which
can wrongly disengage the multi-tfm support.

MSBuild applies the filter internally but exposes no public way to query it,
so SolutionFile.ProjectShouldBuild is resolved by reflection, the way this
server already reaches Roslyn internals. Reusing MSBuild's own predicate also
keeps the match correct: it normalizes path separators and then compares with
a comparer that is case-sensitive on Linux and case-insensitive everywhere
else. The predicate answers true for every project when the solution carries
no filter, so it needs no .slnf check of its own. Microsoft.Build is whichever
assembly MSBuildLocator found in the installed SDK rather than a pinned
package reference, so a member that stops existing logs a warning and degrades
to loading every declared project instead of failing the solution load.

Test-side: a solutionWithFilter fixture (three-project Full.sln plus a
Partial.slnf keeping two) loaded through solutionPathOverride; before the fix
the progress reports read "(1/3)", "(2/3)" and stopped at 66. The test harness
now copies .slnf files into the temp fixture dir, and the progress-report
collection shared by the load-progress tests moved into a helper.
@pbednarcik
pbednarcik force-pushed the fix/slnf-project-count branch from 7827fd0 to d2353d0 Compare September 3, 2026 07:35
@razzmatazz
razzmatazz merged commit 407e76b into razzmatazz:main Sep 6, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants