Skip to content

Commit 0a99e80

Browse files
dfedermdfederm
authored
Consume requested targets when constructing the graph (#64)
1 parent 3927f98 commit 0a99e80

File tree

7 files changed

+52
-114
lines changed

7 files changed

+52
-114
lines changed

‎.azuredevops/pipelines/pr-ci.yml‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,12 @@ jobs:
5757

5858
- checkout: self
5959

60-
- task: Cache@2
61-
displayName: Cache Visual Studio
62-
inputs:
63-
key: '"vs-release" | "$(Agent.OS)"'
64-
path: $(VsInstallDir)
60+
# Avoiding Caching VS for now as the installer command we're using doesn't work for upgrades
61+
# - task: Cache@2
62+
# displayName: Cache Visual Studio
63+
# inputs:
64+
# key: '"vs-release" | "$(Agent.OS)"'
65+
# path: $(VsInstallDir)
6566

6667
- script: |
6768
del %TEMP%\vs_buildtools.exe
@@ -78,6 +79,10 @@ jobs:
7879
7980
del %TEMP%\vs_buildtools.exe
8081
82+
echo Copying installer logs
83+
mkdir "$(LogDirectory)\VSSetup"
84+
move "%TEMP%\dd_*" "$(LogDirectory)\VSSetup\"
85+
8186
echo Current MSBuild version:
8287
"$(MSBuildPath)" --version
8388
displayName: 'Install VS'
@@ -105,11 +110,12 @@ jobs:
105110

106111
- checkout: self
107112

108-
- task: Cache@2
109-
displayName: Cache Visual Studio
110-
inputs:
111-
key: '"vs-pre" | "$(Agent.OS)"'
112-
path: $(VsInstallDir)
113+
# Avoiding Caching VS for now as the installer command we're using doesn't work for upgrades
114+
# - task: Cache@2
115+
# displayName: Cache Visual Studio
116+
# inputs:
117+
# key: '"vs-pre" | "$(Agent.OS)"'
118+
# path: $(VsInstallDir)
113119

114120
- script: |
115121
del %TEMP%\vs_buildtools.exe
@@ -126,6 +132,10 @@ jobs:
126132
127133
del %TEMP%\vs_buildtools.exe
128134
135+
echo Copying installer logs
136+
mkdir "$(LogDirectory)\VSSetup"
137+
move "%TEMP%\dd_*" "$(LogDirectory)\VSSetup\"
138+
129139
echo Current MSBuild version:
130140
"$(MSBuildPath)" --version
131141
displayName: 'Install VS Preview'

‎.azuredevops/pipelines/templates/variables.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,4 @@ variables:
55
ArtifactsDirectory: $(Build.ArtifactStagingDirectory)\artifacts
66
# https://github.com/microsoft/azure-pipelines-agent/pull/4077
77
VSO_DEDUP_REDIRECT_TIMEOUT_IN_SEC: 5
8+
EnablePipelineCache: true

‎CONTRIBUTING.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ contact [[email protected]](mailto:[email protected]) with any additio
1414

1515
## Setup
1616

17-
It is assumed that you are using VS v17.8 or later.
17+
It is assumed that you are using VS v17.9 or later.
1818

1919
## Building
2020

‎Directory.Packages.props‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@
33
<PropertyGroup>
44
<ArtifactsPackageVersion>19.232.34508-buildid25403274</ArtifactsPackageVersion>
55
<BuildXLPackageVersion>0.1.0-20240121.1</BuildXLPackageVersion>
6-
<MSBuildPackageVersion>17.8.3</MSBuildPackageVersion>
6+
<MSBuildPackageVersion>17.9.5</MSBuildPackageVersion>
77
</PropertyGroup>
88
<ItemGroup>
99
<PackageVersion Include="coverlet.collector" Version="3.1.2" />
1010
<PackageVersion Include="DotNet.Glob" Version="2.0.3" />
1111
<PackageVersion Include="ILRepack" Version="2.0.27" />
12-
<PackageVersion Include="Microsoft.Bcl.AsyncInterfaces" Version="7.0.0" />
12+
<PackageVersion Include="Microsoft.Bcl.AsyncInterfaces" Version="8.0.0" />
1313
<PackageVersion Include="Microsoft.Bcl.HashCode" Version="1.1.1" />
1414
<PackageVersion Include="Microsoft.Build" Version="$(MSBuildPackageVersion)" />
1515
<PackageVersion Include="Microsoft.Build.Framework" Version="$(MSBuildPackageVersion)" />
@@ -39,14 +39,14 @@
3939
<PackageVersion Include="protobuf-net" Version="3.2.26" />
4040
<PackageVersion Include="protobuf-net.Core" Version="3.2.26" />
4141
<PackageVersion Include="protobuf-net.Grpc" Version="1.1.1" />
42-
<PackageVersion Include="System.Collections.Immutable" Version="7.0.0" />
42+
<PackageVersion Include="System.Collections.Immutable" Version="8.0.0" />
4343
<PackageVersion Include="System.Memory" Version="4.5.5" />
4444
<PackageVersion Include="System.Private.ServiceModel" Version="4.10.3" />
45-
<PackageVersion Include="System.Reflection.Metadata" Version="7.0.0" />
45+
<PackageVersion Include="System.Reflection.Metadata" Version="8.0.0" />
4646
<PackageVersion Include="System.Runtime.CompilerServices.Unsafe" Version="6.0.0" />
47-
<PackageVersion Include="System.Text.Json" Version="7.0.3" />
48-
<PackageVersion Include="System.Threading.Channels" Version="7.0.0" />
49-
<PackageVersion Include="System.Threading.Tasks.Dataflow" Version="7.0.0" />
47+
<PackageVersion Include="System.Text.Json" Version="8.0.0" />
48+
<PackageVersion Include="System.Threading.Channels" Version="8.0.0" />
49+
<PackageVersion Include="System.Threading.Tasks.Dataflow" Version="8.0.0" />
5050
</ItemGroup>
5151
<ItemGroup>
5252
<GlobalPackageReference Include="Microsoft.VisualStudioEng.MicroBuild.Core" Version="1.0.0" />

‎src/Common/MSBuildCachePluginBase.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,8 +273,8 @@ private async Task BeginBuildInnerAsync(CacheContext context, PluginLoggerBase l
273273
Parser parser = new(logger, _repoRoot);
274274
IReadOnlyDictionary<ProjectGraphNode, ParserInfo> parserInfoForNodes = parser.Parse(graph);
275275

276-
// TODO: MSBuild should give this to us via CacheContext
277-
var entryProjectTargets = Array.Empty<string>();
276+
// 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.
277+
ICollection<string> entryProjectTargets = context.RequestedTargets as ICollection<string> ?? new List<string>(context.RequestedTargets);
278278
IReadOnlyDictionary<ProjectGraphNode, ImmutableList<string>> targetListPerNode = graph.GetTargetLists(entryProjectTargets);
279279

280280
Dictionary<NodeDescriptor, NodeContext> nodeContexts = new(parserInfoForNodes.Count);

‎src/Repack.Tests/RepackTests.cs‎

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
using System.Collections.Generic;
66
using System.IO;
77
using System.Linq;
8-
using System.Reflection;
98
using Microsoft.MSBuildCache.AzureBlobStorage;
109
using Microsoft.MSBuildCache.AzurePipelines;
1110
using Microsoft.MSBuildCache.Tests;
@@ -17,33 +16,29 @@ namespace Microsoft.MSBuildCache.Repack.Tests;
1716
[TestClass]
1817
public class RepackTests
1918
{
20-
private static readonly Type[] TypesToCheck =
19+
[DataTestMethod]
20+
[DataRow(typeof(MSBuildCacheAzureBlobStoragePlugin))]
21+
[DataRow(typeof(MSBuildCacheAzurePipelinesPlugin))]
22+
[DataRow(typeof(MSBuildCacheLocalPlugin))]
23+
[DataRow(typeof(SharedCompilation.ResolveFileAccesses))]
24+
public void PluginInterfaceAssembliesNotMerged(Type typeToCheck)
2125
{
22-
typeof(MSBuildCacheAzureBlobStoragePlugin),
23-
typeof(MSBuildCacheAzurePipelinesPlugin),
24-
typeof(MSBuildCacheLocalPlugin),
25-
typeof(SharedCompilation.ResolveFileAccesses),
26-
};
26+
#if DEBUG
27+
// Using Assert.Inconclusive instead of removing the entire test for visibility that the test exists.
28+
Assert.Inconclusive("This test only applies to Release builds.");
29+
#endif
2730

28-
[TestMethod]
29-
public void PluginInterfaceAssembliesNotMerged()
30-
{
31-
foreach (Type type in TypesToCheck)
32-
{
33-
Dictionary<string, AssemblyName> references = type.Assembly
34-
.GetReferencedAssemblies()
35-
.Where(a => a.Name is not null)
36-
.ToDictionary(
37-
a => a.Name!,
38-
a => a
39-
);
31+
HashSet<string> references = typeToCheck.Assembly
32+
.GetReferencedAssemblies()
33+
.Where(a => a.Name is not null)
34+
.Select(reference => reference.Name!)
35+
.ToHashSet(StringComparer.Ordinal);
4036

41-
// Check to make sure that each of the interface assemblies are still actually referenced
42-
foreach (string expectedRefFileName in PluginInterfaceTypeCheckTests.PluginInterfaceNuGetAssemblies)
43-
{
44-
string expectedRef = Path.GetFileNameWithoutExtension(expectedRefFileName);
45-
Assert.IsNotNull(references.FirstOrDefault(a => a.Value.FullName.IndexOf(expectedRef, StringComparison.Ordinal) > 0));
46-
}
37+
// Check to make sure that each of the interface assemblies are still actually referenced
38+
foreach (string expectedRefFileName in PluginInterfaceTypeCheckTests.PluginInterfaceNuGetAssemblies)
39+
{
40+
string expectedRef = Path.GetFileNameWithoutExtension(expectedRefFileName);
41+
Assert.IsTrue(references.Contains(expectedRef));
4742
}
4843
}
4944
}

‎src/SharedCompilation/VBCSCompilerReporter.cs‎

Lines changed: 1 addition & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99
#endif
1010
using System.IO;
1111
using System.Linq;
12-
using System.Reflection;
13-
using System.Reflection.Emit;
1412
using Microsoft.Build.Experimental.FileAccess;
1513
using Microsoft.Build.Framework;
1614
using Microsoft.CodeAnalysis;
@@ -223,25 +221,10 @@ private sealed class AccessRegistrar
223221
{
224222
private delegate void ReportFileAccessFn(FileAccessData fileAccessData);
225223

226-
private delegate FileAccessData CreateFileAccessFn(
227-
ReportedFileOperation operation,
228-
RequestedAccess requestedAccess,
229-
uint processId,
230-
uint id,
231-
uint correlationId,
232-
uint error,
233-
DesiredAccess desiredAccess,
234-
FlagsAndAttributes flagsAndAttributes,
235-
string path,
236-
string processArgs,
237-
bool isAnAugmentedFileAccess);
238-
239224
private readonly string _basePath;
240225

241226
private readonly ReportFileAccessFn _reportFileAccess;
242227

243-
private readonly CreateFileAccessFn _createFileAccess;
244-
245228
private static readonly uint ProcessId = (uint)
246229
#if NET472
247230
Process.GetCurrentProcess().Id;
@@ -257,57 +240,6 @@ public AccessRegistrar(string basePath, EngineServices engineServices)
257240
// Use reflection to get the ReportFileAccess method since it's not exposed.
258241
// TODO: Once there is a proper API for this, use it.
259242
_reportFileAccess = (ReportFileAccessFn)Delegate.CreateDelegate(typeof(ReportFileAccessFn), engineServices, "ReportFileAccess");
260-
261-
ConstructorInfo fileAccessDataCtor = typeof(FileAccessData).GetConstructors()[0];
262-
ParameterInfo[] fileAccessDataCtorParams = fileAccessDataCtor.GetParameters();
263-
if (fileAccessDataCtorParams.Length == 9)
264-
{
265-
// This is the one we compiled against so just use it.
266-
_createFileAccess = (
267-
ReportedFileOperation operation,
268-
RequestedAccess requestedAccess,
269-
uint processId,
270-
uint id,
271-
uint correlationId,
272-
uint error,
273-
DesiredAccess desiredAccess,
274-
FlagsAndAttributes flagsAndAttributes,
275-
string path,
276-
string processArgs,
277-
bool isAnAugmentedFileAccess)
278-
=> new FileAccessData(operation, requestedAccess, processId, error, desiredAccess, flagsAndAttributes, path, processArgs, isAnAugmentedFileAccess);
279-
}
280-
else if (fileAccessDataCtorParams.Length == 11)
281-
{
282-
// Adjust to a breaking change made in MSBuild to the FileAccessData ctor. See: https://github.com/dotnet/msbuild/pull/9615
283-
// Create a dynamic method which basically just invokes the ctor with all the params from the CreateFileAccessFn delegate.
284-
DynamicMethod dynamicMethod = new(
285-
name: string.Empty,
286-
returnType: typeof(FileAccessData),
287-
parameterTypes: fileAccessDataCtorParams.Select(param => param.ParameterType).ToArray(),
288-
owner: typeof(FileAccessData));
289-
290-
ILGenerator il = dynamicMethod.GetILGenerator();
291-
il.Emit(OpCodes.Ldarg_0);
292-
il.Emit(OpCodes.Ldarg_1);
293-
il.Emit(OpCodes.Ldarg_2);
294-
il.Emit(OpCodes.Ldarg_3);
295-
il.Emit(OpCodes.Ldarg_S, 4);
296-
il.Emit(OpCodes.Ldarg_S, 5);
297-
il.Emit(OpCodes.Ldarg_S, 6);
298-
il.Emit(OpCodes.Ldarg_S, 7);
299-
il.Emit(OpCodes.Ldarg_S, 8);
300-
il.Emit(OpCodes.Ldarg_S, 9);
301-
il.Emit(OpCodes.Ldarg_S, 10);
302-
il.Emit(OpCodes.Newobj, fileAccessDataCtor);
303-
il.Emit(OpCodes.Ret);
304-
305-
_createFileAccess = (CreateFileAccessFn)dynamicMethod.CreateDelegate(typeof(CreateFileAccessFn));
306-
}
307-
else
308-
{
309-
throw new MissingMethodException("Could not find supported constructor for FileAccessData");
310-
}
311243
}
312244

313245
public void RegisterOutput(string? filePath) => RegisterAccess(filePath, RequestedAccess.Write, DesiredAccess.GENERIC_WRITE);
@@ -338,7 +270,7 @@ private void RegisterAccess(string? filePath, RequestedAccess requestedAccess, D
338270
return;
339271
}
340272

341-
FileAccessData fileAccessData = _createFileAccess(
273+
FileAccessData fileAccessData = new(
342274
ReportedFileOperation.CreateFile,
343275
requestedAccess,
344276
ProcessId,

0 commit comments

Comments
 (0)