-
Notifications
You must be signed in to change notification settings - Fork 132
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
VMRs project filtering mechanism is suboptimal #4047
Comments
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
Does the traversal approach work with TFM filtering? How much work would it be to implement such an approach in all the repos? In other words, the approach today does work pretty transparently for most repos without actual changes in those repos. |
In regards to TFM filtering, the For this to work, these projects need to be filtered out manually at the traversal point. I.e.: <!-- Exclude .NET Framework only projects when building source-only. -->
<ItemGroup Condition="'$(DotNetBuildSourceOnly)' == 'true'">
<ProjectReference Remove="a/b/c/some-netfx-only-project.csproj" />
</ItemGroup>
With the design that I have in mind, repositories would only need to add a eng/common/RepoProjectFilter.props <Project>
<!-- Keep glob patterns in sync with the ones in the Arcade SDK's Tests.props file. -->
<ItemGroup Condition="'$(DotNetBuildTests)' != 'true'">
<ProjectReference Remove="$(MSBuildThisFileDirectory)**/*.PerformanceTests.csproj" />
<ProjectReference Remove="$(MSBuildThisFileDirectory)**/*.IntegrationTests.csproj" />
<ProjectReference Remove="$(MSBuildThisFileDirectory)**/*.Tests.csproj" />
<ProjectReference Remove="$(MSBuildThisFileDirectory)**/*.UnitTests.csproj" />
</ItemGroup>
</Project> Directory.Solution.targets <Project>
<Import Project="$(MSBuildThisFileDirectory)eng/common/RepoProjectFilter.props" />
<!-- Exclude VS components -->
<ItemGroup Condition="'$(DotNetBuild)' == 'true'">
<ProjectReference Remove="..." />
</ItemGroup>
<!-- Exclude test utility projects -->
<ItemGroup Condition="'$(DotNetBuildTests)' != 'true'">
<ProjectReference Remove="..." />
</ItemGroup>
</Project> Basically the |
That may end up being a pretty large set to special case. Also keep in mind that the filter is not based on I would prototype what this may look like in repos like roslyn. |
First of all, I hate this whole filtering model. It's super hacky and constantly causes issues. dotnet/source-build#4047 tracks this. Here we need to also empty the `ProcessFrameworkReferences` target as it still runs as it has a Before/AfterTargets hook. Keep it running is causing issues when RuntimeIdentifier is set to a custom value even though the project should be excluded. Unblocks dotnet/sdk#45362
Many repositories inside the VMR use Arcade's filtering mechanism to filter out (TFMs and) projects via the
ExcludeFrom...
properties. The problem with that is that it requires to evaluate the to be filtered out project. This leads to several issues:Alternatives:
Build.proj
) stays in sync with the generated sln file.Directory.Solution.props/targets
file with custom msbuild logic. This would be the best option for small to medium sized repositories which want to continue using a solution file but with a more dynamic filtering mechanism than what slnfs offer. Unfortunately there are couple of downsides when using that filtering mechanism today:I've been dealing with this topic for years now as we needed a solid way to filter projects in XXXL repositories like dotnet/runtime. It's clear to me that neither the
ExcludeFrom...
Arcade infrastructure, nor slnfs satisfy our needs. We need a highly performant filtering mechanism (=> no evaluations) with the flexibility to change the set of projects based on different inputs (i.e.DotNetBuildSourceOnly
vsDotNetBuild
vsDotNetBuildTests
).Repositories like runtime which already use option 2 don't need to change anything. They already declare the set of projects to be built based on various inputs. Ones that use slnfs or Arcade's infra would highly benefit from option 3. Therefore I propose that we iterate on option 3 to make it work without kicking-off evaluations (might need a small product change in msbuild's metaproj generation) and then convert the existing repositories to depend on that.
cc @mmitche @jeffkl @ericstj @MichaelSimons
The text was updated successfully, but these errors were encountered: