Apply code review feedback

Author: dsplaisted

src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs

@@ -85,6 +85,11 @@ public sealed class ResolvePackageAssets : TaskBase
         /// </summary>
         public bool DisableTransitiveProjectReferences { get; set; }
 
+        /// <summary>
+        /// Disables FrameworkReferences from referenced projects or packages
+        /// </summary>
+        public bool DisableTransitiveFrameworkReferences { get; set; }
+
         /// <summary>
         /// Do not add references to framework assemblies as specified by packages.
         /// </summary>
@@ -370,6 +375,7 @@ internal byte[] HashSettings()
                     writer.Write(DisableFrameworkAssemblies);
                     writer.Write(DisableRuntimeTargets);
                     writer.Write(DisableTransitiveProjectReferences);
+                    writer.Write(DisableTransitiveFrameworkReferences);
                     writer.Write(DotNetAppHostExecutableNameWithoutExtension);
                     writer.Write(EmitAssetsLogMessages);
                     writer.Write(EnsureRuntimePackageDependencies);
@@ -1110,11 +1116,36 @@ private void WriteTransitiveProjectReferences()
 
             private void WriteFrameworkReferences()
             {
+                if (_task.DisableTransitiveFrameworkReferences)
+                {
+                    return;
+                }
+
+                //  Currently there are only 2 different frameworks that we expect to
+                //  show up as framework references, so use a list here instead of a
+                //  HashSet
+                List<string> frameworkReferences = null;
+
                 foreach (var library in _runtimeTarget.Libraries)
                 {
                     foreach (var frameworkReference in library.FrameworkReferences)
                     {
-                        WriteItem(frameworkReference, library);
+                        if (frameworkReferences == null)
+                        {
+                            frameworkReferences = new List<string>();
+                        }
+                        if (!frameworkReferences.Contains(frameworkReference))
+                        {
+                            frameworkReferences.Add(frameworkReference);
+                        }
+                    }
+                }
+
+                if (frameworkReferences != null)
+                {
+                    foreach (var frameworkReference in frameworkReferences)
+                    {
+                        WriteItem(frameworkReference);
                     }
                 }
             }

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.Sdk.TargetingPackResolution.targets

@@ -155,15 +155,10 @@ Copyright (c) .NET Foundation. All rights reserved.
   </Target>
 
   <Target Name="AddTransitiveFrameworkReferences" AfterTargets="ResolvePackageAssets"
-          Condition="'@(TransitiveFrameworkReferences)' != ''" >
+          Condition="'@(TransitiveFrameworkReference)' != ''" >
 
     <ItemGroup>
-      <!-- Deduplicate transitive framework references, and ignore any that were already directly
-           referenced-->
-      <_TransitiveFrameworkReferenceToResolve Include="@(TransitiveFrameworkReferences->Distinct())"/>
-      <_TransitiveFrameworkReferenceToResolve Remove="@(FrameworkReference)" />
-
-      <FrameworkReference Include="@(_TransitiveFrameworkReferenceToResolve)" />
+      <FrameworkReference Include="@(TransitiveFrameworkReference)" Exclude="@(FrameworkReference)"/>
     </ItemGroup>
 
   </Target>

src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.PackageDependencyResolution.targets

@@ -246,6 +246,7 @@ Copyright (c) .NET Foundation. All rights reserved.
       DisableFrameworkAssemblies="$(DisableLockFileFrameworks)"
       DisableRuntimeTargets="$(DisableRuntimeTargets)"
       DisableTransitiveProjectReferences="$(DisableTransitiveProjectReferences)"
+      DisableTransitiveFrameworkReferences="$(DisableTransitiveFrameworkReferences)"
       DotNetAppHostExecutableNameWithoutExtension="$(_DotNetAppHostExecutableNameWithoutExtension)"
       ShimRuntimeIdentifiers="@(_PackAsToolShimRuntimeIdentifiers)"
       EnsureRuntimePackageDependencies="$(EnsureRuntimePackageDependencies)"
@@ -259,7 +260,7 @@ Copyright (c) .NET Foundation. All rights reserved.
       <Output TaskParameter="ApphostsForShimRuntimeIdentifiers" ItemName="_ApphostsForShimRuntimeIdentifiersResolvePackageAssets" />
       <Output TaskParameter="ContentFilesToPreprocess" ItemName="_ContentFilesToPreprocess" />
       <Output TaskParameter="FrameworkAssemblies" ItemName="ResolvedFrameworkAssemblies" />
-      <Output TaskParameter="FrameworkReferences" ItemName="TransitiveFrameworkReferences" />
+      <Output TaskParameter="FrameworkReferences" ItemName="TransitiveFrameworkReference" />
       <Output TaskParameter="NativeLibraries" ItemName="NativeCopyLocalItems" />
       <Output TaskParameter="ResourceAssemblies" ItemName="ResourceCopyLocalItems" />
       <Output TaskParameter="RuntimeAssemblies" ItemName="RuntimeCopyLocalItems" />

src/Tests/Microsoft.NET.Build.Tests/GivenFrameworkReferences.cs

@@ -445,7 +445,10 @@ public void TargetingPackVersionCanBeSpecifiedOnFrameworkReference()
             resolvedVersions.AppHostPack["AppHost"].Should().Be("3.0.0-apphostversion");
         }
 
-        [Fact]
+        //  Transitive framework references require NuGet support, which isn't currently
+        //  in the full Framework MSBuild we use in CI, so only run these tests for
+        //  core MSBuild for now
+        [CoreMSBuildOnlyFact]
         public void TransitiveFrameworkReferenceFromProjectReference()
         {
             var testProject = new TestProject()
@@ -487,7 +490,7 @@ public void TransitiveFrameworkReferenceFromProjectReference()
             runtimeFrameworkNames.Should().BeEquivalentTo("Microsoft.AspNetCore.App");
         }
 
-        [Fact]
+        [CoreMSBuildOnlyFact]
         public void TransitiveFrameworkReferenceFromPackageReference()
         {
             var referencedPackage = new TestProject()