Skip to content

[release/2.1] Add @(ProjectReference) items for patched projects #29037

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 10, 2021
Merged

[release/2.1] Add @(ProjectReference) items for patched projects #29037

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 55 additions & 1 deletion eng/targets/ResolveReferences.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
Turn Reference items into a ProjectReference when UseProjectReferences is true.
Order matters. This comes before package resolution because projects should be used when possible instead of packages.
-->
<_ProjectReferenceByAssemblyName Condition="'$(UseProjectReferences)' == 'true'"
<_ProjectReferenceByAssemblyName Condition=" $(UseProjectReferences) "
Include="@(ProjectReferenceProvider)"
Exclude="@(_UnusedProjectReferenceProvider)" />

Expand Down Expand Up @@ -148,6 +148,60 @@
Text="Could not resolve this reference. Could not locate the package or project for &quot;%(Reference.Identity)&quot;" />
</Target>

<!--
Remove @(ResolvedCompileFileDefinitions) and @(RuntimeCopyLocalItems) items for patched assemblies between item
generation and use. Instead, ensure @(ProjectReference) items include the patched projects. This covers indirect
references and improves test coverage; @(ProjectReference) items were set correctly for direct references. Changes
are necessary only in servicing builds, in the release/2.1 branch, and when building non-implementation projects.
This is imperfect because it covers only indirect references to project reference providers (most C#
implementation projects) and does not work for legacy samples or other non-.NET SDK projects.
Note: Project reference providers have matching assembly and project names. %(Filename) matches work :)
-->
<Target Name="_TestWithPatchedAssemblies"
Condition=" $(UseProjectReferences) AND '$(PackagesInPatch)' != '' AND
(@(ResolvedCompileFileDefinitions->Count()) != 0 OR @(RuntimeCopyLocalItems->Count()) != 0) "
AfterTargets="ResolvePackageAssets"
BeforeTargets="AssignProjectConfiguration;
GenerateBuildDependencyFile;
GeneratePublishDependencyFile;
ILLink;
ResolveLockFileCopyLocalFiles;
ResolveLockFileReferences"
DependsOnTargets="ResolvePackageAssets">
<!-- Find projects patched in this build that need to be added as references. -->
<ItemGroup>
<_ProjectsInPatch Remove="@(_ProjectsInPatch)" />
<_ProjectsInPatch Include="@(ProjectReferenceProvider->'%(ProjectPath)')"
Condition=" $(PackagesInPatch.Contains(' %(ProjectReferenceProvider.Identity);')) "
KeepMetadata="None" />
<!-- dotnet-razorpagegenerator project is unique in overriding its package ID. -->
<_ProjectsInPatch Include="@(ProjectReferenceProvider->WithMetadataValue('Identity', 'dotnet-razorpagegenerator')->'%(ProjectPath)')"
Condition=" $(PackagesInPatch.Contains(' RazorPageGenerator;')) "
KeepMetadata="None" />

<!-- Ignore unreferenced assemblies and directly-referenced projects. -->
<_ProjectsToAdd Remove="@(_ProjectsToAdd)" />
<!-- Condition uses same Filename metadata when evaluating the other two item groups. -->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Put another way, the Conditions here and at the end of the target use a corner case of the task batching features. Shared metadata and batching on that metadata are the keys.

<_ProjectsToAdd Include="@(_ProjectsInPatch)"
Condition=" '%(Filename)' != '' AND
Copy link
Member

Choose a reason for hiding this comment

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

What is this condition doing exactly? Add a direct reference to the projects being patched, unless...

Copy link
Contributor Author

@dougbu dougbu Jan 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

My mental model may be wrong (@Forgind and @yuehuang010, please chime in) but https://docs.microsoft.com/en-us/visualstudio/msbuild/msbuild-batching?view=vs-2019#item-and-property-mutations contains some of the information

  1. The %(Filename) item metadata causes the implicit CreateItem task to be batched
  2. Use of that metadata also limits the @(ResolvedCompileFileDefinitions) and @(RuntimeCopyLocalItems) item groups to the item(s) with the same %(Filename)
    • This part doesn't seem to be explicit in the documentation
  3. The OR clause confirms at least one such item exists

At a higher level, this creates a @(_ProjectsToAdd) item group containing those @(_ProjectsInPatch) items that match on filename with one or more @(ResolvedCompileFileDefinitions) or @(RuntimeCopyLocalItems) items, excluding existing @(ProjectReference) items. Another way of looking at it is the created item group contains the projects in the current patch that (a) aren't referenced directly but (b) match assemblies in the current build.

Copy link

Choose a reason for hiding this comment

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

I see it used in this exampled (https://docs.microsoft.com/en-us/visualstudio/msbuild/item-metadata-in-task-batching?view=vs-2019#divide-several-item-lists-into-batches). So, it is feature name is "Divide several item lists into batches":

    <Target Name="ShowMessage">
        <Message
            Text = "Number: %(Number) -- Items in ExampColl: @(ExampColl) ExampColl2: @(ExampColl2)"/>
    </Target>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point @yuehuang010. I'm not sure how many times I read that example without thinking "Oh, this makes intersection / subsetting dead easy"😃

('@(ResolvedCompileFileDefinitions)' != '' OR '@(RuntimeCopyLocalItems)' != '') "
Exclude="@(ProjectReference)" />
</ItemGroup>

<ItemGroup Condition=" @(_ProjectsToAdd->Count()) != 0 ">
<!-- Add direct project references. -->
<ProjectReference Include="@(_ProjectsToAdd)" IsImplicitlyDefined="true" />

<!-- Remove assembly references for newly-direct project references (were from baseline packages). -->
<ResolvedCompileFileDefinitions Remove="@(ResolvedCompileFileDefinitions)"
Condition=" '%(Filename)' != '' AND '@(_ProjectsToAdd)' != '' " />
<RuntimeCopyLocalItems Remove="@(RuntimeCopyLocalItems)"
Condition=" '%(Filename)' != '' AND '@(_ProjectsToAdd)' != '' " />
</ItemGroup>
</Target>

<!-- These targets are used to generate the map of assembly name to project files. See also the /t:GenerateProjectList target in build/repo.targets. -->
<Target Name="GetReferencesProvided" Returns="@(ProvidesReference)">
<ItemGroup>
Expand Down