Skip to content

Commit b1ccf9c

Browse files
azure-sdkscbedd
andauthored
Sync eng/common directory with azure-sdk-tools for PR 10411 (#2520)
* when processing a PR diff, treat packages added via their triggeringPaths as directly changed (used to be indirect) --------- Co-authored-by: Scott Beddall <[email protected]>
1 parent fd8da1a commit b1ccf9c

File tree

1 file changed

+23
-23
lines changed

1 file changed

+23
-23
lines changed

eng/common/scripts/Package-Properties.ps1

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,11 @@ function Update-TargetedFilesForTriggerPaths([string[]]$TargetedFiles, [string[]
288288

289289
for ($i = 0; $i -lt $Triggers.Count; $i++) {
290290
$triggerPath = $Triggers[$i]
291-
if ($triggerPath -and $file -eq "$triggerPath") {
291+
# targeted files comes from the `changedPaths` property of the diff, which is
292+
# a list of relative file paths from root. Not starting with a /.
293+
# However, the triggerPaths are absolute paths, so we need to resolve the targeted file
294+
# to the same format
295+
if ($triggerPath -and "/$file" -eq "$triggerPath") {
292296
$isExistingTriggerPath = $true
293297
break
294298
}
@@ -365,8 +369,8 @@ function Get-PrPkgProperties([string]$InputDiffJson) {
365369
# this is the primary loop that identifies the packages that have changes
366370
foreach ($pkg in $allPackageProperties) {
367371
Write-Host "Processing changed files against $($pkg.Name). $pkgCounter of $($allPackageProperties.Count)."
368-
$pkgDirectory = Resolve-Path "$($pkg.DirectoryPath)"
369-
$lookupKey = ($pkg.DirectoryPath).Replace($RepoRoot, "").TrimStart('\/')
372+
$pkgDirectory = (Resolve-Path "$($pkg.DirectoryPath)").Path.Replace("`\", "/")
373+
$lookupKey = $pkgDirectory.Replace($RepoRoot, "").TrimStart('\/')
370374
$lookup[$lookupKey] = $pkg
371375

372376
# we only honor the individual artifact triggers
@@ -380,22 +384,21 @@ function Get-PrPkgProperties([string]$InputDiffJson) {
380384
}
381385

382386
foreach ($file in $targetedFiles) {
383-
$filePath = (Join-Path $RepoRoot $file)
387+
$filePath = (Join-Path $RepoRoot $file).Replace("`\", "/")
384388

385389
# handle direct changes to packages
386-
$shouldInclude = $filePath -eq $pkgDirectory -or $filePath -like (Join-Path "$pkgDirectory" "*")
390+
$shouldInclude = $filePath -eq $pkgDirectory -or $filePath -like "$pkgDirectory/*"
387391

388392
# we only need to do additional work for indirect packages if we haven't already decided
389393
# to include this package due to this file
390394
if (-not $shouldInclude) {
391395
# handle changes to files that are RELATED to each package
392396
foreach($triggerPath in $triggeringPaths) {
393-
$resolvedRelativePath = (Join-Path $RepoRoot $triggerPath)
397+
$resolvedRelativePath = (Join-Path $RepoRoot $triggerPath).Replace("`\", "/")
394398
# triggerPaths can be direct files, so we need to check both startswith and direct equality
395-
$includedForValidation = ($filePath -like (Join-Path "$resolvedRelativePath" "*") -or $filePath -eq $resolvedRelativePath)
399+
$includedForValidation = ($filePath -like ("$resolvedRelativePath/*") -or $filePath -eq $resolvedRelativePath)
396400
$shouldInclude = $shouldInclude -or $includedForValidation
397401
if ($includedForValidation) {
398-
$pkg.IncludedForValidation = $true
399402
break
400403
}
401404
}
@@ -406,20 +409,22 @@ function Get-PrPkgProperties([string]$InputDiffJson) {
406409
# there is a single ci.yml in that directory, we can assume that any file change in that directory
407410
# will apply to all packages that exist in that directory.
408411
$triggeringCIYmls = $triggeringPaths | Where-Object { $_ -like "*ci*.yml" }
409-
410412
foreach($yml in $triggeringCIYmls) {
411413
# given that this path is coming from the populated triggering paths in the artifact,
412414
# we can assume that the path to the ci.yml will successfully resolve.
413415
$ciYml = Join-Path $RepoRoot $yml
414416
# ensure we terminate the service directory with a /
415417
$directory = [System.IO.Path]::GetDirectoryName($ciYml).Replace("`\", "/")
416418

417-
# we should only continue with this check if the file being changed is "in the service directory"
418-
# files that are directly included in triggerPaths will kept in full form, but otherwise we pre-process the targetedFiles to the
419-
# directory containing the change. Given that pre-process, we should check both direct equality (when not triggeringPath) and parent directory
420-
# for the case where the full form of the file has been left behind (because it was a triggeringPath)
421-
$serviceDirectoryChange = (Split-Path $filePath -Parent).Replace("`\", "/") -eq $directory -or $filePath.Replace("`\", "/") -eq $directory
422-
if (!$serviceDirectoryChange) {
419+
# this filepath doesn't apply to this service directory at all, so we can break out of this loop
420+
if (-not $filePath.StartsWith("$directory/")) {
421+
break
422+
}
423+
424+
$relative = $filePath.SubString($directory.Length + 1)
425+
426+
if ($relative.Contains("/") -or -not [IO.Path]::GetExtension($relative)){
427+
# this is a bare folder OR exists deeper than the service directory, so we can skip
423428
break
424429
}
425430

@@ -433,18 +438,12 @@ function Get-PrPkgProperties([string]$InputDiffJson) {
433438
$directoryIndex[$directory] = $soleCIYml
434439
}
435440

436-
if ($soleCIYml -and $filePath.Replace("`\", "/").StartsWith($directory)) {
441+
if ($soleCIYml -and $filePath.StartsWith($directory)) {
437442
if (-not $shouldInclude) {
438-
$pkg.IncludedForValidation = $true
439443
$shouldInclude = $true
440444
}
441445
break
442446
}
443-
else {
444-
# 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
445-
# should trigger this package
446-
Write-Host "Skipping adding package for file `"$file`" because the ci yml `"$yml`" is not the only file in the service directory `"$directory`""
447-
}
448447
}
449448
}
450449

@@ -483,7 +482,8 @@ function Get-PrPkgProperties([string]$InputDiffJson) {
483482
# now pass along the set of packages we've identified, the diff itself, and the full set of package properties
484483
# to locate any additional packages that should be included for validation
485484
if ($AdditionalValidationPackagesFromPackageSetFn -and (Test-Path "Function:$AdditionalValidationPackagesFromPackageSetFn")) {
486-
$packagesWithChanges += &$AdditionalValidationPackagesFromPackageSetFn $packagesWithChanges $diff $allPackageProperties
485+
$additionalPackages = &$AdditionalValidationPackagesFromPackageSetFn $packagesWithChanges $diff $allPackageProperties
486+
$packagesWithChanges += $additionalPackages
487487
}
488488

489489
# finally, if we have gotten all the way here and we still don't have any packages, we should include the template service

0 commit comments

Comments
 (0)