Skip to content

Commit 2e92036

Browse files
committed
[SPARK-59005][INFRA] Offer to update the JIRA Affects Version from the fix version in the merge script
### What changes were proposed in this pull request? `resolve_jira_issue` in `dev/merge_spark_pr.py` now offers to update the JIRA Affects Version/s, mirroring the existing Fix Version prompt (previously they were display-only). It only prompts when the earliest fix version precedes the earliest affected version, so consistent tickets are untouched. This covers a too-high affected version on a fresh resolve (fixed `4.4.0`, affects only `5.0.0`) and a backport that adds an earlier fix line (affects `4.4.0`, backport adds fix `4.3.1`, so `4.3` is affected too). The suggested default is derived from the fix version(s), validated against all unarchived versions, and the write goes through `jira_ops` so `--dry-run` only logs. Fix Version, assignee, and component logic are unchanged. New pure helpers `parse_affects_versions_input`, `fix_precedes_affects`, and `suggest_affects_versions` carry doctests. ### Why are the changes needed? The script already reconciles assignee, components, and Fix Version/s, but Affects Version/s were read-only, so a version inconsistent with where the fix lands (too high, or missing a backported line) could not be corrected in-flow. ### Does this PR introduce _any_ user-facing change? No. Committer-facing merge tooling only. ### How was this patch tested? Doctests for the three helpers (`python -m doctest dev/merge_spark_pr.py`, all pass) plus a local dry-run of the fresh-resolve and backport flows with a stubbed issue and `DryRunJira`. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #58288 from Yicong-Huang/affects-version-prompt. Authored-by: Yicong Huang <17627829+Yicong-Huang@users.noreply.github.com> Signed-off-by: Yicong-Huang <17627829+Yicong-Huang@users.noreply.github.com>
1 parent 93d4016 commit 2e92036

1 file changed

Lines changed: 156 additions & 10 deletions

File tree

dev/merge_spark_pr.py

Lines changed: 156 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
import subprocess
4545
import sys
4646
import traceback
47-
from typing import List
47+
from typing import List, Optional
4848
from urllib.error import HTTPError
4949
from urllib.request import Request, urlopen
5050

@@ -137,6 +137,25 @@ def semver_branch_rank(name):
137137
return (-1, -1)
138138

139139

140+
def parse_version(name: str) -> Optional[tuple[int, int, int]]:
141+
"""Parse a dotted ``x.y.z`` version name into an ``(int, int, int)`` tuple.
142+
143+
Returns None for anything that is not a plain three-part numeric version, so the
144+
result can be compared directly and non-version names are skipped by callers.
145+
146+
>>> parse_version("4.4.0")
147+
(4, 4, 0)
148+
>>> parse_version("4.10.2")
149+
(4, 10, 2)
150+
>>> parse_version("branch-4.x") is None
151+
True
152+
>>> parse_version("4.4") is None
153+
True
154+
"""
155+
m = re.match(r"^(\d+)\.(\d+)\.(\d+)$", name)
156+
return tuple(int(g) for g in m.groups()) if m else None
157+
158+
140159
def _semver_max_version(names):
141160
"""
142161
Highest dotted version by numeric semver (SPARK Fix Version naming).
@@ -146,9 +165,10 @@ def _semver_max_version(names):
146165
>>> _semver_max_version(["5.0.0", "6.0.0"])
147166
'6.0.0'
148167
"""
149-
if not names:
168+
parsed = [(parse_version(n), n) for n in names]
169+
parsed = [(t, n) for t, n in parsed if t is not None]
170+
if not parsed:
150171
return None
151-
parsed = [(tuple(int(p) for p in n.split(".")), n) for n in names]
152172
return max(parsed)[1]
153173

154174

@@ -368,6 +388,24 @@ def fix_version_additions(inferred_versions, existing_versions):
368388
return additions, bool(inferred_versions) and not additions
369389

370390

