Skip to content

Commit 315c69f

Browse files
azure-sdkscbedd
azure-sdk
and
scbedd
authored
Sync eng/common directory with azure-sdk-tools for PR 9993 (Azure#2317)
* optimize save-package-properties for large diffs --------- Co-authored-by: Scott Beddall <[email protected]>
1 parent c784ff4 commit 315c69f

File tree

1 file changed

+164
-62
lines changed

1 file changed

+164
-62
lines changed

‎eng/common/scripts/Package-Properties.ps1‎

Lines changed: 164 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -128,12 +128,35 @@ class PackageProps {
128128
if ($ciArtifactResult) {
129129
$this.ArtifactDetails = [Hashtable]$ciArtifactResult.ArtifactConfig
130130

131+
$repoRoot = Resolve-Path (Join-Path $PSScriptRoot ".." ".." "..")
132+
$ciYamlPath = (Resolve-Path -Path $ciArtifactResult.Location -Relative -RelativeBasePath $repoRoot).TrimStart(".").Replace("`\", "/")
133+
$relRoot = [System.IO.Path]::GetDirectoryName($ciYamlPath).Replace("`\", "/")
134+
131135
if (-not $this.ArtifactDetails["triggeringPaths"]) {
132136
$this.ArtifactDetails["triggeringPaths"] = @()
133137
}
134-
$RepoRoot = Resolve-Path (Join-Path $PSScriptRoot ".." ".." "..")
135-
$relativePath = (Resolve-Path -Path $ciArtifactResult.Location -Relative -RelativeBasePath $RepoRoot).TrimStart(".").Replace("`\", "/")
136-
$this.ArtifactDetails["triggeringPaths"] += $relativePath
138+
else {
139+
$adjustedPaths = @()
140+
141+
# we need to convert relative references to absolute references within the repo
142+
# this will make it extremely easy to compare triggering paths to files in the deleted+changed file list.
143+
for ($i = 0; $i -lt $this.ArtifactDetails["triggeringPaths"].Count; $i++) {
144+
$currentPath = $this.ArtifactDetails["triggeringPaths"][$i]
145+
$newPath = Join-Path $repoRoot $currentPath
146+
if (!$currentPath.StartsWith("/")) {
147+
$newPath = Join-Path $repoRoot $relRoot $currentPath
148+
}
149+
# it is a possibility that users may have a triggerPath dependency on a file that no longer exists.
150+
# before we resolve it to get rid of possible relative references, we should check if the file exists
151+
# if it doesn't, we should just leave it as is. Otherwise we would _crash_ here when a user accidentally
152+
# left a triggeringPath on a file that had been deleted
153+
if (Test-Path $newPath) {
154+
$adjustedPaths += (Resolve-Path -Path $newPath -Relative -RelativeBasePath $repoRoot).TrimStart(".").Replace("`\", "/")
155+
}
156+
}
157+
$this.ArtifactDetails["triggeringPaths"] = $adjustedPaths
158+
}
159+
$this.ArtifactDetails["triggeringPaths"] += $ciYamlPath
137160

138161
$this.CIParameters["CIMatrixConfigs"] = @()
139162

@@ -180,8 +203,86 @@ function Get-PkgProperties {
180203
return $null
181204
}
182205

206+
207+
function Get-TriggerPaths([PSCustomObject]$AllPackageProperties) {
208+
$existingTriggeringPaths = @()
209+
$AllPackageProperties | ForEach-Object {
210+
if ($_.ArtifactDetails) {
211+
$pathsForArtifact = $_.ArtifactDetails["triggeringPaths"]
212+
foreach ($triggerPath in $pathsForArtifact){
213+
# we only care about triggering paths that are actual files, not directories
214+
# go by by the assumption that if the triggerPath has an extension, it's a file :)
215+
if ([System.IO.Path]::HasExtension($triggerPath)) {
216+
$existingTriggeringPaths += $triggerPath
217+
}
218+
}
219+
}
220+
}
221+
222+
return ($existingTriggeringPaths | Select-Object -Unique)
223+
}
224+
225+
function Update-TargetedFilesForTriggerPaths([string[]]$TargetedFiles, [string[]]$TriggeringPaths) {
226+
# now we simply loop through the files a single time, keeping all the files that are a triggeringPath
227+
# for the rest of the files, simply group by what directory they belong to
228+
# the new TargetedFiles array will contain only the changed directories + the files that actually aligned to a triggeringPath
229+
$processedFiles = @()
230+
$Triggers = [System.Collections.ArrayList]$TriggeringPaths
231+
$i = 0
232+
foreach ($file in $TargetedFiles) {
233+
$isExistingTriggerPath = $false
234+
235+
for ($i = 0; $i -lt $Triggers.Count; $i++) {
236+
$triggerPath = $Triggers[$i]
237+
if ($triggerPath -and $file -eq "$triggerPath") {
238+
$isExistingTriggerPath = $true
239+
break
240+
}
241+
}
242+
243+
if ($isExistingTriggerPath) {
244+
# we know that we should have a valid $i that we can use to remove the triggerPath from the list
245+
# so that it gets smaller as we find items
246+
$Triggers.RemoveAt($i)
247+
$processedFiles += $file
248+
}
249+
else {
250+
# Get directory path by removing the filename
251+
$directoryPath = Split-Path -Path $file -Parent
252+
if ($directoryPath) {
253+
$processedFiles += $directoryPath
254+
} else {
255+
# In case there's no parent directory (root file), keep the original
256+
$processedFiles += $file
257+
}
258+
}
259+
}
260+
261+
return ($processedFiles | Select-Object -Unique)
262+
}
263+
264+
function Update-TargetedFilesForExclude([string[]]$TargetedFiles, [string[]]$ExcludePaths) {
265+
$files = @()
266+
foreach ($file in $TargetedFiles) {
267+
$shouldExclude = $false
268+
foreach ($exclude in $ExcludePaths) {
269+
if (!$file.StartsWith($exclude,'CurrentCultureIgnoreCase')) {
270+
$shouldExclude = $true
271+
break
272+
}
273+
}
274+
if (!$shouldExclude) {
275+
$files += $file
276+
}
277+
}
278+
return ,$files
279+
}
280+
183281
function Get-PrPkgProperties([string]$InputDiffJson) {
184282
$packagesWithChanges = @()
283+
$additionalValidationPackages = @()
284+
$lookup = @{}
285+
$directoryIndex = @{}
185286

186287
$allPackageProperties = Get-AllPkgProperties
187288
$diff = Get-Content $InputDiffJson | ConvertFrom-Json
@@ -194,10 +295,9 @@ function Get-PrPkgProperties([string]$InputDiffJson) {
194295
$targetedFiles += $diff.DeletedFiles
195296
}
196297

197-
$excludePaths = $diff.ExcludePaths
198-
199-
$additionalValidationPackages = @()
200-
$lookup = @{}
298+
$existingTriggeringPaths = Get-TriggerPaths $allPackageProperties
299+
$targetedFiles = Update-TargetedFilesForExclude $targetedFiles $diff.ExcludePaths
300+
$targetedFiles = Update-TargetedFilesForTriggerPaths $targetedFiles $existingTriggeringPaths
201301

202302
# Sort so that we very quickly find any directly changed packages before hitting service level changes.
203303
# This is important because due to the way we traverse the changed files, the instant we evaluate a pkg
@@ -206,9 +306,11 @@ function Get-PrPkgProperties([string]$InputDiffJson) {
206306
# To avoid this without wonky changes to the detection algorithm, we simply sort our files by their depth, so we will always
207307
# detect direct package changes first!
208308
$targetedFiles = $targetedFiles | Sort-Object { ($_.Split("/").Count) } -Descending
309+
$pkgCounter = 1
209310

210311
# this is the primary loop that identifies the packages that have changes
211312
foreach ($pkg in $allPackageProperties) {
313+
Write-Host "Processing changed files against $($pkg.Name). $pkgCounter of $($allPackageProperties.Count)."
212314
$pkgDirectory = Resolve-Path "$($pkg.DirectoryPath)"
213315
$lookupKey = ($pkg.DirectoryPath).Replace($RepoRoot, "").TrimStart('\/')
214316
$lookup[$lookupKey] = $pkg
@@ -224,73 +326,71 @@ function Get-PrPkgProperties([string]$InputDiffJson) {
224326
}
225327

226328
foreach ($file in $targetedFiles) {
227-
$pathComponents = $file -split "/"
228-
$shouldExclude = $false
229-
foreach ($exclude in $excludePaths) {
230-
if ($file.StartsWith($exclude,'CurrentCultureIgnoreCase')) {
231-
$shouldExclude = $true
232-
break
233-
}
234-
}
235-
if ($shouldExclude) {
236-
continue
237-
}
238329
$filePath = (Join-Path $RepoRoot $file)
239330

240331
# handle direct changes to packages
241-
$shouldInclude = $filePath -like (Join-Path "$pkgDirectory" "*")
242-
243-
# handle changes to files that are RELATED to each package
244-
foreach($triggerPath in $triggeringPaths) {
245-
$resolvedRelativePath = (Join-Path $RepoRoot $triggerPath)
246-
if (!$triggerPath.StartsWith("/")){
247-
$resolvedRelativePath = (Join-Path $RepoRoot "sdk" "$($pkg.ServiceDirectory)" $triggerPath)
248-
}
249-
250-
# if we are including this package due to one of its additional trigger paths, we need
251-
# to ensure we're counting it as included for validation, not as an actual package change
252-
if ($resolvedRelativePath) {
253-
$includedForValidation = $filePath -like (Join-Path "$resolvedRelativePath" "*")
332+
$shouldInclude = $filePath -eq $pkgDirectory -or $filePath -like (Join-Path "$pkgDirectory" "*")
333+
334+
# we only need to do additional work for indirect packages if we haven't already decided
335+
# to include this package due to this file
336+
if (-not $shouldInclude) {
337+
# handle changes to files that are RELATED to each package
338+
foreach($triggerPath in $triggeringPaths) {
339+
$resolvedRelativePath = (Join-Path $RepoRoot $triggerPath)
340+
# triggerPaths can be direct files, so we need to check both startswith and direct equality
341+
$includedForValidation = ($filePath -like (Join-Path "$resolvedRelativePath" "*") -or $filePath -eq $resolvedRelativePath)
254342
$shouldInclude = $shouldInclude -or $includedForValidation
255343
if ($includedForValidation) {
256344
$pkg.IncludedForValidation = $true
257345
}
258346
break
259347
}
260-
}
261348

262-
# handle service-level changes to the ci.yml files
263-
# we are using the ci.yml file being added automatically to each artifactdetails as the input
264-
# for this task. This is because we can resolve a service directory from the ci.yml, and if
265-
# there is a single ci.yml in that directory, we can assume that any file change in that directory
266-
# will apply to all packages that exist in that directory.
267-
$triggeringCIYmls = $triggeringPaths | Where-Object { $_ -like "*ci*.yml" }
268-
269-
foreach($yml in $triggeringCIYmls) {
270-
# given that this path is coming from the populated triggering paths in the artifact,
271-
# we can assume that the path to the ci.yml will successfully resolve.
272-
$ciYml = Join-Path $RepoRoot $yml
273-
# ensure we terminate the service directory with a /
274-
$directory = [System.IO.Path]::GetDirectoryName($ciYml).Replace("`\", "/")
275-
$soleCIYml = (Get-ChildItem -Path $directory -Filter "ci*.yml" -File).Count -eq 1
276-
277-
# we should only continue with this check if the file being changed is "in the service directory"
278-
$serviceDirectoryChange = (Split-Path $filePath -Parent).Replace("`\", "/") -eq $directory
279-
if (!$serviceDirectoryChange) {
280-
break
281-
}
349+
# handle service-level changes to the ci.yml files
350+
# we are using the ci.yml file being added automatically to each artifactdetails as the input
351+
# for this task. This is because we can resolve a service directory from the ci.yml, and if
352+
# there is a single ci.yml in that directory, we can assume that any file change in that directory
353+
# will apply to all packages that exist in that directory.
354+
$triggeringCIYmls = $triggeringPaths | Where-Object { $_ -like "*ci*.yml" }
355+
356+
foreach($yml in $triggeringCIYmls) {
357+
# given that this path is coming from the populated triggering paths in the artifact,
358+
# we can assume that the path to the ci.yml will successfully resolve.
359+
$ciYml = Join-Path $RepoRoot $yml
360+
# ensure we terminate the service directory with a /
361+
$directory = [System.IO.Path]::GetDirectoryName($ciYml).Replace("`\", "/")
362+
363+
# we should only continue with this check if the file being changed is "in the service directory"
364+
# files that are directly included in triggerPaths will kept in full form, but otherwise we pre-process the targetedFiles to the
365+
# directory containing the change. Given that pre-process, we should check both direct equality (when not triggeringPath) and parent directory
366+
# for the case where the full form of the file has been left behind (because it was a triggeringPath)
367+
$serviceDirectoryChange = (Split-Path $filePath -Parent).Replace("`\", "/") -eq $directory -or $filePath.Replace("`\", "/") -eq $directory
368+
if (!$serviceDirectoryChange) {
369+
break
370+
}
282371

283-
if ($soleCIYml -and $filePath.Replace("`\", "/").StartsWith($directory)) {
284-
if (-not $shouldInclude) {
285-
$pkg.IncludedForValidation = $true
286-
$shouldInclude = $true
372+
# this GCI is very expensive, so we want to cache the result
373+
$soleCIYml = $true
374+
if ($directoryIndex[$directory]) {
375+
$soleCIYml = $directoryIndex[$directory]
376+
}
377+
else {
378+
$soleCIYml = (Get-ChildItem -Path $directory -Filter "ci*.yml" -File).Count -eq 1
379+
$directoryIndex[$directory] = $soleCIYml
380+
}
381+
382+
if ($soleCIYml -and $filePath.Replace("`\", "/").StartsWith($directory)) {
383+
if (-not $shouldInclude) {
384+
$pkg.IncludedForValidation = $true
385+
$shouldInclude = $true
386+
}
387+
break
388+
}
389+
else {
390+
# if the ci.yml is not the only file in the directory, we cannot assume that any file changed within the directory that isn't the ci.yml
391+
# should trigger this package
392+
Write-Host "Skipping adding package for file `"$file`" because the ci yml `"$yml`" is not the only file in the service directory `"$directory`""
287393
}
288-
break
289-
}
290-
else {
291-
# if the ci.yml is not the only file in the directory, we cannot assume that any file changed within the directory that isn't the ci.yml
292-
# should trigger this package
293-
Write-Host "Skipping adding package for file `"$file`" because the ci yml `"$yml`" is not the only file in the service directory `"$directory`""
294394
}
295395
}
296396

@@ -305,6 +405,8 @@ function Get-PrPkgProperties([string]$InputDiffJson) {
305405
break
306406
}
307407
}
408+
409+
$pkgCounter++
308410
}
309411

310412
# add all of the packages that were added purely for validation purposes

0 commit comments

Comments
 (0)