Skip to content

Commit f2e08fd

Browse files
committed
branch_worker: Fix _series_already_applied() helper
After a partial `git am` failure (some patches in a series apply successfully before one hits a merge conflict), _series_already_applied() walks commit history starting from the current HEAD. This HEAD includes commits that were just created by the partially-successful `git am`, so the function matches the series' own patch subjects against its own just-applied commits and incorrectly concludes the series was "already merged" into upstream. The consequence is that a NewPRWithNoChangeException is raised instead of creating a merge-conflict PR. The series remains in-queue on Patchwork with no corresponding GitHub PR, producing a persistent KPD/Patchwork discrepancy. Fix this by adding an explicit `rev` parameter to _series_already_applied() and passing it through to repo.iter_commits(). The caller now supplies the upstream remote-tracking ref (e.g. "upstream/bpf-next"), so the history walk is anchored to the genuine upstream branch tip rather than the working HEAD that may contain partially-applied series commits. Assisted-by: Claude:claude-opus-4-6 Signed-off-by: Ihor Solodrai <isolodrai@meta.com>
1 parent 46ba364 commit f2e08fd

File tree

2 files changed

+15
-12
lines changed

2 files changed

+15
-12
lines changed

kernel_patches_daemon/branch_worker.py

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -580,10 +580,10 @@ def _reset_repo(repo, branch: str) -> None:
580580
repo.git.checkout(branch)
581581

582582

583-
async def _series_already_applied(repo: git.Repo, series: Series) -> bool:
583+
async def _series_already_applied(repo: git.Repo, series: Series, rev: str) -> bool:
584584
"""
585585
Returns whether or not the given series has already been applied
586-
to the active branch on `repo`.
586+
to the given revision in `repo`.
587587
588588
We consider even a single applied patch in a series as the series being
589589
applied. This is b/c the applied patch is marked as "accepted" and thus the
@@ -597,7 +597,7 @@ async def _series_already_applied(repo: git.Repo, series: Series) -> bool:
597597
try:
598598
summaries = {
599599
commit.summary.lower()
600-
for commit in repo.iter_commits(max_count=ALREADY_MERGED_LOOKBACK)
600+
for commit in repo.iter_commits(rev, max_count=ALREADY_MERGED_LOOKBACK)
601601
}
602602
except git.exc.GitCommandError:
603603
logger.exception("Failed to check series application status")
@@ -687,7 +687,7 @@ def __init__(
687687
self.repo_dir = _uniq_tmp_folder(repo_url, repo_branch, base_directory)
688688
self.repo_branch = repo_branch
689689
self.repo_pr_base_branch = repo_branch + "_base"
690-
self.repo_local = None
690+
self.repo_local: Optional[git.Repo] = None
691691

692692
self.upstream_url = upstream_url
693693
self.upstream_branch = upstream_branch
@@ -844,7 +844,6 @@ def fetch_repo_branch(self) -> None:
844844
"""
845845
Fetch the repository branch of interest only once
846846
"""
847-
# pyrefly: ignore # bad-assignment
848847
self.repo_local = self.fetch_repo(
849848
self.repo_dir, self.repo_url, self.repo_branch
850849
)
@@ -1109,8 +1108,12 @@ async def apply_push_comment(
11091108
# In other words, patchwork could be reporting a relevant
11101109
# status (ie. !accepted) while the series has already been
11111110
# merged and pushed.
1112-
# pyrefly: ignore # bad-argument-type
1113-
if await _series_already_applied(self.repo_local, series):
1111+
assert self.repo_local is not None
1112+
if await _series_already_applied(
1113+
self.repo_local,
1114+
series,
1115+
f"{UPSTREAM_REMOTE_NAME}/{self.upstream_branch}",
1116+
):
11141117
logger.info(f"Series {series.url} already applied to tree")
11151118
raise NewPRWithNoChangeException(self.repo_pr_base_branch, branch_name)
11161119

tests/test_branch_worker.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1015,7 +1015,7 @@ async def test_applied_all(self, m: aioresponses):
10151015
in_1 = ALREADY_MERGED_LOOKBACK + 33
10161016
in_2 = ALREADY_MERGED_LOOKBACK + 34
10171017
series = await self._get_series(m, [f"Commit {in_1}", f"[tag] Commit {in_2}"])
1018-
self.assertTrue(await _series_already_applied(self.repo, series))
1018+
self.assertTrue(await _series_already_applied(self.repo, series, "HEAD"))
10191019

10201020
@aioresponses()
10211021
async def test_applied_none_newer(self, m: aioresponses):
@@ -1024,26 +1024,26 @@ async def test_applied_none_newer(self, m: aioresponses):
10241024
series = await self._get_series(
10251025
m, [f"[some tags]Commit {out_1}", f"[tag] Commit {out_2}"]
10261026
)
1027-
self.assertFalse(await _series_already_applied(self.repo, series))
1027+
self.assertFalse(await _series_already_applied(self.repo, series, "HEAD"))
10281028

10291029
@aioresponses()
10301030
async def test_applied_none_older(self, m: aioresponses):
10311031
series = await self._get_series(m, ["[some tags]Commit 33", "[tag] Commit 34"])
1032-
self.assertFalse(await _series_already_applied(self.repo, series))
1032+
self.assertFalse(await _series_already_applied(self.repo, series, "HEAD"))
10331033

10341034
@aioresponses()
10351035
async def test_applied_some(self, m: aioresponses):
10361036
inside = ALREADY_MERGED_LOOKBACK + 55
10371037
out = ALREADY_MERGED_LOOKBACK * 3
10381038
series = await self._get_series(m, [f"Commit {inside}", f"Commit {out}"])
1039-
self.assertTrue(await _series_already_applied(self.repo, series))
1039+
self.assertTrue(await _series_already_applied(self.repo, series, "HEAD"))
10401040

10411041
@aioresponses()
10421042
async def test_applied_all_case_insensitive(self, m: aioresponses):
10431043
in_1 = ALREADY_MERGED_LOOKBACK + 33
10441044
in_2 = ALREADY_MERGED_LOOKBACK + 34
10451045
series = await self._get_series(m, [f"commit {in_1}", f"[tag] COMMIT {in_2}"])
1046-
self.assertTrue(await _series_already_applied(self.repo, series))
1046+
self.assertTrue(await _series_already_applied(self.repo, series, "HEAD"))
10471047

10481048

10491049
class TestBranchChanged(unittest.TestCase):

0 commit comments

Comments
 (0)