Glob-match required status contexts when diagnosing a merge #275
Loading…
Reference in a new issue
No description provided.
Delete branch "fix-required-check-glob"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
fj pr mergeblames a required check on fully green PRs whenever the base branch is protected withstatus_check_contexts: ["*"], which is every rasterstate repo:Forgejo compiles each entry of
status_check_contextsas a glob, so*means every reported context andCI / *every context with that prefix.failing_requiredinsrc/cli/pr_merge_check.rslooked each entry up by name in the combined status instead. No check is literally called*, so it was alwaysmissing, and because checks rank above approvals and the raw error indiagnose, that invented reason also buried whatever actually made the server refuse the merge.The comparison is now a pure
required_check_failures, modelled on Forgejo's merge gate,MergeRequiredContextsCommitStatusinservices/pull/commit_status.go(read from a Forgejo 16 dev checkout). Each pattern must match at least one reported status, and every status it matches must besuccessorskipped;pending,warning,errorandfailureblock.skippedpasses because Forgejo ranks it abovesuccess, so it never lowers the verdict; that is what lets paragon's promotion-onlylint / develop-superset-of-mainsit skipped on every develop PR. A pattern that matches nothing is stillmissing, as Forgejo treats it as pending.Two wording changes follow from that. A failing check is now reported under its real name, so a
*rule says"CI / test (pull_request)" is not green (state: failure)rather than quoting the pattern. And a glob that matches nothing saysno reported check matches the required pattern "*". Literal context names keep the existing message.The matcher is about a hundred lines, hand-written, covering the syntax gobwas/glob accepts without separators (
*,?,[a-z],[!x],{a,b},\escapes). Theglobcrate is already inCargo.lock(transitively), but it rejects**anywhere other than a whole path component and has no{a,b}alternation, so it would disagree with Forgejo on patterns Forgejo accepts. A pattern that fails to compile is ignored, which is what Forgejo does with it.This was the only place fj compared required contexts. The
Mergeable:line infj pr viewandfj pr checksreads only the PR's ownmergeableflag, and--autoreaches the same diagnosis after a rejected merge, so both pick the fix up without changes.One consequence worth knowing: the diagnosis only runs after Forgejo has already refused the merge. On #1634 the server did refuse something, the web UI and a raw
POST .../mergewithDo=squashboth succeeded, and with the fake*blocker gone that underlying reason, or the raw API error, is whatfj pr mergewill now print. If it turns out to be another fj defect it wants its own issue.Closes #238
Verified on macOS:
cargo fmt --allclean,cargo clippy --all-targets --all-features -- -D warningsclean,cargo test --allgreen (831 + 5 + 1), pre-push hook passed.