Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/changelog-preview.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,13 @@ jobs:
# names the right PR. PR_NUMBER is the authoritative event PR (empty on push
# to main, where each fragment's PR is inferred from its squash-merge
# commit); it must have full history to attribute fragments, hence the
# fetch-depth: 0 checkout above.
# fetch-depth: 0 checkout above. BASE_REF is the PR's base branch, so a
# gh-stacked PR's inherited fragments (which already carry the earlier PR's
# link) aren't attributed to this PR.
- name: Validate .nextchanges placement
env:
PR_NUMBER: ${{ github.event.number }}
BASE_REF: ${{ github.event.pull_request.base.ref }}
run: uv run tools/validate_nextchanges.py

- name: Render changelog preview
Expand Down
89 changes: 81 additions & 8 deletions tools/validate_nextchanges.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,15 +165,18 @@ def pr_link_problem(text, require_pr_link, expected_pr):
return None


def infer_expected_pr(path, fallback_pr, root):
def infer_expected_pr(path, fallback_pr, base_ref, root):
"""Return the PR number that introduced the fragment at ``path``.

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


def fragment_on_base(path, base_ref, root):
"""Whether the fragment at ``path`` already exists on the PR's base branch.

A stacked PR inherits the fragments of the PRs below it; those already live on
the base branch and carry their own PR link, so they are not attributed to the
current PR. The base is looked up as the remote-tracking ``origin/<base_ref>``
(CI's checkout), falling back to a local branch ``<base_ref>``. Best-effort: any
git failure returns ``False``, attributing the fragment to the current PR as
before."""
rel = path.relative_to(root).as_posix()
for ref in (f"origin/{base_ref}", base_ref):
try:
result = subprocess.run(
["git", "cat-file", "-e", f"{ref}:{rel}"],
capture_output=True,
text=True,
timeout=10,
cwd=root,
)
except (OSError, subprocess.SubprocessError) as e:
print(f"git cat-file failed: {e}", file=sys.stderr)
return False
Comment thread
janniklasrose marked this conversation as resolved.
if result.returncode == 0:
return True
return False


def is_shallow(root):
"""Whether ``root`` is a shallow git clone. Best-effort; a git failure (or a
non-repo) returns False. On a shallow clone every file looks added at the
Expand Down Expand Up @@ -225,13 +257,15 @@ def load_sections(root):
return tuple(sections)


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


def current_branch_base(root):
"""Best-effort base branch of the current branch's PR (via ``gh``), or ``None``.

A local convenience mirroring ``current_branch_pr``; CI uses the authoritative
event base instead (see ``detect_base_ref``). Any ``gh`` failure returns
``None`` so local runs never hard-fail on tooling."""
try:
result = subprocess.run(
["gh", "pr", "view", "--json", "baseRefName", "-q", ".baseRefName"],
capture_output=True,
text=True,
timeout=10,
cwd=root,
)
except (OSError, subprocess.SubprocessError) as e:
print(f"gh pr view failed: {e}", file=sys.stderr)
return None
out = result.stdout.strip()
return out if result.returncode == 0 and out else None


def detect_base_ref(root):
"""Base branch of the PR being validated, or ``None``.

In CI, the authoritative event base (``BASE_REF``, set from
``github.event.pull_request.base.ref``); empty on a push to main. Locally, a
best-effort ``gh`` lookup of the branch's PR. Used to spare a stacked PR's
inherited fragments from being attributed to the current PR (see
``infer_expected_pr``)."""
base = os.environ.get("BASE_REF", "").strip()
if base:
return base
if in_ci():
return None
return current_branch_base(root)


def has_fragments(changelog_dir):
"""Whether any *.md fragment (excluding README.md) exists under a section."""
return any(p.name != README for p in changelog_dir.glob("*/*.md"))
Expand Down Expand Up @@ -403,11 +474,13 @@ def main(argv=None):
# only when there are fragments, to avoid a `gh` call on unrelated runs.
require_pr_link = False
fallback_pr = None
base_ref = None
if has_fragments(changelog_dir):
fallback_pr = detect_current_pr(args.root)
base_ref = detect_base_ref(args.root)
require_pr_link = in_ci() or fallback_pr is not None

problems = find_problems(changelog_dir, sections, require_pr_link, fallback_pr, args.root)
problems = find_problems(changelog_dir, sections, require_pr_link, fallback_pr, base_ref, args.root)
if problems:
for path, msg in problems:
print(f"{path}: {msg}", file=sys.stderr)
Expand Down
Loading