Skip to content

Commit 877b66d

Browse files
thalbernthalbern
authored
Merge pull request #7043 from keveleigh/cherrypick-f4d872c13550123cde37d99625448e7bcb94af19
Cherry-pick #7027 to stabilization
2 parents 21f0a6d + 2a48c5e commit 877b66d

File tree

9 files changed

+677
-300
lines changed

9 files changed

+677
-300
lines changed

‎pipelines/pr.yaml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,8 @@ jobs:
2222
# the ongoing rolling CI build. This build in particular is highly correlated
2323
# the .NET x86 build and ARM IL2CPP build.
2424
buildUWPX86: false
25+
# For mrtk_pr builds, validation scripts (code and docs) should be scoped to only
26+
# the set of changed files, so that we can save some time. There's no need to
27+
# check unchanged files for code style/doc style violations.
28+
scopedValidation: true
2529
- template: templates/end.yml

‎pipelines/templates/common.yml‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,31 @@ parameters:
55
buildUWPDotNet: true
66
buildUWPX86: true
77
runTests: true
8+
# If true, validation scripts will be run on code and docs changes. By default
9+
# validation will run on every single line of code and docs. It's possible to
10+
# scope this to only changed files within a given pull request by using
11+
# the scopedValidation parameter below.
812
validateCode: true
13+
# If true, validation scripts will only be run on the set of changed files
14+
# associated with a given pull request. Note that this is only valid to use
15+
# for pull request validation, since this functionality requires a pull request
16+
# ID in order to determine the two commits to diff. Only relevant if validateCode
17+
# is true.
18+
scopedValidation: false
919

1020
steps:
21+
# Generate the list of changed files in this pull request.
22+
- ${{ if and(eq(parameters.validateCode, true), eq(parameters.scopedValidation, true)) }}:
23+
- template: tasks/githubchanges.yml
24+
parameters:
25+
changesFile: '$(Build.ArtifactStagingDirectory)\build\changedfiles.txt'
26+
1127
# Validate that the code doesn't contain common issues
1228
- ${{ if eq(parameters.validateCode, true) }}:
1329
- template: tasks/validatecode.yml
30+
parameters:
31+
scopedValidation: ${{ parameters.scopedValidation }}
32+
changesFile: '$(Build.ArtifactStagingDirectory)\build\changedfiles.txt'
1433

1534
# Build Standalone x86.
1635
# This must happen before tests run, because if the build fails and tests attempt

‎pipelines/templates/tasks/githubchanges.yml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
parameters:
2+
# The output file that will contain the list of changed files in this
3+
# pull request.
4+
changesFile: ''
5+
6+
steps:
7+
- task: PowerShell@2
8+
displayName: 'Get PullRequest Changes'
9+
inputs:
10+
targetType: filePath
11+
filePath: ./scripts/ci/githubchanges.ps1
12+
arguments: >
13+
-PullRequestId: $(System.PullRequest.PullRequestNumber)
14+
-TargetBranch: '$(System.PullRequest.TargetBranch)'
15+
-OutputFile: '${{ parameters.changesFile }}'
16+
-RepoRoot: '$(Build.SourcesDirectory)'

