Skip to content

Commit 5c3434d

Browse files
authored
Add PR pipeline stage to validate VMR changes (#1270)
1 parent 599f6c2 commit 5c3434d

14 files changed

+515
-24
lines changed

.github/workflows/protected-files-validation.yml

Lines changed: 0 additions & 22 deletions
This file was deleted.

Directory.Packages.props

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
<ItemGroup>
1111
<!-- Arcade dependencies -->
1212
<PackageVersion Include="Microsoft.DotNet.Build.Manifest" Version="$(MicrosoftDotNetBuildManifestVersion)" />
13+
<!-- Arcade-services dependencies -->
14+
<PackageVersion Include="Microsoft.DotNet.DarcLib" Version="$(MicrosoftDotNetDarcLibVersion)" />
1315
<!-- Command-line-api dependencies -->
1416
<PackageVersion Include="System.CommandLine" Version="$(SystemCommandLineVersion)" />
1517
<!-- MSBuild dependencies -->
@@ -22,6 +24,7 @@
2224
<PackageVersion Include="Microsoft.Extensions.FileSystemGlobbing" Version="$(MicrosoftExtensionsFileSystemGlobbingVersion)" />
2325
<PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="$(MicrosoftExtensionsLoggingConsoleVersion)" />
2426
<PackageVersion Include="Microsoft.Extensions.Logging" Version="$(MicrosoftExtensionsLoggingVersion)" />
27+
<PackageVersion Include="Microsoft.Extensions.Hosting" Version="$(MicrosoftExtensionsHostingVersion)" />
2528
</ItemGroup>
2629

2730
<!-- External dependencies -->

eng/Version.Details.props

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,6 @@ This file should be imported by eng/Versions.props
1717
<MicrosoftDotNetArcadeSdkVersion>$(MicrosoftDotNetArcadeSdkPackageVersion)</MicrosoftDotNetArcadeSdkVersion>
1818
<MicrosoftDotNetBuildManifestVersion>$(MicrosoftDotNetBuildManifestPackageVersion)</MicrosoftDotNetBuildManifestVersion>
1919
<!-- dotnet/arcade-services dependencies -->
20+
<MicrosoftDotNetDarcLibVersion>1.1.0-beta.25407.5</MicrosoftDotNetDarcLibVersion>
2021
</PropertyGroup>
2122
</Project>

eng/Version.Details.xml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,9 @@
1414
<Uri>https://github.com/dotnet/arcade-services</Uri>
1515
<Sha>396eba49bd539a8791adbe3ef60fcbaa02d60b13</Sha>
1616
</Dependency>
17+
<Dependency Name="Microsoft.DotNet.DarcLib" Version="1.1.0-beta.25407.5">
18+
<Uri>arcade-services</Uri>
19+
<Sha>cf3578623981f08c57b1bec18eaacdeaedb479f6</Sha>
20+
</Dependency>
1721
</ToolsetDependencies>
1822
</Dependencies>

eng/Versions.props

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
-->
2828
<PrivateSourceBuiltSdkVersion>10.0.100-rc.1.25411.109</PrivateSourceBuiltSdkVersion>
2929
<PrivateSourceBuiltArtifactsVersion>10.0.100-rc.1.25411.109</PrivateSourceBuiltArtifactsVersion>
30+
<!-- arcade-services dependencies -->
31+
<MicrosoftDotNetDarcLibVersion>1.1.0-beta.25404.1</MicrosoftDotNetDarcLibVersion>
3032
<!-- command-line-api dependencies -->
3133
<SystemCommandLineVersion>2.0.0-beta5.25208.1</SystemCommandLineVersion>
3234
<!-- msbuild dependencies -->
@@ -40,6 +42,6 @@
4042
<MicrosoftExtensionsLoggingVersion>9.0.0</MicrosoftExtensionsLoggingVersion>
4143
<!-- external dependencies -->
4244
<NewtonsoftJsonVersion>13.0.3</NewtonsoftJsonVersion>
43-
<OctokitVersion>10.0.0</OctokitVersion>
45+
<OctokitVersion>13.0.1</OctokitVersion>
4446
</PropertyGroup>
4547
</Project>

eng/pipelines/pr.yml

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,3 +46,41 @@ stages:
4646
scope: lite
4747
${{ else }}:
4848
scope: full
49+
50+
- stage: ValidateUserChanges
51+
displayName: Validate user changes in VMR
52+
dependsOn: []
53+
condition: and(
54+
eq(variables['isPRTrigger'], 'true'),
55+
and(
56+
ne(variables['Build.RequestedForEmail'], 'maestro-dotnet-bot@microsoft.com'),
57+
ne(variables['Build.RequestedForEmail'], 'dotnet-sb-bot@microsoft.com')
58+
)
59+
)
60+
jobs:
61+
- job: Validate
62+
displayName: Run Validation Script
63+
pool:
64+
vmImage: windows-latest
65+
steps:
66+
- checkout: self
67+
fetchDepth: 0
68+
69+
- powershell: |
70+
Write-Host "Fetching target branch: $(System.PullRequest.TargetBranch)"
71+
git fetch origin $(System.PullRequest.TargetBranch)
72+
displayName: Fetch target branch
73+
74+
- powershell: |
75+
. "$(Build.SourcesDirectory)\eng\common\tools.ps1"
76+
InitializeDotNetCli -install $true
77+
Write-Host "##vso[task.setvariable variable=DOTNET_INSTALL_DIR]$env:DOTNET_INSTALL_DIR"
78+
displayName: Initialize .NET CLI
79+
80+
- powershell: |
81+
& "$(DOTNET_INSTALL_DIR)\dotnet.exe" run --project eng/tools/ChangeValidation/ChangeValidation.csproj
82+
displayName: Run ChangeValidation tool
83+
env:
84+
SYSTEM_PULLREQUEST_TARGETBRANCH: $(System.PullRequest.TargetBranch)
85+
SYSTEM_PULLREQUEST_SOURCEBRANCH: $(System.PullRequest.SourceBranch)
86+
SYSTEM_PULLREQUEST_PULLREQUESTNUMBER: $(System.PullRequest.PullRequestNumber)
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
  <PropertyGroup>
3+
    <OutputType>Exe</OutputType>
4+
    <TargetFramework>$(NetCurrent)</TargetFramework>
5+
    <Nullable>enable</Nullable>
6+
    <ImplicitUsings>enable</ImplicitUsings>
7+
</PropertyGroup>
8+
9+
<ItemGroup>
10+
<PackageReference Include="Microsoft.Extensions.FileSystemGlobbing" />
11+
<PackageReference Include="Microsoft.Extensions.Hosting" />
12+
<PackageReference Include="Microsoft.Extensions.Logging" />
13+
<PackageReference Include="Microsoft.DotNet.DarcLib" />
14+
</ItemGroup>
15+
16+
</Project>
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using Microsoft.Extensions.Logging;
5+
using Microsoft.Extensions.Logging.Abstractions;
6+
using Microsoft.Extensions.FileSystemGlobbing;
7+
using Microsoft.Extensions.FileSystemGlobbing.Abstractions;
8+
using Microsoft.DotNet.DarcLib;
9+
using Microsoft.DotNet.DarcLib.VirtualMonoRepo;
10+
using Microsoft.DotNet.DarcLib.Models.VirtualMonoRepo;
11+
using System.Linq;
12+
using System.Collections.Generic;
13+
using System.Text;
14+
using Newtonsoft.Json.Linq;
15+
using Newtonsoft.Json;
16+
using Microsoft.DotNet.DarcLib.Helpers;
17+
using static ChangeValidation.Validation;
18+
19+
namespace ChangeValidation;
20+
21+
internal class ExclusionFileValidation : IValidationStep
22+
{
23+
private static readonly int maxDisplayedFiles = 20;
24+
private readonly IVmrDependencyTracker _dependencyTracker;
25+
private readonly IProcessManager _processManager;
26+
private readonly IVmrInfo _vmrInfo;
27+
private readonly ILocalGitRepoFactory _localGitRepoFactory;
28+
29+
public ExclusionFileValidation(
30+
IVmrDependencyTracker dependencyTracker,
31+
IProcessManager processManager,
32+
ILocalGitRepoFactory localGitRepoFactory,
33+
IVmrInfo vmrInfo)
34+
{
35+
_dependencyTracker = dependencyTracker;
36+
_processManager = processManager;
37+
_localGitRepoFactory = localGitRepoFactory;
38+
_vmrInfo = vmrInfo;
39+
}
40+
41+
public string DisplayName => "Exclusion File Validation";
42+
43+
public async Task<bool> Validate(PrInfo prInfo)
44+
{
45+
ILocalGitRepo vmr = _localGitRepoFactory.Create(_vmrInfo.VmrPath);
46+
47+
var newExclusionRules = await GetExclusionPatterns(); // We are already checked to the PR Head commit, just parse exclusions from file
48+
var originalExclusionRules = await GetExclusionPatternsFromBranch(vmr, prInfo.TargetBranch);
49+
50+
var newExcludedFiles = FindMatchingFiles(newExclusionRules);
51+
var originalExcludedFiles = FindMatchingFiles(originalExclusionRules);
52+
53+
var excludedFilesInPr = prInfo.ChangedFiles
54+
.Where(file => newExcludedFiles.Contains(NormalizePath(file)))
55+
.ToList();
56+
57+
var filesMatchingNewExclusionRules = newExcludedFiles
58+
.Where(file => !originalExcludedFiles.Contains(NormalizePath(file)) && !prInfo.ChangedFiles.Contains(NormalizePath(file)))
59+
.ToList();
60+
61+
foreach (var file in excludedFilesInPr.Take(maxDisplayedFiles).ToList())
62+
{
63+
LogError($"This PR modifies the file `{file}`, which is part of the excluded files" +
64+
$" defined in {VmrInfo.DefaultRelativeSourceMappingsPath}. If these changes are" +
65+
$" necessary, please contact @dotnet/product-construction.");
66+
}
67+
68+
if (excludedFilesInPr.Count > maxDisplayedFiles)
69+
{
70+
LogError($"... {excludedFilesInPr.Count - maxDisplayedFiles} more excluded files detected" +
71+
$" in the PR (only showing the first {maxDisplayedFiles} files).");
72+
}
73+
74+
foreach (var file in filesMatchingNewExclusionRules.Take(maxDisplayedFiles).ToList())
75+
{
76+
LogError($"The new exclusion rules defined in {VmrInfo.DefaultRelativeSourceMappingsPath} " +
77+
$"include the VMR file `{file}`. If this file is not needed in the VMR, consider " +
78+
$"deleting it as part of the PR.");
79+
}
80+
81+
if (filesMatchingNewExclusionRules.Count > maxDisplayedFiles)
82+
{
83+
LogError($"... {filesMatchingNewExclusionRules.Count - maxDisplayedFiles} more VMR files " +
84+
$"match the new exclusion rules (only showing the first {maxDisplayedFiles} files). " +
85+
"Please review the modifications made to exclusion rules");
86+
}
87+
88+
return !excludedFilesInPr.Any() && !filesMatchingNewExclusionRules.Any();
89+
}
90+
91+
private async Task<List<string>> GetExclusionPatternsFromBranch(ILocalGitRepo vmr, string branchName)
92+
{
93+
string originalRef = await vmr.GetCheckedOutBranchAsync();
94+
try
95+
{
96+
LogInfo($"Checking out branch {branchName} to get exclusion patterns.");
97+
await vmr.CheckoutAsync(branchName);
98+
return await GetExclusionPatterns();
99+
}
100+
finally
101+
{
102+
await vmr.CheckoutAsync(originalRef);
103+
}
104+
}
105+
106+
private async Task<List<string>> GetExclusionPatterns()
107+
{
108+
await _dependencyTracker.RefreshMetadata();
109+
110+
var sourceMappings = _dependencyTracker.Mappings;
111+
112+
var exclusionPatterns = sourceMappings.SelectMany(mapping => GetVmrGlobFromSourceMapping(mapping))
113+
.Distinct()
114+
.ToList();
115+
116+
LogInfo("Successfully parsed exclusion patterns...");
117+
LogInfo(string.Join(Environment.NewLine, exclusionPatterns));
118+
119+
return exclusionPatterns;
120+
}
121+
122+
private List<string> GetVmrGlobFromSourceMapping(SourceMapping mapping)
123+
{
124+
return mapping.Exclude.Select(exclusion => "src/" + mapping.Name + "/" + NormalizePath(exclusion)).ToList();
125+
}
126+
127+
private HashSet<string> FindMatchingFiles(List<string> globPatterns)
128+
{
129+
var matcher = new Matcher();
130+
131+
foreach (var pattern in globPatterns)
132+
{
133+
matcher.AddInclude(pattern);
134+
}
135+
136+
var directoryInfo = new DirectoryInfo(_vmrInfo.VmrPath);
137+
var directoryWrapper = new DirectoryInfoWrapper(directoryInfo);
138+
var result = matcher.Execute(directoryWrapper);
139+
140+
var res = result.Files
141+
.Select(file => file.Path)
142+
.ToHashSet();
143+
144+
return res;
145+
}
146+
147+
internal static string NormalizePath(string path)
148+
{
149+
return path.Replace('\\', '/');
150+
}
151+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
namespace ChangeValidation;
5+
6+
internal interface IValidationStep
7+
{
8+
/// <summary>
9+
/// Executes the validation step with the provided file names.
10+
/// </summary>
11+
/// <param name="fileNames">List of file names to validate.</param>
12+
/// <returns>boolean determining the success of the validation step</returns>
13+
Task<bool> Validate(PrInfo prInfo);
14+
15+
/// <summary>
16+
/// Gets the display name of the validation step.
17+
/// </summary>
18+
string DisplayName { get; }
19+
}

eng/tools/ChangeValidation/PrInfo.cs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Collections.Immutable;
6+
using System.Linq;
7+
8+
namespace ChangeValidation;
9+
10+
internal record PrInfo(
11+
string TargetBranch,
12+
ImmutableList<string> ChangedFiles);

0 commit comments

Comments
 (0)