391+
def parse_version_list(raw: str) -> list[str]:
392+
"""Split a comma-separated version string into trimmed, de-duplicated names.
393+
394+
Surrounding whitespace on each entry is stripped and empty entries are dropped, so a
395+
blank or all-separator string yields no names. Input order is preserved. Shared by the
396+
Fix Version and Affects Version prompts so both parse committer input the same way.
397+
398+
>>> parse_version_list("5.0.0, 4.3.0")
399+
['5.0.0', '4.3.0']
400+
>>> parse_version_list(" 4.4.0 , , 4.4.0 ")
401+
['4.4.0']
402+
>>> parse_version_list(" ")
403+
[]
404+
"""
405+
names = [segment.strip() for segment in raw.split(",")]
406+
return list(dict.fromkeys(n for n in names if n))
407+
408+
371409
def fix_versions_from_input(raw_input, default_fix_versions):
372410
"""Resolve the Fix Version prompt's raw input into a list of version names.
373411
@@ -387,10 +425,35 @@ def fix_versions_from_input(raw_input, default_fix_versions):
387425
"""
388426
if raw_input == "":
389427
raw_input = default_fix_versions
390-
stripped = raw_input.replace(" ", "")
391-
if stripped == "":
392-
return []
393-
return stripped.split(",")
428+
return parse_version_list(raw_input)
429+
430+
431+
def fix_precedes_affects(fix_version_names: list[str], affects_version_names: list[str]) -> bool:
432+
"""Whether the earliest fix version is below the earliest recorded Affects Version.
433+
434+
True means the affected floor sits above a fixed release, so the ticket omits a
435+
version the fix reaches -- either too-high on a fresh resolve (fixed 4.4.0, affects
436+
only 5.0.0) or a backport to an earlier line (affects 4.4.0, backport adds fix 4.3.x).
437+
Non-``x.y.z`` names and empty lists compare as absent, so they never trigger a prompt.
438+
439+
>>> fix_precedes_affects(["4.4.0"], ["5.0.0"])
440+
True
441+
>>> fix_precedes_affects(["4.4.0"], ["4.3.0"])
442+
False
443+
>>> fix_precedes_affects(["4.4.0"], ["4.4.0"])
444+
False
445+
>>> fix_precedes_affects(["4.4.0", "4.3.1"], ["4.4.0"])
446+
True
447+
>>> fix_precedes_affects(["4.4.0"], ["4.10.0"])
448+
True
449+
>>> fix_precedes_affects([], ["5.0.0"])
450+
False
451+
"""
452+
fix = [t for t in map(parse_version, fix_version_names) if t is not None]
453+
affects = [t for t in map(parse_version, affects_version_names) if t is not None]
454+
if not fix or not affects:
455+
return False
456+
return min(fix) < min(affects)
394457

395458

396459
def red(text):
@@ -1247,6 +1310,62 @@ def reconcile_jira_components(issue, title_components):
12471310
jira_ops.update_components(issue, new_names)
12481311

12491312