‎pipelines/templates/tasks/validatecode.yml‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,29 @@
1+
parameters:
2+
# If true, validation scripts will only be run on the set of changed files
3+
# associated with a given pull request. Note that this is only valid to use
4+
# for pull request validation, since this functionality requires a pull request
5+
# ID in order to determine the two commits to diff. Only relevant if validateCode
6+
# is true.
7+
scopedValidation: false
8+
# The file containing the list of changes, used if scopedValidation is true.
9+
changesFile: ''
10+
111
steps:
2-
- task: PowerShell@2
3-
displayName: 'Validate code'
4-
inputs:
5-
targetType: filePath
6-
filePath: ./scripts/ci/validatecode.ps1
7-
arguments: >
8-
-Directory: '$(Build.SourcesDirectory)\Assets'
12+
- ${{ if eq(parameters.scopedValidation, false) }}:
13+
- task: PowerShell@2
14+
displayName: 'Global code validation'
15+
inputs:
16+
targetType: filePath
17+
filePath: ./scripts/ci/validatecode.ps1
18+
arguments: >
19+
-Directory: '$(Build.SourcesDirectory)\Assets'
20+
- ${{ if eq(parameters.scopedValidation, true) }}:
21+
- task: PowerShell@2
22+
displayName: 'Scoped code validation'
23+
inputs:
24+
targetType: filePath
25+
filePath: ./scripts/ci/validatecode.ps1
26+
arguments: >
27+
-Directory: '$(Build.SourcesDirectory)\Assets'
28+
-ChangesFile: '${{ parameters.changesFile }}'
29+
-RepoRoot: '$(Build.SourcesDirectory)'

‎scripts/ci/common.psm1‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
<#
2+
.SYNOPSIS
3+
Given the path to the list of raw git changes, returns an array of
4+
those changes rooted in the git root directory.
5+
.DESCRIPTION
6+
For example, the raw git changes will contain lines like:
7+
8+
Assets/File.cs
9+
10+
This function will return a list of paths that look like (assuming
11+
that RepoRoot is C:\repo):
12+
13+
C:\repo\Assets\File.cs
14+
#>
15+
function GetChangedFiles {
16+
[CmdletBinding()]
17+
param(
18+
[string]$Filename,
19+
[string]$RepoRoot
20+
)
21+
process {
22+
$rawContent = Get-Content -Path $Filename
23+
$processedContent = @()
24+
foreach ($line in $rawContent) {
25+
$joinedPath = Join-Path -Path $RepoRoot -ChildPath $line
26+
$processedContent += $joinedPath
27+
}
28+
$processedContent
29+
}
30+
}
31+
32+
<#
33+
.SYNOPSIS
34+
Returns true if the given file is a markdown document and
35+
false otherwise. Uses the extension of the file, not the actual
36+
content to determine this.
37+
#>
38+
function IsMarkdownFile {
39+
[CmdletBinding()]
40+
param(
41+
[string]$Filename
42+
)
43+
process {
44+
[IO.Path]::GetExtension($Filename).ToLower() -eq ".md"
45+
}
46+
}
47+
48+
<#
49+
.SYNOPSIS
50+
Returns true if the given file is a csharp file and
51+
false otherwise. Uses the extension of the file, not the actual
52+
content to determine this.
53+
#>
54+
function IsCSharpFile {
55+
[CmdletBinding()]
56+
param(
57+
[string]$Filename
58+
)
59+
process {
60+
[IO.Path]::GetExtension($Filename).ToLower() -eq ".cs"
61+
}
62+
}
63+
64+
<#
65+
.SYNOPSIS
66+
Returns true if the given file is a .asset file and
67+
false otherwise. Uses the extension of the file, not the actual
68+
content to determine this.
69+
#>
70+
function IsAssetFile {
71+
[CmdletBinding()]
72+
param(
73+
[string]$Filename
74+
)
75+
process {
76+
[IO.Path]::GetExtension($Filename).ToLower() -eq ".asset"
77+
}
78+
}
79+
80+
<#
81+
.SYNOPSIS
82+
Returns true if the given file is a Unity scene and
83+
false otherwise. Uses the extension of the file, not the actual
84+
content to determine this.
85+
#>
86+
function IsUnityScene {
87+
[CmdletBinding()]
88+
param(
89+
[string]$Filename
90+
)
91+
process {
92+
[IO.Path]::GetExtension($Filename).ToLower() -eq ".unity"
93+
}
94+
}
95+
96+
<#
97+
.SYNOPSIS
98+
Returns true if the given file is a Unity meta file and
99+
false otherwise. Uses the extension of the file, not the actual
100+
content to determine this.
101+
#>
102+
function IsMetaFile {
103+
[CmdletBinding()]
104+
param(
105+
[string]$Filename
106+
)
107+
process {
108+
[IO.Path]::GetExtension($Filename).ToLower() -eq ".meta"
109+
}
110+
}

