Skip to content

Commit eb07a83

Browse files
janniklasroseIsaac
janniklasrose
and
Isaac
committed
nextchanges: Attribute stacked-PR fragments to the PR that added them (#6546)
## Changes Fix `tools/validate_nextchanges.py` to attribute each `.nextchanges/` fragment to the PR that actually introduced it, rather than always the current PR. The changelog-preview check requires every fragment's trailing PR link to name the current PR. It infers the expected PR from the squash-merge commit subject (`(#N)`), falling back to the current PR for fragments not yet on main. That fallback is wrong for a **gh-stacked PR**: the fragments inherited from earlier PRs in the stack are added by ordinary branch commits (no `(#N)` subject), so they were attributed to the top PR — and CI failed even though each fragment already carried the correct link to the earlier PR. Concretely, [#6544](#6544) (stacked on the SDK bump [#6448](#6448)) failed with: ``` .nextchanges/dependency-updates/bump-sdk-0.177.0.md: trailing PR link #6448 must include the PR that added this fragment (#6544) ``` ## Fix A fragment that already exists on the PR's **base branch** was introduced by an earlier PR (upstream, or lower in the stack), so it is no longer attributed to the current PR — it keeps its own link and is only required to be well-formed. Fragments the PR genuinely adds still must name the current PR, as before. - `infer_expected_pr` returns `None` (no specific PR imposed) when the fragment is present on the base branch (`fragment_on_base`), instead of falling back to the current PR. - The base branch is `BASE_REF` (`github.event.pull_request.base.ref`) in CI, or a best-effort `gh` lookup locally (`detect_base_ref` / `current_branch_base`, mirroring the existing PR detection). - The changelog-preview workflow now passes `BASE_REF` to the validate step. Behavior is unchanged for non-stacked PRs, pushes to main (fragments keep their `(#N)` attribution), and the merge queue's shallow checkout (PR-link checks are already skipped there). ## Tests - Existing doctests pass (`python -m doctest tools/validate_nextchanges.py`). - Reproduced a two-branch stack in a scratch repo: the old code fails on the inherited fragment with the same error as #6544; with `BASE_REF` set to the base branch it passes. The genuinely-new fragment still requires the top PR's link. This pull request and its description were written by Isaac. --------- Co-authored-by: Isaac <no-reply@databricks.com>
1 parent 8fb31b8 commit eb07a83

2 files changed

Lines changed: 85 additions & 9 deletions

File tree

‎.github/workflows/changelog-preview.yml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,13 @@ jobs:
4747
# names the right PR. PR_NUMBER is the authoritative event PR (empty on push
4848
# to main, where each fragment's PR is inferred from its squash-merge
4949
# commit); it must have full history to attribute fragments, hence the
50-
# fetch-depth: 0 checkout above.
50+
# fetch-depth: 0 checkout above. BASE_REF is the PR's base branch, so a
51+
# gh-stacked PR's inherited fragments (which already carry the earlier PR's
52+
# link) aren't attributed to this PR.
5153
- name: Validate .nextchanges placement
5254
env:
5355
PR_NUMBER: ${{ github.event.number }}
56+
BASE_REF: ${{ github.event.pull_request.base.ref }}
5457
run: uv run tools/validate_nextchanges.py
5558

5659
- name: Render changelog preview

‎tools/validate_nextchanges.py‎

Lines changed: 81 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -165,15 +165,18 @@ def pr_link_problem(text, require_pr_link, expected_pr):
165165
return None
166166

167167

168-
def infer_expected_pr(path, fallback_pr, root):
168+
def infer_expected_pr(path, fallback_pr, base_ref, root):
169169
"""Return the PR number that introduced the fragment at ``path``.
170170
171171
databricks/cli squash-merges end the commit subject with ``(#N)``, so the
172172
commit that most recently added the file names its PR. A fragment not yet on
173173
main (added on the current branch, or uncommitted) has no such commit, so
174-
``fallback_pr`` — the current PR — is used. ``git`` runs in ``root`` so the
175-
repo being validated is queried even when ``--root`` differs from the process
176-
CWD. Requires full git history (the workflow checks out with
174+
``fallback_pr`` — the current PR — is used, unless the fragment already exists
175+
on the PR's base branch: a gh-stacked PR carries the fragments of the PRs it is
176+
stacked on, which keep their own link, so ``None`` is returned for those and no
177+
specific PR is imposed (see ``fragment_on_base``). ``git`` runs in ``root`` so
178+
the repo being validated is queried even when ``--root`` differs from the
179+
process CWD. Requires full git history (the workflow checks out with
177180
``fetch-depth: 0``); best-effort, so any git failure falls back rather than
178181
erroring."""
179182
try:
@@ -190,9 +193,38 @@ def infer_expected_pr(path, fallback_pr, root):
190193
m = re.search(r"\(#(\d+)\)\s*$", result.stdout.strip())
191194
if m:
192195
return m.group(1)
196+
if base_ref and fragment_on_base(path, base_ref, root):
197+
return None
193198
return fallback_pr
194199

195200

201+
def fragment_on_base(path, base_ref, root):
202+
"""Whether the fragment at ``path`` already exists on the PR's base branch.
203+
204+
A stacked PR inherits the fragments of the PRs below it; those already live on
205+
the base branch and carry their own PR link, so they are not attributed to the
206+
current PR. The base is looked up as the remote-tracking ``origin/<base_ref>``
207+
(CI's checkout), falling back to a local branch ``<base_ref>``. Best-effort: any
208+
git failure returns ``False``, attributing the fragment to the current PR as
209+
before."""
210+
rel = path.relative_to(root).as_posix()
211+
for ref in (f"origin/{base_ref}", base_ref):
212+
try:
213+
result = subprocess.run(
214+
["git", "cat-file", "-e", f"{ref}:{rel}"],
215+
capture_output=True,
216+
text=True,
217+
timeout=10,
218+
cwd=root,
219+
)
220+
except (OSError, subprocess.SubprocessError) as e:
221+
print(f"git cat-file failed: {e}", file=sys.stderr)
222+
return False
223+
if result.returncode == 0:
224+
return True
225+
return False
226+
227+
196228
def is_shallow(root):
197229
"""Whether ``root`` is a shallow git clone. Best-effort; a git failure (or a
198230
non-repo) returns False. On a shallow clone every file looks added at the
@@ -225,13 +257,15 @@ def load_sections(root):
225257
return tuple(sections)
226258

227259

228-
def find_problems(changelog_dir, sections, require_pr_link=False, fallback_pr=None, root=None):
260+
def find_problems(changelog_dir, sections, require_pr_link=False, fallback_pr=None, base_ref=None, root=None):
229261
"""Return a list of ``(path, message)`` for anything unexpected under
230262
``.nextchanges/``: files that aren't a section fragment or known scaffolding,
231263
malformed fragments, a trailing PR link that is missing or names the wrong
232264
PR, and a missing/malformed version file. ``require_pr_link`` and
233265
``fallback_pr`` drive the PR-link checks (set in CI / from the branch's PR,
234-
see ``main``); ``root`` is the repo the PR inference queries via git."""
266+
see ``main``); ``base_ref`` is the PR's base branch, so fragments inherited
267+
from an earlier PR in a stack aren't attributed to the current PR (see
268+
``infer_expected_pr``). ``root`` is the repo the PR inference queries via git."""
235269
problems = []
236270
known_sections = set(sections)
237271
# A shallow clone (e.g. the merge queue's checkout) makes every fragment look
@@ -268,7 +302,7 @@ def find_problems(changelog_dir, sections, require_pr_link=False, fallback_pr=No
268302
if problem is None:
269303
# Infer the expected PR (a git call) only for a structurally
270304
# valid fragment, and only with full history — see `shallow`.
271-
expected_pr = None if shallow else infer_expected_pr(path, fallback_pr, root)
305+
expected_pr = None if shallow else infer_expected_pr(path, fallback_pr, base_ref, root)
272306
problem = pr_link_problem(text, require_pr_link, expected_pr)
273307
if problem:
274308
problems.append((path, problem))
@@ -332,6 +366,43 @@ def detect_current_pr(root):
332366
return current_branch_pr(root)
333367

334368

369+
def current_branch_base(root):
370+
"""Best-effort base branch of the current branch's PR (via ``gh``), or ``None``.
371+
372+
A local convenience mirroring ``current_branch_pr``; CI uses the authoritative
373+
event base instead (see ``detect_base_ref``). Any ``gh`` failure returns
374+
``None`` so local runs never hard-fail on tooling."""
375+
try:
376+
result = subprocess.run(
377+
["gh", "pr", "view", "--json", "baseRefName", "-q", ".baseRefName"],
378+
capture_output=True,
379+
text=True,
380+
timeout=10,
381+
cwd=root,
382+
)
383+
except (OSError, subprocess.SubprocessError) as e:
384+
print(f"gh pr view failed: {e}", file=sys.stderr)
385+
return None
386+
out = result.stdout.strip()
387+
return out if result.returncode == 0 and out else None
388+
389+
390+
def detect_base_ref(root):
391+
"""Base branch of the PR being validated, or ``None``.
392+
393+
In CI, the authoritative event base (``BASE_REF``, set from
394+
``github.event.pull_request.base.ref``); empty on a push to main. Locally, a
395+
best-effort ``gh`` lookup of the branch's PR. Used to spare a stacked PR's
396+
inherited fragments from being attributed to the current PR (see
397+
``infer_expected_pr``)."""
398+
base = os.environ.get("BASE_REF", "").strip()
399+
if base:
400+
return base
401+
if in_ci():
402+
return None
403+
return current_branch_base(root)
404+
405+
335406
def has_fragments(changelog_dir):
336407
"""Whether any *.md fragment (excluding README.md) exists under a section."""
337408
return any(p.name != README for p in changelog_dir.glob("*/*.md"))
@@ -403,11 +474,13 @@ def main(argv=None):
403474
# only when there are fragments, to avoid a `gh` call on unrelated runs.
404475
require_pr_link = False
405476
fallback_pr = None
477+
base_ref = None
406478
if has_fragments(changelog_dir):
407479
fallback_pr = detect_current_pr(args.root)
480+
base_ref = detect_base_ref(args.root)
408481
require_pr_link = in_ci() or fallback_pr is not None
409482

410-
problems = find_problems(changelog_dir, sections, require_pr_link, fallback_pr, args.root)
483+
problems = find_problems(changelog_dir, sections, require_pr_link, fallback_pr, base_ref, args.root)
411484
if problems:
412485
for path, msg in problems:
413486
print(f"{path}: {msg}", file=sys.stderr)

0 commit comments

Comments
 (0)