1313+
def reconcile_jira_affects_versions(
1314+
issue, fix_version_names: list[str], affects_available: set[str]
1315+
) -> None:
1316+
"""Prompt the committer to correct the Affects Version/s when none precedes the fix.
1317+
1318+
The caller gates on ``fix_precedes_affects``, so this runs only when the affected
1319+
version is likely wrong. The merge target cannot reveal when the bug was introduced,
1320+
so the committer types the version(s) explicitly (validated against
1321+
``affects_available``); when the issue already has versions, they choose to append
1322+
(default), overwrite, or keep. A blank input or [k]eep leaves the field untouched.
1323+
Writes go through ``jira_ops`` so a dry run only logs them.
1324+
"""
1325+
current_names = [v.name for v in issue.fields.versions]
1326+
print()
1327+
print("=" * 80)
1328+
print(
1329+
f"JIRA {issue.key} Affects Version/s {current_names if current_names else '(none)'} "
1330+
f"vs Fix Version/s {fix_version_names}: at least one Affects Version must precede "
1331+
f"the earliest Fix Version, so the recorded affected version is likely wrong."
1332+
)
1333+
print("=" * 80)
1334+
while True:
1335+
try:
1336+
raw = bold_input("Enter comma-separated affects version(s) (blank to skip): ")
1337+
if raw.strip() == "":
1338+
print(f"Affects Version/s left unchanged; update {issue.key} manually.")
1339+
return
1340+
new_names = parse_version_list(raw)
1341+
if new_names and set(new_names).issubset(affects_available):
1342+
break
1343+
print(
1344+
f"Specified version(s) [{', '.join(new_names)}] not found in the available "
1345+
f"versions, try again (or leave blank to skip)."
1346+
)
1347+
except KeyboardInterrupt:
1348+
raise
1349+
except BaseException:
1350+
traceback.print_exc()
1351+
print("Error setting affects version(s), try again (or leave blank to skip).")
1352+
1353+
if current_names:
1354+
choice = get_input(
1355+
f"[a]ppend to / [o]verwrite existing {current_names} / [k]eep as is "
1356+
"(default: append): ",
1357+
{"a": ["a", "append", ""], "o": ["o", "overwrite"], "k": ["k", "keep"]},
1358+
)
1359+
if choice == "k":
1360+
print(f"Keeping JIRA {issue.key} Affects Version/s unchanged.")
1361+
return
1362+
if choice == "a":
1363+
# Keep the existing versions first, then add the entered ones.
1364+
new_names = list(dict.fromkeys(current_names + new_names))
1365+
1366+
jira_ops.update_affects_versions(issue, new_names)
1367+
1368+
12501369
def get_jira_issue(prompt, default_jira_id=""):
12511370
jira_id = bold_input("%s [%s]: " % (prompt, default_jira_id))
12521371
if jira_id == "":
@@ -1299,14 +1418,19 @@ def resolve_jira_issue(
12991418

13001419
reconcile_jira_components(issue, title_components)
13011420

1302-
versions = asf_jira.project_versions("SPARK")
1421+
all_versions = asf_jira.project_versions("SPARK")
13031422
# Consider only x.y.z, unreleased, unarchived versions
13041423
versions = [
13051424
x
1306-
for x in versions
1307-
if not x.raw["released"] and not x.raw["archived"] and re.match(r"\d+\.\d+\.\d+", x.name)
1425+
for x in all_versions
1426+
if not x.raw["released"] and not x.raw["archived"] and parse_version(x.name) is not None
13081427
]
13091428
versions = sorted(versions, key=lambda x: x.name, reverse=True)
1429+
# Affects Version/s may name an already-released version, so validate the affects prompt
1430+
# against all unarchived x.y.z versions, not just the unreleased fix candidates.
1431+
affects_available = {
1432+
x.name for x in all_versions if not x.raw["archived"] and parse_version(x.name) is not None
1433+
}
13101434

13111435
unreleased_names = [v.name for v in versions]
13121436
default_fix_list, infer_warnings = compute_merge_default_fix_versions(
@@ -1369,6 +1493,13 @@ def resolve_jira_issue(
13691493
traceback.print_exc()
13701494
print("Error setting fix version(s), try again (or leave blank and fix manually)")
13711495

1496+
# On a fresh resolve, offer to update the Affects Version/s when they sit above the fix
1497+
# version(s) just chosen; the backport path above handles the already-resolved case.
1498+
if not is_resolved and fix_precedes_affects(
1499+
fix_versions, [v.name for v in issue.fields.versions]
1500+
):
1501+
reconcile_jira_affects_versions(issue, fix_versions, affects_available)
1502+
13721503
def get_version_json(version_str):
13731504
return list(filter(lambda v: v.name == version_str, versions))[0].raw
13741505

@@ -1380,6 +1511,11 @@ def get_version_json(version_str):
13801511
if not jira_fix_versions:
13811512
print("No new fix versions selected for JIRA issue %s; no update needed." % issue.key)
13821513
return
1514+
# A backport adds an earlier fix line, which usually means that line is affected too;
1515+
# offer to extend the Affects Version/s down when they miss the full fix set.
1516+
full_fix_names = existing_fix_version_names + [v["name"] for v in jira_fix_versions]
1517+
if fix_precedes_affects(full_fix_names, [v.name for v in issue.fields.versions]):
1518+
reconcile_jira_affects_versions(issue, full_fix_names, affects_available)
13831519
jira_ops.add_fix_versions(issue, existing_fix_versions, jira_fix_versions)
13841520
return
13851521

@@ -1469,6 +1605,13 @@ def update_components(self, issue, new_names):
14691605
except Exception as e:
14701606
print_error("Failed to update components on JIRA %s: %s" % (issue.key, e))
14711607

1608+
def update_affects_versions(self, issue, new_names: list[str]) -> None:
1609+
try:
1610+
issue.update(fields={"versions": [{"name": n} for n in new_names]})
1611+
print(f"Updated JIRA {issue.key} Affects Version/s to: {', '.join(new_names)}")
1612+
except Exception as e:
1613+
print_error(f"Failed to update Affects Version/s on JIRA {issue.key}: {e}")
1614+
14721615
def add_fix_versions(self, issue, existing_fix_versions, new_version_jsons):
14731616
issue.update(
14741617
fields={"fixVersions": [v.raw for v in existing_fix_versions] + new_version_jsons}
@@ -1518,6 +1661,9 @@ class DryRunJira(Jira):
15181661
def update_components(self, issue, new_names):
15191662
print("DRY-RUN: would set JIRA %s components to: %s" % (issue.key, ", ".join(new_names)))
15201663

1664+
def update_affects_versions(self, issue, new_names: list[str]) -> None:
1665+
print(f"DRY-RUN: would set JIRA {issue.key} Affects Version/s to: {', '.join(new_names)}")
1666+
15211667
def add_fix_versions(self, issue, existing_fix_versions, new_version_jsons):
15221668
print(
15231669
"DRY-RUN: would add fixVersions=%s to JIRA %s."

0 commit comments

Comments
 (0)