‎scripts/ci/githubchanges.ps1‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<#
2+
.SYNOPSIS
3+
This script generates a list of changed files in a given pull request and outputs
4+
that list to a file.
5+
.DESCRIPTION
6+
Generates a file containing a list of all modified files (added/modified) in
7+
the given pull request. The output file contains a list that is newline delimited, for
8+
example:
9+
10+
Assets/MixedRealityToolkit.SDK/AssemblyInfo.cs
11+
Assets/MixedRealityToolkit.Tests/PlayModeTests/ManipulationHandlerTests.cs
12+
13+
The file will not contain files that have been completely deleted (for example, if
14+
a change deleted Assets/MixedRealityToolkit.SDK/DeletedFile.cs), that line will not exist
15+
in the output file.
16+
17+
Note that this script assumes that the local git repo doesn't already contain the
18+
target branch (e.g. mrtk_development). This is what happens by default on Azure DevOps
19+
pipeline integrations with Github pull requests.
20+
21+
In particular, this will checkout (via this command:
22+
git fetch --force --tags --prune --progress --no-recurse-submodules origin $(System.PullRequest.TargetBranch))
23+
.EXAMPLE
24+
.\githubchanges.ps1 -OutputFile c:\path\to\changes\file.txt -PullRequestId 1234 -RepoRoot c:\path\to\mrtk -TargetBranch mrtk_development
25+
#>
26+
param(
27+
# The ID of the pull request. (e.g. 1234)
28+
[string]$PullRequestId,
29+
30+
# The target branch that the pull request will merge into (e.g. mrtk_development)
31+
[string]$TargetBranch,
32+
33+
# The output filename (e.g. c:\path\to\output.txt)
34+
[Parameter(Mandatory=$true)]
35+
[string]$OutputFile,
36+
37+
# The root folder of the repo (e.g. c:\repo)
38+
# This primarily used to filter out files that were deleted.
39+
[Parameter(Mandatory=$true)]
40+
[string]$RepoRoot
41+
)
42+
43+
# The pull request ID might not be present (i.e. this is an adhoc build being spun up)
44+
# and the target branch might not be set in which case there's nothing to validate.
45+
if ([string]::IsNullOrEmpty($PullRequestId) -or [string]::IsNullOrEmpty($TargetBranch))
46+
{
47+
Write-Warning "PullRequestId or TargetBranch aren't specified, skipping."
48+
exit 0;
49+
}
50+
51+
# If the output file already exists, blow it away. Each run should get a new set of changed files.
52+
if (Test-Path $OutputFile -PathType leaf) {
53+
Remove-Item $OutputFile
54+
}
55+
New-Item -ItemType File -Force -Path $OutputFile
56+
57+
# The path to the .git file is necessary when invoking the git command below, as the working
58+
# directory may not actually be pointed toward the git path.
59+
$gitDir = Join-Path -Path $RepoRoot -ChildPath ".git"
60+
61+
# Fetches the target branch so that the git diffing down below will actually be possible. git diff will list
62+
# the set of changed files between two different commit stamps (or branches, in this case), and needs
63+
# both branches to exist in order to make this happen.
64+
# Uses a shallow fetch (i.e. depth=1) because only the latest commit from the target branch is
65+
# needed to do the diff.
66+
git --git-dir=$gitDir --work-tree=$RepoRoot fetch --depth=1 --force --tags --prune --progress --no-recurse-submodules origin $TargetBranch
67+
68+
# The set of changed files is the diff between the target branch and the pull request
69+
# branch that was checked out locally. Note that the format of the pull request branch
70+
# (i.e. "pull/$PullRequestId/merge") is based on the format that Azure DevOps does for its
71+
# local checkout of the pull request code.
72+
# WARNING: This is a loose dependency on Azure DevOps git checkout mechanism - if this errors out
73+
# we'd likely need to another fetch. Something like:
74+
#
75+
# git fetch origin pull/$PullRequestId/head:local_branch
76+
# $changedFiles=$(git --git-dir=$gitDir --work-tree=$RepoRoot diff --name-only local_branch origin/$TargetBranch 2>&1)
77+
$changedFiles=$(git --git-dir=$gitDir --work-tree=$RepoRoot diff --name-only pull/$PullRequestId/merge origin/$TargetBranch 2>&1)
78+
79+
foreach ($changedFile in $changedFiles) {
80+
$joinedPath = Join-Path -Path $RepoRoot -ChildPath $changedFile
81+
# Only save the path if the file still exists - also, do not store the absolute path
82+
# of the file, in case this set of information is used later in the pipeline on a different
83+
# machine/context.
84+
if (Test-Path $joinedPath -PathType leaf) {
85+
Add-Content -Path $OutputFile -Value $changedFile
86+
}
87+
}

