Skip to content

Commit 5dd762a

Browse files
XiaofeiCaoCopilot
andcommitted
Fix ordering regression: use fixed-point loop for patch detection
The simplified single-pass algorithm assumed patch_release_client.txt was ordered by dependency (dependencies before dependents). In reality it's alphabetical. This caused dependents like azure-messaging-eventhubs- checkpointstore-blob to be missed when they appeared before their dependency (azure-storage-blob) in the iteration order. Replace the single foreach with a do/while fixed-point loop that iterates until no new patches are found, making the algorithm correct regardless of artifact ordering. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 65f7581 commit 5dd762a

3 files changed

Lines changed: 58 additions & 13 deletions

File tree

eng/scripts/Generate-Patch.ps1

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
#
2020
# 7. CreateNewBranch - Whether to create a new branch or use an existing branch. You would want to use an existing branch if you are using the same release tag for multiple libraries.
2121
#
22+
# 8. UseCurrentBranch - When set, creates patch branches from the current branch instead of remote main.
23+
# Useful for local testing and dry-runs without requiring changes to be merged first. This is not a required parameter.
24+
#
2225
# Example: .\eng\scripts\Generate-Patch.ps1 -ArtifactName azure-mixedreality-remoterendering -ServiceDirectory remoterendering -ReleaseVersion 1.0.0 -PatchVersion 1.0.1
2326
# This creates a remote branch "release/azure-mixedreality-remoterendering" with all the necessary changes.
2427

eng/scripts/patchhelpers.ps1

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,8 @@ function UpdateCIInformation($ArtifactInfos) {
102102
}
103103

104104
# Find all the artifacts that will need to be patched based on dependency analysis.
105-
# Artifacts are processed in the same order as defined in patch_release_client.txt
106-
# (dependencies appear before dependents). This guarantees cascading: if a dependency
107-
# is patched, all its dependents will be included as well.
105+
# Iterates until no more patches are found (fixed-point), so the result is correct
106+
# regardless of artifact ordering in patch_release_client.txt.
108107
# Only dependencies that are themselves in the patch list are checked — external
109108
# dependencies (reactor-core, jackson, etc.) are ignored.
110109
function FindArtifactsThatNeedPatching($ArtifactInfos) {
@@ -113,18 +112,23 @@ function FindArtifactsThatNeedPatching($ArtifactInfos) {
113112
$latestVersions[$arId] = $ArtifactInfos[$arId].LatestGAOrPatchVersion
114113
}
115114

116-
foreach ($arId in $ArtifactInfos.Keys) {
117-
$arInfo = $ArtifactInfos[$arId]
118-
foreach ($depId in $arInfo.Dependencies.Keys) {
119-
if (-not $latestVersions.ContainsKey($depId)) { continue }
120-
if ($arInfo.Dependencies[$depId] -ne $latestVersions[$depId]) {
121-
$patchVersion = GetPatchVersion -ReleaseVersion $arInfo.LatestGAOrPatchVersion
122-
$arInfo.FutureReleasePatchVersion = $patchVersion
123-
$latestVersions[$arId] = $patchVersion
124-
break
115+
do {
116+
$changed = $false
117+
foreach ($arId in $ArtifactInfos.Keys) {
118+
$arInfo = $ArtifactInfos[$arId]
119+
if ($arInfo.FutureReleasePatchVersion) { continue }
120+
foreach ($depId in $arInfo.Dependencies.Keys) {
121+
if (-not $latestVersions.ContainsKey($depId)) { continue }
122+
if ($arInfo.Dependencies[$depId] -ne $latestVersions[$depId]) {
123+
$patchVersion = GetPatchVersion -ReleaseVersion $arInfo.LatestGAOrPatchVersion
124+
$arInfo.FutureReleasePatchVersion = $patchVersion
125+
$latestVersions[$arId] = $patchVersion
126+
$changed = $true
127+
break
128+
}
125129
}
126130
}
127-
}
131+
} while ($changed)
128132
}
129133

130134
# Update dependencies in the version client file.

eng/scripts/tests/patchhelpers.tests.ps1

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,44 @@ Describe "FindArtifactsThatNeedPatching" {
177177
# azure-core is flagged once (patch version = 1.41.1, not 1.41.2)
178178
$infos["azure-core"].FutureReleasePatchVersion | Should -Be "1.41.1"
179179
}
180+
181+
It "Unordered hashtable: cascading still works when iteration order is non-deterministic" {
182+
# Production callers (patchreleases.ps1) pass plain @{} not [ordered]@{}.
183+
# With an unordered hashtable the iteration order is undefined, so the
184+
# fixed-point loop must converge regardless.
185+
$infos = @{
186+
"azure-identity" = (New-TestArtifactInfo -ArtifactId "azure-identity" -LatestGAOrPatchVersion "1.10.0" -Dependencies @{
187+
"azure-core" = "1.41.0"
188+
})
189+
"azure-json" = (New-TestArtifactInfo -ArtifactId "azure-json" -LatestGAOrPatchVersion "1.4.0" -Dependencies @{})
190+
"azure-core" = (New-TestArtifactInfo -ArtifactId "azure-core" -LatestGAOrPatchVersion "1.41.0" -Dependencies @{
191+
"azure-json" = "1.3.0"
192+
})
193+
}
194+
FindArtifactsThatNeedPatching -ArtifactInfos $infos
195+
$infos["azure-json"].FutureReleasePatchVersion | Should -BeNullOrEmpty
196+
$infos["azure-core"].FutureReleasePatchVersion | Should -Be "1.41.1"
197+
$infos["azure-identity"].FutureReleasePatchVersion | Should -Be "1.10.1"
198+
}
199+
200+
It "Handles out-of-order: dependent listed before its dependency (checkpointstore scenario)" {
201+
# checkpointstore appears BEFORE blob in the iteration order (alphabetical in patch_release_client.txt).
202+
# blob gets patched because its own dep (storage-common) was independently released.
203+
# checkpointstore must still be patched even though it's iterated first.
204+
$infos = [ordered]@{
205+
"azure-storage-common" = (New-TestArtifactInfo -ArtifactId "azure-storage-common" -LatestGAOrPatchVersion "12.32.2" -Dependencies @{})
206+
"azure-messaging-eventhubs-checkpointstore-blob" = (New-TestArtifactInfo -ArtifactId "azure-messaging-eventhubs-checkpointstore-blob" -LatestGAOrPatchVersion "1.21.4" -Dependencies @{
207+
"azure-storage-blob" = "12.33.2"
208+
})
209+
"azure-storage-blob" = (New-TestArtifactInfo -ArtifactId "azure-storage-blob" -LatestGAOrPatchVersion "12.33.2" -Dependencies @{
210+
"azure-storage-common" = "12.32.1"
211+
})
212+
}
213+
FindArtifactsThatNeedPatching -ArtifactInfos $infos
214+
$infos["azure-storage-common"].FutureReleasePatchVersion | Should -BeNullOrEmpty
215+
$infos["azure-storage-blob"].FutureReleasePatchVersion | Should -Be "12.33.3"
216+
$infos["azure-messaging-eventhubs-checkpointstore-blob"].FutureReleasePatchVersion | Should -Be "1.21.5"
217+
}
180218
}
181219

182220
# ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)