Consume requested targets when constructing the graph

.azuredevops/pipelines/pr-ci.yml

@@ -57,11 +57,12 @@ jobs:
 
   - checkout: self
 
-  - task: Cache@2
-    displayName: Cache Visual Studio
-    inputs:
-      key: '"vs-release" | "$(Agent.OS)"'
-      path: $(VsInstallDir)
+  # Avoiding Caching VS for now as the installer command we're using doesn't work for upgrades
+  # - task: Cache@2
+  #   displayName: Cache Visual Studio
+  #   inputs:
+  #     key: '"vs-release" | "$(Agent.OS)"'
+  #     path: $(VsInstallDir)
 
   - script: |
       del %TEMP%\vs_buildtools.exe
@@ -78,6 +79,10 @@ jobs:
 
       del %TEMP%\vs_buildtools.exe
 
+      echo Copying installer logs
+      mkdir "$(LogDirectory)\VSSetup"
+      move "%TEMP%\dd_*" "$(LogDirectory)\VSSetup\"
+
       echo Current MSBuild version:
       "$(MSBuildPath)" --version
     displayName: 'Install VS'
@@ -105,11 +110,12 @@ jobs:
 
   - checkout: self
 
-  - task: Cache@2
-    displayName: Cache Visual Studio
-    inputs:
-      key: '"vs-pre" | "$(Agent.OS)"'
-      path: $(VsInstallDir)
+  # Avoiding Caching VS for now as the installer command we're using doesn't work for upgrades
+  # - task: Cache@2
+  #   displayName: Cache Visual Studio
+  #   inputs:
+  #     key: '"vs-pre" | "$(Agent.OS)"'
+  #     path: $(VsInstallDir)
 
   - script: |
       del %TEMP%\vs_buildtools.exe
@@ -126,6 +132,10 @@ jobs:
 
       del %TEMP%\vs_buildtools.exe
 
+      echo Copying installer logs
+      mkdir "$(LogDirectory)\VSSetup"
+      move "%TEMP%\dd_*" "$(LogDirectory)\VSSetup\"
+
       echo Current MSBuild version:
       "$(MSBuildPath)" --version
     displayName: 'Install VS Preview'

.azuredevops/pipelines/templates/variables.yml

@@ -5,3 +5,4 @@ variables:
   ArtifactsDirectory: $(Build.ArtifactStagingDirectory)\artifacts
   # https://github.com/microsoft/azure-pipelines-agent/pull/4077
   VSO_DEDUP_REDIRECT_TIMEOUT_IN_SEC: 5
+  EnablePipelineCache: true

CONTRIBUTING.md

@@ -14,7 +14,7 @@ contact [[email protected]](mailto:[email protected]) with any additio
 
 ## Setup
 
-It is assumed that you are using VS v17.8 or later.
+It is assumed that you are using VS v17.9 or later.
 
 ## Building
 

Directory.Packages.props

@@ -3,13 +3,13 @@
   <PropertyGroup>
     <ArtifactsPackageVersion>19.232.34508-buildid25403274</ArtifactsPackageVersion>
     <BuildXLPackageVersion>0.1.0-20240121.1</BuildXLPackageVersion>
-    <MSBuildPackageVersion>17.8.3</MSBuildPackageVersion>
+    <MSBuildPackageVersion>17.9.5</MSBuildPackageVersion>
   </PropertyGroup>
   <ItemGroup>
     <PackageVersion Include="coverlet.collector" Version="3.1.2" />
     <PackageVersion Include="DotNet.Glob" Version="2.0.3" />
     <PackageVersion Include="ILRepack" Version="2.0.27" />
-    <PackageVersion Include="Microsoft.Bcl.AsyncInterfaces" Version="7.0.0" />
+    <PackageVersion Include="Microsoft.Bcl.AsyncInterfaces" Version="8.0.0" />
     <PackageVersion Include="Microsoft.Bcl.HashCode" Version="1.1.1" />
     <PackageVersion Include="Microsoft.Build" Version="$(MSBuildPackageVersion)" />
     <PackageVersion Include="Microsoft.Build.Framework" Version="$(MSBuildPackageVersion)" />
@@ -39,14 +39,14 @@
     <PackageVersion Include="protobuf-net" Version="3.2.26" />
     <PackageVersion Include="protobuf-net.Core" Version="3.2.26" />
     <PackageVersion Include="protobuf-net.Grpc" Version="1.1.1" />
-    <PackageVersion Include="System.Collections.Immutable" Version="7.0.0" />
+    <PackageVersion Include="System.Collections.Immutable" Version="8.0.0" />
     <PackageVersion Include="System.Memory" Version="4.5.5" />
     <PackageVersion Include="System.Private.ServiceModel" Version="4.10.3" />