‎scripts/ci/validatebuildlog.ps1‎

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,42 +8,45 @@
88
because they are not build errors.
99
1010
Returns 0 if there are no errors, non-zero if there are.
11-
.PARAMETER LogFile
12-
The log file to validate
1311
.EXAMPLE
1412
.\validatebuildlog.ps1 -LogFile c:\path\to\log.txt
1513
#>
1614
param(
15+
# The log file to validate
1716
[string]$LogFile
1817
)
1918

20-
function CheckDuplicateGuids(
21-
[string[]]$LogFileContent,
22-
[int]$LineNumber
23-
) {
24-
<#
25-
.SYNOPSIS
26-
Checks if the given log file has a conflict guid at the specified line number.
27-
Outputs to console if such an error exists.
28-
Returns true if there is a duplicate guid.
29-
#>
30-
if ($LogFileContent[$LineNumber] -match "GUID \[[a-g0-9]{32}?\] for asset '.*' conflicts with") {
31-
for ($i = $LineNumber; $i -lt $LogFileContent.Length; $i++) {
32-
if ($LogFileContent[$i] -eq "Assigning a new guid.") {
33-
Write-Host "Found duplicated GUID, Unity will non-deterministically use one of them, please manually "
34-
Write-Host "regenerate the intended one by deleting the .meta file and re-opening the Unity editor locally."
19+
<#
20+
.SYNOPSIS
21+
Checks if the given log file has a conflict guid at the specified line number.
22+
Outputs to console if such an error exists.
23+
Returns true if there is a duplicate guid.
24+
#>
25+
function CheckDuplicateGuids {
26+
[CmdletBinding()]
27+
param(
28+
[string[]]$LogFileContent,
29+
[int]$LineNumber
30+
)
31+
process {
32+
if ($LogFileContent[$LineNumber] -match "GUID \[[a-g0-9]{32}?\] for asset '.*' conflicts with") {
33+
for ($i = $LineNumber; $i -lt $LogFileContent.Length; $i++) {
34+
if ($LogFileContent[$i] -eq "Assigning a new guid.") {
35+
Write-Host "Found duplicated GUID, Unity will non-deterministically use one of them, please manually "
36+
Write-Host "regenerate the intended one by deleting the .meta file and re-opening the Unity editor locally."
3537

36-
# Found the end of the guid conflict message - output to the console
37-
# all lines between these two locations (including the assigning a new guid message
38-
# in case it falls on the same line)
39-
for ($j = $LineNumber; $j -le $i; $j++) {
40-
Write-Host $LogFileContent[$j]
38+
# Found the end of the guid conflict message - output to the console
39+
# all lines between these two locations (including the assigning a new guid message
40+
# in case it falls on the same line)
41+
for ($j = $LineNumber; $j -le $i; $j++) {
42+
Write-Host $LogFileContent[$j]
43+
}
44+
$true
4145
}
42-
return $true
4346
}
4447
}
48+
$false
4549
}
46-
return $false
4750
}
4851

4952
if (-not $LogFile) {

0 commit comments

Comments
 (0)