-    <PackageVersion Include="System.Reflection.Metadata" Version="7.0.0" />
+    <PackageVersion Include="System.Reflection.Metadata" Version="8.0.0" />
     <PackageVersion Include="System.Runtime.CompilerServices.Unsafe" Version="6.0.0" />
-    <PackageVersion Include="System.Text.Json" Version="7.0.3" />
-    <PackageVersion Include="System.Threading.Channels" Version="7.0.0" />
-    <PackageVersion Include="System.Threading.Tasks.Dataflow" Version="7.0.0" />
+    <PackageVersion Include="System.Text.Json" Version="8.0.0" />
+    <PackageVersion Include="System.Threading.Channels" Version="8.0.0" />
+    <PackageVersion Include="System.Threading.Tasks.Dataflow" Version="8.0.0" />
   </ItemGroup>
   <ItemGroup>
     <GlobalPackageReference Include="Microsoft.VisualStudioEng.MicroBuild.Core" Version="1.0.0" />

src/Common/MSBuildCachePluginBase.cs

@@ -273,8 +273,8 @@ private async Task BeginBuildInnerAsync(CacheContext context, PluginLoggerBase l
         Parser parser = new(logger, _repoRoot);
         IReadOnlyDictionary<ProjectGraphNode, ParserInfo> parserInfoForNodes = parser.Parse(graph);
 
-        // TODO: MSBuild should give this to us via CacheContext
-        var entryProjectTargets = Array.Empty<string>();
+        // In practice, the underlying type of the IReadOnlyCollection is a ICollection<string> so attempt to cast first. We're not mutating the collection so still abiding by the readonly-ness.
+        ICollection<string> entryProjectTargets = context.RequestedTargets as ICollection<string> ?? new List<string>(context.RequestedTargets);
         IReadOnlyDictionary<ProjectGraphNode, ImmutableList<string>> targetListPerNode = graph.GetTargetLists(entryProjectTargets);
 
         Dictionary<NodeDescriptor, NodeContext> nodeContexts = new(parserInfoForNodes.Count);

src/Repack.Tests/RepackTests.cs

@@ -5,7 +5,6 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
-using System.Reflection;
 using Microsoft.MSBuildCache.AzureBlobStorage;
 using Microsoft.MSBuildCache.AzurePipelines;
 using Microsoft.MSBuildCache.Tests;
@@ -17,33 +16,29 @@ namespace Microsoft.MSBuildCache.Repack.Tests;
 [TestClass]
 public class RepackTests
 {
-    private static readonly Type[] TypesToCheck =
+    [DataTestMethod]
+    [DataRow(typeof(MSBuildCacheAzureBlobStoragePlugin))]
+    [DataRow(typeof(MSBuildCacheAzurePipelinesPlugin))]
+    [DataRow(typeof(MSBuildCacheLocalPlugin))]
+    [DataRow(typeof(SharedCompilation.ResolveFileAccesses))]
+    public void PluginInterfaceAssembliesNotMerged(Type typeToCheck)
     {
-        typeof(MSBuildCacheAzureBlobStoragePlugin),
-        typeof(MSBuildCacheAzurePipelinesPlugin),
-        typeof(MSBuildCacheLocalPlugin),
-        typeof(SharedCompilation.ResolveFileAccesses),
-    };
+#if DEBUG
+        // Using Assert.Inconclusive instead of removing the entire test for visibility that the test exists.
+        Assert.Inconclusive("This test only applies to Release builds.");
+#endif
 
-    [TestMethod]
-    public void PluginInterfaceAssembliesNotMerged()
-    {
-        foreach (Type type in TypesToCheck)
-        {
-            Dictionary<string, AssemblyName> references = type.Assembly
-                .GetReferencedAssemblies()
-                .Where(a => a.Name is not null)
-                .ToDictionary(
-                    a => a.Name!,
-                    a => a
-                );
+        HashSet<string> references = typeToCheck.Assembly
+            .GetReferencedAssemblies()
+            .Where(a => a.Name is not null)
+            .Select(reference => reference.Name!)
+            .ToHashSet(StringComparer.Ordinal);
 
-            // Check to make sure that each of the interface assemblies are still actually referenced
-            foreach (string expectedRefFileName in PluginInterfaceTypeCheckTests.PluginInterfaceNuGetAssemblies)
-            {
-                string expectedRef = Path.GetFileNameWithoutExtension(expectedRefFileName);
-                Assert.IsNotNull(references.FirstOrDefault(a => a.Value.FullName.IndexOf(expectedRef, StringComparison.Ordinal) > 0));
-            }
+        // Check to make sure that each of the interface assemblies are still actually referenced
+        foreach (string expectedRefFileName in PluginInterfaceTypeCheckTests.PluginInterfaceNuGetAssemblies)
+        {
+            string expectedRef = Path.GetFileNameWithoutExtension(expectedRefFileName);
+            Assert.IsTrue(references.Contains(expectedRef));
         }
     }
 }

src/SharedCompilation/VBCSCompilerReporter.cs

@@ -9,8 +9,6 @@
 #endif
 using System.IO;
 using System.Linq;
-using System.Reflection;
-using System.Reflection.Emit;
 using Microsoft.Build.Experimental.FileAccess;
 using Microsoft.Build.Framework;
 using Microsoft.CodeAnalysis;
@@ -223,25 +221,10 @@ private sealed class AccessRegistrar
     {
         private delegate void ReportFileAccessFn(FileAccessData fileAccessData);
 
-        private delegate FileAccessData CreateFileAccessFn(
-            ReportedFileOperation operation,
-            RequestedAccess requestedAccess,
-            uint processId,
-            uint id,
-            uint correlationId,
-            uint error,
-            DesiredAccess desiredAccess,
-            FlagsAndAttributes flagsAndAttributes,
-            string path,
-            string processArgs,
-            bool isAnAugmentedFileAccess);
-
         private readonly string _basePath;
 
         private readonly ReportFileAccessFn _reportFileAccess;
 
-        private readonly CreateFileAccessFn _createFileAccess;
-
         private static readonly uint ProcessId = (uint)
 #if NET472
                 Process.GetCurrentProcess().Id;
@@ -257,57 +240,6 @@ public AccessRegistrar(string basePath, EngineServices engineServices)
             // Use reflection to get the ReportFileAccess method since it's not exposed.
             // TODO: Once there is a proper API for this, use it.
             _reportFileAccess = (ReportFileAccessFn)Delegate.CreateDelegate(typeof(ReportFileAccessFn), engineServices, "ReportFileAccess");
-
-            ConstructorInfo fileAccessDataCtor = typeof(FileAccessData).GetConstructors()[0];
-            ParameterInfo[] fileAccessDataCtorParams = fileAccessDataCtor.GetParameters();
-            if (fileAccessDataCtorParams.Length == 9)
-            {
-                // This is the one we compiled against so just use it.
-                _createFileAccess = (
-                    ReportedFileOperation operation,
-                    RequestedAccess requestedAccess,
-                    uint processId,
-                    uint id,
-                    uint correlationId,
-                    uint error,
-                    DesiredAccess desiredAccess,
-                    FlagsAndAttributes flagsAndAttributes,
-                    string path,
-                    string processArgs,
-                    bool isAnAugmentedFileAccess)
-                    => new FileAccessData(operation, requestedAccess, processId, error, desiredAccess, flagsAndAttributes, path, processArgs, isAnAugmentedFileAccess);
-            }
-            else if (fileAccessDataCtorParams.Length == 11)
-            {
-                // Adjust to a breaking change made in MSBuild to the FileAccessData ctor. See: https://github.com/dotnet/msbuild/pull/9615
-                // Create a dynamic method which basically just invokes the ctor with all the params from the CreateFileAccessFn delegate.
-                DynamicMethod dynamicMethod = new(
-                    name: string.Empty,
-                    returnType: typeof(FileAccessData),
-                    parameterTypes: fileAccessDataCtorParams.Select(param => param.ParameterType).ToArray(),
-                    owner: typeof(FileAccessData));
-
-                ILGenerator il = dynamicMethod.GetILGenerator();
-                il.Emit(OpCodes.Ldarg_0);
-                il.Emit(OpCodes.Ldarg_1);
-                il.Emit(OpCodes.Ldarg_2);
-                il.Emit(OpCodes.Ldarg_3);
-                il.Emit(OpCodes.Ldarg_S, 4);
-                il.Emit(OpCodes.Ldarg_S, 5);
-                il.Emit(OpCodes.Ldarg_S, 6);
-                il.Emit(OpCodes.Ldarg_S, 7);
-                il.Emit(OpCodes.Ldarg_S, 8);
-                il.Emit(OpCodes.Ldarg_S, 9);
-                il.Emit(OpCodes.Ldarg_S, 10);
-                il.Emit(OpCodes.Newobj, fileAccessDataCtor);
-                il.Emit(OpCodes.Ret);
-
-                _createFileAccess = (CreateFileAccessFn)dynamicMethod.CreateDelegate(typeof(CreateFileAccessFn));
-            }
-            else
-            {
-                throw new MissingMethodException("Could not find supported constructor for FileAccessData");
-            }
         }
 
         public void RegisterOutput(string? filePath) => RegisterAccess(filePath, RequestedAccess.Write, DesiredAccess.GENERIC_WRITE);
@@ -338,7 +270,7 @@ private void RegisterAccess(string? filePath, RequestedAccess requestedAccess, D
                 return;
             }
 
-            FileAccessData fileAccessData = _createFileAccess(
+            FileAccessData fileAccessData = new(
                 ReportedFileOperation.CreateFile,
                 requestedAccess,
                 ProcessId,