Script 'mail_helper' called by obssrc Hello community, here is the log from the commit of package b4 for openSUSE:Factory checked in at 2026-04-10 17:54:01 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/b4 (Old) and /work/SRC/openSUSE:Factory/.b4.new.21863 (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "b4" Fri Apr 10 17:54:01 2026 rev:47 rq:1345761 version:0.15.2 Changes: -------- --- /work/SRC/openSUSE:Factory/b4/b4.changes 2026-03-27 06:36:43.102353095 +0100 +++ /work/SRC/openSUSE:Factory/.b4.new.21863/b4.changes 2026-04-10 18:03:23.179533739 +0200 @@ -1,0 +2,24 @@ +Fri Apr 10 06:30:23 UTC 2026 - Jiri Slaby <[email protected]> + +- update to 0.15.2 + * review: show visual cue for patches unchanged between revisions -- + when upgrading a series, identical patches now display a dim "≡" + marker so you can focus your review on patches that actually changed + * review: prevent double-sending of reviews carried over from a prior + revision after a series upgrade + * review: fix crash when stored message-id points to wrong version + thread after a series upgrade + * review: fix IndexError crash when toggling followups + * review: fix crash when background rescan completes while a modal + screen is active + * review: fix replies to additional patches being treated as inline + reviews on the wrong patch + * review: fix range-diff failing to find previous series revision + * review: fix upgrade branches polluting tracking DB with wrong + change*id + * review: show per-series error details after "Update all" instead of + only a total error count + * mbox: don't split on "---" in free-form reply messages (fixes body + content being silently discarded when encountering a "---" marker) + +------------------------------------------------------------------- Old: ---- b4-0.15.1.tar.gz New: ---- b4-0.15.2.tar.gz ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ b4.spec ++++++ --- /var/tmp/diff_new_pack.Cz4eI2/_old 2026-04-10 18:03:23.711555683 +0200 +++ /var/tmp/diff_new_pack.Cz4eI2/_new 2026-04-10 18:03:23.711555683 +0200 @@ -24,7 +24,7 @@ %global pprefix python311 %endif Name: b4 -Version: 0.15.1 +Version: 0.15.2 Release: 0 Summary: Helper scripts for kernel.org patches License: GPL-2.0-or-later ++++++ _scmsync.obsinfo ++++++ --- /var/tmp/diff_new_pack.Cz4eI2/_old 2026-04-10 18:03:23.751557333 +0200 +++ /var/tmp/diff_new_pack.Cz4eI2/_new 2026-04-10 18:03:23.763557828 +0200 @@ -1,4 +1,4 @@ -mtime: 1774432836 -commit: bb6143d8c64459532c569877848de2561a8049b220433ef36d7a79872b7e470c +mtime: 1775802678 +commit: 7eda34324e58d70cb364695437dbe4407f98f734039fc3a6670fe78fdca43b3c url: https://src.opensuse.org/jirislaby/d-t-b4.git ++++++ b4-0.15.1.tar.gz -> b4-0.15.2.tar.gz ++++++ diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/b4-0.15.1/.gitignore new/b4-0.15.2/.gitignore --- old/b4-0.15.1/.gitignore 2026-03-24 15:09:05.000000000 +0100 +++ new/b4-0.15.2/.gitignore 2026-04-09 20:43:04.000000000 +0200 @@ -17,6 +17,7 @@ *.cover *.thanks .venv +uv.lock qodana.yaml *.ipynb pytest.log diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/b4-0.15.1/docs/maintainer/review.rst new/b4-0.15.2/docs/maintainer/review.rst --- old/b4-0.15.1/docs/maintainer/review.rst 2026-03-24 15:09:05.000000000 +0100 +++ new/b4-0.15.2/docs/maintainer/review.rst 2026-04-09 20:43:04.000000000 +0200 @@ -486,6 +486,7 @@ State Marker Key Meaning ========== ====== ====== ==================================================== *(none)* |sp| — Not yet reviewed +Unchanged ``≡`` — Patch identical to prior revision (auto, on upgrade) External ``±`` — External reviewers have inline comments (auto) Draft ``✎`` — You have comments, a reply, or a nack trailer (auto) Done ``✓`` ``d`` Review complete, include in outgoing emails @@ -497,13 +498,16 @@ The marker column shows the highest-priority state that applies. For example, if external reviewers have commented but you have not yet reviewed, the marker shows ``±``; once you add your own comments it -changes to ``✎``. +changes to ``✎``. The ``≡`` marker is set automatically when upgrading +a series to a newer revision — it indicates that the patch content has +not changed, giving you a visual cue to focus on patches that did. Any +review action (adding a trailer, reply, or comment) supersedes it. Pressing ``d`` or ``x`` toggles the state for the current patch. The patch list on the left shows the state visually: done patches appear in -bold, skipped patches appear dimmed. Press ``H`` to hide skipped patches -from the list entirely — useful for large series where many patches are -irrelevant to your review. +bold, skipped and unchanged patches appear dimmed. Press ``H`` to hide +skipped patches from the list entirely — useful for large series where +many patches are irrelevant to your review. Skipped patches are automatically excluded when sending review emails. When taking patches via cherry-pick, skipped patches are pre-deselected diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/b4-0.15.1/docs/releases.rst new/b4-0.15.2/docs/releases.rst --- old/b4-0.15.1/docs/releases.rst 2026-03-24 15:09:05.000000000 +0100 +++ new/b4-0.15.2/docs/releases.rst 2026-04-09 20:43:04.000000000 +0200 @@ -1,6 +1,98 @@ Release notes ============= +.. _release-0.15.2: + +v0.15.2 +------- + +Bug fixes for the review TUI and mbox parsing, plus a new visual +indicator for unchanged patches during series upgrades. + +Review TUI fixes +~~~~~~~~~~~~~~~~ + +- **Unchanged-patch visual cue** — when upgrading a series to a newer + revision, patches whose content is identical between revisions now + display a ``≡`` marker and are rendered dim in the patch list, giving + the maintainer an at-a-glance signal to focus on patches that actually + changed. The ``≡`` state is automatically superseded by any review + action (adding a trailer, reply, or comment). +- **Prevent double-sending of carried-over reviews** — after a + successful send, each review is stamped with a ``sent-revision`` + field so ``collect_review_emails()`` skips reviews that have already + been sent for a previous revision. +- **Crash fix: stored message-id pointing to wrong version thread** — + after upgrading a series, the stored message-id could still reference + the old version's thread, causing a ``LookupError``. B4 now retries + with ``get_extra_series()`` to discover the correct version. +- **Crash fix: IndexError when toggling followups** — a race between + Textual's ``ListView.clear()`` and DOM removal could leave the cursor + past the end of the list. Fixed by tracking the new-item count + directly and deferring index restoration. +- **Crash fix: modal screen blocking _refresh_list** — the background + rescan finishing while a modal screen was active caused a + ``NoMatches`` crash. Added guards to return early when the default + screen's widgets are unreachable. +- **Replies to additional patches no longer treated as inline reviews** + — when a follow-up message contains its own diff, replies to it are + no longer injected as code reviews on the wrong patch. +- **Range-diff finds previous series revision** — pressing ``d`` + previously passed the current revision's blob SHA when fetching the + older one, causing a version mismatch. +- **Upgrade branches no longer pollute tracking DB** — temporary + upgrade branches now use a ``_tmp-`` prefix and are skipped during + rescan, preventing ghost entries with garbled change-ids. +- **Per-series error details after update-all** — the "Update all" + action now reports which series failed and why, instead of only + showing a total error count. + +Mbox parsing fix +~~~~~~~~~~~~~~~~ + +- **Free-form replies no longer split on** ``---`` — reply messages + that use ``---`` as a visual separator (e.g. AI-assisted reviews) no + longer have their body content silently discarded. The ``---``/diff + split is now only applied to messages that contain actual diff content. + +Thanks +~~~~~~ + +Thanks to the following people for reporting bugs and testing: + +- Mark Brown +- Miroslav Benes + + +.. _release-0.15.1: + +v0.15.1 +------- + +Bug fixes for the review TUI and documentation corrections. + +- **Patch list scrolling** — the patch list in the review TUI now + scrolls properly on large series instead of expanding beyond its + container. Keyboard navigation keeps the selected patch visible. + (Reported by Chris Samuel) +- **CI results list scrolling** — the check results matrix no longer + resets to the top on every re-render, so keyboard navigation works + correctly. (Reported by Chris Samuel) +- **TUI tests outside a git repository** — tests no longer fail when + run from a release tarball with no ``.git`` directory. + (Reported by Jiri Slaby) +- **Documentation** — fixed incorrect ``b4 dig`` examples in the v0.15 + release notes. + +Thanks +~~~~~~ + +Thanks to the following people for reporting bugs: + +- Chris Samuel +- Jiri Slaby + + .. _release-0.15: v0.15 diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/b4-0.15.1/pyproject.toml new/b4-0.15.2/pyproject.toml --- old/b4-0.15.1/pyproject.toml 2026-03-24 15:09:05.000000000 +0100 +++ new/b4-0.15.2/pyproject.toml 2026-04-09 20:43:04.000000000 +0200 @@ -4,7 +4,7 @@ [project] name = "b4" -version = "0.15.1" +version = "0.15.2" description = "A tool to work with public-inbox and patch archives" readme = "README.rst" keywords = ["git", "public-inbox", "lore.kernel.org", "patch", "email", "workflow"] @@ -72,7 +72,7 @@ log_file_date_format = "%Y-%m-%d %H:%M:%S" [tool.bumpversion] -current_version = "0.15.1" +current_version = "0.15.2" files = [ {filename = "src/b4/__init__.py"}, {filename = "src/b4/man/b4.1.rst"}, diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/b4-0.15.1/src/b4/__init__.py new/b4-0.15.2/src/b4/__init__.py --- old/b4-0.15.1/src/b4/__init__.py 2026-03-24 15:09:05.000000000 +0100 +++ new/b4-0.15.2/src/b4/__init__.py 2026-04-09 20:43:04.000000000 +0200 @@ -69,7 +69,7 @@ # global setting allowing us to turn off networking can_network = True -__VERSION__ = '0.15.1' +__VERSION__ = '0.15.2' PW_REST_API_VERSION = '1.2' @@ -2474,20 +2474,25 @@ signature = sparts[1] body = sparts[0] - parts = re.split(r'^---\s*\n', body, maxsplit=1, flags=re.M) - if len(parts) == 2: - basement = parts[1] - elif body.find('\ndiff ') >= 0: - parts = body.split('\ndiff ', 1) + # Only apply patch-separator splitting for messages that contain actual + # diff content. Free-form replies should not be split on '---', which + # may appear as a visual separator unrelated to any patch. + parts = [body] + if DIFF_RE.search(body) or DIFFSTAT_RE.search(body): + parts = re.split(r'^---\s*\n', body, maxsplit=1, flags=re.M) if len(parts) == 2: - parts[1] = 'diff ' + parts[1] - basement = parts[1] - elif body.find('\n--- a/') >= 0: - # patches generated by some really peculiar tools - parts = body.split('\n--- a/', 1) - if len(parts) == 2: - parts[1] = '--- a/' + parts[1] - basement = parts[1] + basement = parts[1] + elif body.find('\ndiff ') >= 0: + parts = body.split('\ndiff ', 1) + if len(parts) == 2: + parts[1] = 'diff ' + parts[1] + basement = parts[1] + elif body.find('\n--- a/') >= 0: + # patches generated by some really peculiar tools + parts = body.split('\n--- a/', 1) + if len(parts) == 2: + parts[1] = '--- a/' + parts[1] + basement = parts[1] mbody = parts[0].strip('\n') diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/b4-0.15.1/src/b4/review/_review.py new/b4-0.15.2/src/b4/review/_review.py --- old/b4-0.15.1/src/b4/review/_review.py 2026-03-24 15:09:05.000000000 +0100 +++ new/b4-0.15.2/src/b4/review/_review.py 2026-04-09 20:43:04.000000000 +0200 @@ -194,7 +194,10 @@ if wantver is None: wantver = max(lmbx.series.keys()) if wantver not in lmbx.series: - raise LookupError(f'Series version {wantver} not found in retrieved messages') + found = ', '.join(f'v{v}' for v in sorted(lmbx.series.keys())) + raise LookupError( + f'Series version {wantver} not found in retrieved messages' + f' (found: {found})') lser = lmbx.get_series(wantver, sloppytrailers=sloppytrailers, codereview_trailers=False) if not lser: @@ -696,11 +699,11 @@ def _get_patch_state(target: Dict[str, Any], usercfg: b4.ConfigDictT) -> str: """Derive the effective per-patch state for the current user. - Returns 'done', 'draft', 'external', 'skip', or '' (no state). - Only 'done' and 'skip' are stored explicitly; other states are derived. - The 'external' state indicates that external reviewers (agents, - sashiko, follow-ups) have inline comments but the maintainer has - not yet commented. + Returns 'done', 'draft', 'external', 'skip', 'unchanged', or '' + (no state). 'done', 'skip', and 'unchanged' are stored explicitly; + other states are derived. 'unchanged' is low-priority: it is only + returned when no derived state (draft, done, external) applies, so + any maintainer action automatically supersedes it. """ review = _get_my_review(target, usercfg) explicit = str(review.get('patch-state', '')) @@ -722,12 +725,12 @@ if any(addr != my_email and rev.get('comments') for addr, rev in all_reviews.items()): return 'external' - return '' + return explicit if explicit else '' def _set_patch_state(target: Dict[str, Any], usercfg: b4.ConfigDictT, state: str) -> None: - """Store an explicit patch state ('done', 'skip', or '' to clear).""" + """Store an explicit patch state ('done', 'skip', 'unchanged', or '' to clear).""" if state: review = _ensure_my_review(target, usercfg) review['patch-state'] = state @@ -2339,7 +2342,9 @@ # Cover letter review (maintainer only) cover_review = series.get('reviews', {}).get(my_email, {}) - if cover_review and cover_review.get('patch-state') != 'skip': + if (cover_review + and cover_review.get('patch-state') != 'skip' + and cover_review.get('sent-revision') is None): msg = _build_review_email(series, None, cover_review, cover_text, topdir, None) if msg is not None: @@ -2350,6 +2355,8 @@ patch_review = patch_meta.get('reviews', {}).get(my_email, {}) if not patch_review or patch_review.get('patch-state') == 'skip': continue + if patch_review.get('sent-revision') is not None: + continue commit_sha = commit_shas[idx] if idx < len(commit_shas) else None msg = _build_review_email(series, patch_meta, patch_review, cover_text, topdir, commit_sha) diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/b4-0.15.1/src/b4/review/tracking.py new/b4-0.15.2/src/b4/review/tracking.py --- old/b4-0.15.1/src/b4/review/tracking.py 2026-03-24 15:09:05.000000000 +0100 +++ new/b4-0.15.2/src/b4/review/tracking.py 2026-04-09 20:43:04.000000000 +0200 @@ -1646,6 +1646,9 @@ changed = 0 for br in branches: + # Skip temporary branches (e.g. upgrade work branches) + if br.startswith('b4/review/_tmp-'): + continue # Derive change_id from the branch name for the fast SHA check. change_id_from_branch = br.removeprefix('b4/review/') diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/b4-0.15.1/src/b4/review_tui/_common.py new/b4-0.15.2/src/b4/review_tui/_common.py --- old/b4-0.15.1/src/b4/review_tui/_common.py 2026-03-24 15:09:05.000000000 +0100 +++ new/b4-0.15.2/src/b4/review_tui/_common.py 2026-04-09 20:43:04.000000000 +0200 @@ -65,11 +65,12 @@ # Per-patch state indicators — same glyphs as _tracking_app._STATUS_SYMBOLS PATCH_STATE_MARKERS: Dict[str, str] = { - '': ' ', - 'external': '\u00b1', # ± plus-minus (= external comments available) - 'draft': '\u270e', # ✎ pencil (= maintainer reviewing) - 'done': '\u2713', # ✓ check (= done) - 'skip': '\u2715', # ✕ cross (= skipped) + '': ' ', + 'external': '\u00b1', # ± plus-minus (= external comments available) + 'draft': '\u270e', # ✎ pencil (= maintainer reviewing) + 'done': '\u2713', # ✓ check (= done) + 'skip': '\u2715', # ✕ cross (= skipped) + 'unchanged': '\u2261', # ≡ identical-to (= unchanged from prior rev) } # CI check label text (colour-free constant — used with ci_check_styles()) @@ -661,6 +662,33 @@ return None +def _chain_has_additional_patch( + in_reply_to: Optional[str], + patch_msgids: Dict[str, int], + msgid_map: Dict[str, 'b4.LoreMessage'], +) -> bool: + """Check whether the in-reply-to chain passes through a message that + contains its own diff before reaching a known series patch. + + Such a message is an additional patch posted as a follow-up. + Replies to it discuss that new code, so their quoted diffs should not + be treated as inline reviews of the original series patch. + """ + seen: Set[str] = set() + current = in_reply_to + while current and current not in seen: + if current in patch_msgids: + return False + seen.add(current) + lmsg = msgid_map.get(current) + if lmsg is None: + break + if lmsg.has_diff: + return True + current = lmsg.in_reply_to + return False + + def _write_comments( viewer: 'RichLog', entries: List[Tuple[str, str, str]], diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/b4-0.15.1/src/b4/review_tui/_modals.py new/b4-0.15.2/src/b4/review_tui/_modals.py --- old/b4-0.15.1/src/b4/review_tui/_modals.py 2026-03-24 15:09:05.000000000 +0100 +++ new/b4-0.15.2/src/b4/review_tui/_modals.py 2026-04-09 20:43:04.000000000 +0200 @@ -2718,7 +2718,7 @@ self.dismiss((self._ok, self._fail, self._new_state)) -class UpdateAllScreen(ModalScreen[Dict[str, int]]): +class UpdateAllScreen(ModalScreen[Dict[str, Any]]): """Modal showing progress while updating all tracked series. Iterates every non-archived series, fetching threads and updating @@ -2763,13 +2763,14 @@ self._linkmask = linkmask self._topdir = topdir self._cancelled = False - self._result: Dict[str, int] = { + self._result: Dict[str, Any] = { 'series_checked': 0, 'series_updated': 0, 'promoted': 0, 'errors': 0, 'gone': 0, 'followup_updated': 0, + 'error_details': [], } def compose(self) -> ComposeResult: @@ -2787,7 +2788,7 @@ def action_cancel(self) -> None: self._cancelled = True - def _do_updates(self) -> Dict[str, int]: + def _do_updates(self) -> Dict[str, Any]: import b4.review with _quiet_worker(): @@ -2819,6 +2820,8 @@ self._result['promoted'] += 1 if r.get('error'): self._result['errors'] += 1 + submitter = series.get('submitter', 'unknown') + self._result['error_details'].append((submitter, r['error'])) if r.get('counts_updated'): self._result['followup_updated'] += 1 diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/b4-0.15.1/src/b4/review_tui/_review_app.py new/b4-0.15.2/src/b4/review_tui/_review_app.py --- old/b4-0.15.1/src/b4/review_tui/_review_app.py 2026-03-24 15:09:05.000000000 +0100 +++ new/b4-0.15.2/src/b4/review_tui/_review_app.py 2026-04-09 20:43:04.000000000 +0200 @@ -32,7 +32,7 @@ _quiet_worker, get_thread_msgs, _has_review_data, _make_initials, _wait_for_enter, _write_comments, _write_followup_comments, - _write_followup_trailers, _resolve_patch_for_followup, + _write_followup_trailers, _resolve_patch_for_followup, _chain_has_additional_patch, _get_followup_depth, _render_email_to_viewer, _suspend_to_shell, SeparatedFooter, _fix_ansi_theme, ) @@ -59,7 +59,7 @@ def _apply_state_style(self) -> None: lbl = self.query_one(Label) - if self._state == 'skip': + if self._state in ('skip', 'unchanged'): lbl.styles.text_style = 'dim' elif self._state == 'done': lbl.styles.text_style = 'bold' @@ -325,6 +325,10 @@ lv.clear() total = len(self._commit_shas) + # Track new-item count ourselves: lv.children may still + # contain old items pending async removal after clear(). + new_count = 0 + restore_index = 0 # Cover letter entry (skip if no actual cover letter) if self._has_cover: @@ -332,7 +336,11 @@ mark = PATCH_STATE_MARKERS[state] cover_label = f'{mark} 0/{total} {self._cover_subject_clean[:40]}' lv.append(PatchListItem(cover_label, 0, state)) + if 0 == self._selected_idx: + restore_index = new_count + new_count += 1 self._append_followup_items(lv, 0) + new_count += len(self._followup_comments.get(0, [])) # Patch entries for idx, _sha in enumerate(self._commit_shas): @@ -345,15 +353,21 @@ mark = PATCH_STATE_MARKERS[state] label = f'{mark} {patch_num}/{total} {subject[:40]}' lv.append(PatchListItem(label, patch_num, state)) + if patch_num == self._selected_idx: + restore_index = new_count + new_count += 1 self._append_followup_items(lv, patch_num) + new_count += len(self._followup_comments.get(patch_num, [])) - if lv.children: - # Find the child index for the currently selected display_idx - for i, child in enumerate(lv.children): - if isinstance(child, PatchListItem) and child.patch_idx == self._selected_idx: - lv.index = i - lv.scroll_visible() - break + if new_count > 0: + # Defer index restoration: clear() only schedules DOM + # removal, so lv._nodes still contains old items right + # now. Setting the index immediately would highlight a + # stale item that disappears once pruning completes. + def _restore() -> None: + lv.index = restore_index + lv.scroll_visible() + self.call_after_refresh(_restore) def _append_followup_items(self, lv: ListView, display_idx: int) -> None: """Append FollowupItem entries to *lv* for each follow-up message.""" @@ -719,7 +733,12 @@ if target and b4.review._get_patch_state(target, self._usercfg) == 'skip': total = len(self._commit_shas) label = 'cover' if display_idx == 0 else f'{display_idx}/{total}' - viewer.write(f'[dim]Patch {label} is marked as skipped — no email will be sent.[/dim]') + my_review = b4.review._get_my_review(target, self._usercfg) + skip_reason = str(my_review.get('skip-reason', '')) + if skip_reason: + viewer.write(f'[dim]Patch {label} is marked as skipped: {skip_reason}[/dim]') + else: + viewer.write(f'[dim]Patch {label} is marked as skipped — no email will be sent.[/dim]') return if not review or not (review.get('trailers') or review.get('reply', '') @@ -1428,6 +1447,16 @@ else: self._reply_sent = True self._tracking['series']['status'] = 'replied' + # Stamp sent-revision on every non-skip review so that + # if this series is later upgraded to a newer revision, + # the upgrade step can detect which reviews were already + # sent and auto-skip the unchanged patches. + my_email = str(self._usercfg.get('email', '[email protected]')) + current_rev = int(self._series.get('revision', 1)) + for target in [self._series] + list(self._patches): + review = target.get('reviews', {}).get(my_email, {}) + if review and review.get('patch-state') != 'skip': + review['sent-revision'] = current_rev self._save_tracking() self._mark_patches_answered(msgs) self.notify(f'Sent {sent} review email(s).') @@ -1576,6 +1605,8 @@ 'reply': lmsg.reply, 'depth': _get_followup_depth(lmsg.in_reply_to, patch_msgids, lmbx.msgid_map), 'lmsg': lmsg, + 'replies-to-diff': _chain_has_additional_patch( + lmsg.in_reply_to, patch_msgids, lmbx.msgid_map), } self._followup_comments.setdefault(display_idx, []).append(entry) count += 1 @@ -1639,6 +1670,10 @@ continue if '> diff --git ' not in body and '> @@ ' not in body: continue + # Skip followups that reply to an additional patch: + # quoted diff content is from that patch, not ours. + if fc.get('replies-to-diff'): + continue reviewer_email = fc.get('fromemail', '') if not reviewer_email: diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/b4-0.15.1/src/b4/review_tui/_tracking_app.py new/b4-0.15.2/src/b4/review_tui/_tracking_app.py --- old/b4-0.15.1/src/b4/review_tui/_tracking_app.py 2026-03-24 15:09:05.000000000 +0100 +++ new/b4-0.15.2/src/b4/review_tui/_tracking_app.py 2026-04-09 20:43:04.000000000 +0200 @@ -5,6 +5,7 @@ # __author__ = 'Konstantin Ryabitsev <[email protected]>' +import copy import datetime import email.message import email.parser @@ -579,6 +580,10 @@ # only written when a branch SHA differs, so _check_db_changed() # naturally stays quiet on a no-op rescan. if gone or result.get('changed', 0): + # Skip while a modal is active — _check_db_changed() will + # pick up the change once the modal closes. + if len(self.app.screen_stack) > 1: + return if self._selected_series: self._focus_change_id = self._selected_series.get('change_id') self._load_series() @@ -760,7 +765,14 @@ if self._matches_limit(s, self._limit_pattern) ] - left = self.query_one('#title-left', Static) + try: + left = self.query_one('#title-left', Static) + except NoMatches: + # A modal screen is active or the app is shutting down — + # the widget lives on the default screen and is not + # reachable right now. The 1-second _check_db_changed + # timer will re-trigger the refresh once the modal closes. + return title_text = f' Tracked Series — {self._identifier}' if self._email_dryrun: title_text += ' (DRY-RUN)' @@ -1025,15 +1037,26 @@ except Exception: self.notify('Could not load revision data', severity='error') return + rev_info = None for rev in revs: if rev['revision'] == chosen: - series['message_id'] = rev['message_id'] - series['revision'] = chosen + rev_info = rev break - else: + if rev_info is None: self.notify(f'Revision v{chosen} not found in database', severity='error') return + series['message_id'] = rev_info['message_id'] + series['revision'] = chosen + # Persist the switch so it survives checkout failures + try: + conn = b4.review.tracking.get_db(self._identifier) + b4.review.tracking.update_series_revision( + conn, change_id, current_rev, chosen, + rev_info['message_id'], rev_info.get('subject')) + conn.close() + except Exception: + pass self._checkout_new_series() def action_thread(self) -> None: @@ -1074,7 +1097,23 @@ msgs = b4.review._retrieve_messages(message_id) self._refresh_msg_count(series, len(msgs)) wantver = series.get('revision') - lser = b4.review._get_lore_series(msgs, wantver=wantver) + try: + lser = b4.review._get_lore_series(msgs, wantver=wantver) + except LookupError: + if wantver is not None and b4.can_network: + # The stored message-id may point to a different + # version's thread. Search for the wanted version + # in other threads before giving up. + msgs = b4.mbox.get_extra_series( + msgs, direction=1, nocache=True) + if wantver > 1: + msgs = b4.mbox.get_extra_series( + msgs, direction=-1, + wantvers=[wantver], nocache=True) + lser = b4.review._get_lore_series( + msgs, wantver=wantver) + else: + raise am_msgs = lser.get_am_ready( noaddtrailers=True, addmysob=False, addlink=False, @@ -1681,7 +1720,7 @@ callback=self._on_update_complete, ) - def _on_update_complete(self, result: Optional[Dict[str, int]]) -> None: + def _on_update_complete(self, result: Optional[Dict[str, Any]]) -> None: """Build a summary notification from an update result.""" if result is None: return @@ -1690,6 +1729,7 @@ promoted = result.get('promoted', 0) errors = result.get('errors', 0) gone = result.get('gone', 0) + error_details: list[tuple[str, str]] = result.get('error_details', []) if gone: self.notify(f'{gone} review branch(es) are gone', severity='warning') @@ -1704,6 +1744,9 @@ severity: Literal['information', 'warning'] = 'warning' if errors else 'information' self.notify(', '.join(parts), severity=severity) + for submitter, error in error_details: + logger.warning('Update error (%s): %s', submitter, error) + self.notify(f'{submitter}: {error}', severity='error') self._load_series() def action_hide_details(self) -> None: @@ -2753,7 +2796,6 @@ branch = f'b4/review/{change_id}' cur_start: Optional[str] = None cur_end: Optional[str] = None - blob_sha = '' if b4.git_branch_exists(topdir, branch): try: _cover_text, tracking = b4.review.load_tracking(topdir, branch) @@ -2764,7 +2806,6 @@ else: cur_start = t_series.get('base-commit', '') cur_end = f'{branch}~1' - blob_sha = t_series.get('thread-blob', '') except SystemExit: pass @@ -2776,9 +2817,9 @@ return cur_start, cur_end = result - # --- Fetch the other version (try cached blob, fall back to lore) --- - result = self._fetch_fake_am_range(topdir, revisions, other_rev, - blob_sha=blob_sha) + # --- Fetch the other version (always from lore; the cached blob + # belongs to the current revision's thread, not the other one) --- + result = self._fetch_fake_am_range(topdir, revisions, other_rev) if result is None: _wait_for_enter() return @@ -3140,7 +3181,7 @@ self.notify('Update cancelled', severity='information') return - upgrade_branch = f'b4/review/{change_id}-v{target_rev}-upgrade' + upgrade_branch = f'b4/review/_tmp-{change_id}-v{target_rev}-upgrade' with self.suspend(): topdir = b4.git_get_toplevel() @@ -3156,9 +3197,11 @@ patches = tracking.get('patches', []) saved_reviews: Dict[str, Dict[str, Any]] = {} + prior_patch_ids: set[str] = set() for idx, _sha, patch_id in patch_ids: if patch_id is None or idx >= len(patches): continue + prior_patch_ids.add(patch_id) reviews = patches[idx].get('reviews') if reviews: saved_reviews[patch_id] = { @@ -3250,19 +3293,40 @@ topdir, upgrade_branch) new_patches = new_tracking.get('patches', []) + usercfg = b4.get_user_config() + my_email = str(usercfg.get('email', '[email protected]')) restored = 0 for idx, _sha, patch_id in new_patch_ids: if patch_id is None or idx >= len(new_patches): continue + if patch_id not in prior_patch_ids: + continue if patch_id in saved_reviews: - new_patches[idx]['reviews'] = saved_reviews[patch_id]['reviews'] + # Deepcopy so we don't mutate the saved review data + reviews = copy.deepcopy(saved_reviews[patch_id]['reviews']) restored += 1 + else: + reviews = {} + # Mark the patch as unchanged so the TUI can show a visual cue. + # _get_patch_state() will override this with any derived state + # (draft, done, …) if the maintainer already has review content. + my_review = reviews.setdefault( + my_email, {'name': str(usercfg.get('name', ''))}) + my_review['patch-state'] = 'unchanged' + reviews['b4-upgrade@internal'] = { + 'name': 'B4 Upgrade', + 'note': f'Patch unchanged from v{current_rev}', + } + new_patches[idx]['reviews'] = reviews # Re-anchor inline comments against new revision diffs new_shas = [sha for _idx, sha, _pid in new_patch_ids] b4.review.reanchor_patch_comments(topdir, new_shas, new_patches) new_tracking['series']['status'] = 'reviewing' + # Ensure the tracking commit has the real change-id, not + # the temporary upgrade branch name. + new_tracking['series']['change-id'] = change_id if prior_context: new_tracking['series']['prior-review-context'] = prior_context if prior_thread_blob: diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/b4-0.15.1/src/tests/test_review.py new/b4-0.15.2/src/tests/test_review.py --- old/b4-0.15.1/src/tests/test_review.py 2026-03-24 15:09:05.000000000 +0100 +++ new/b4-0.15.2/src/tests/test_review.py 2026-04-09 20:43:04.000000000 +0200 @@ -2876,3 +2876,250 @@ # The field should be None after init for a fresh app app2 = ReviewApp(self._make_session()) assert app2._selected_followup_msgid is None + + + +# --------------------------------------------------------------------------- +# _get_lore_series version-mismatch tests (cc529aa) +# --------------------------------------------------------------------------- + +_MINIMAL_DIFF = """\ +Fix bar. + +Signed-off-by: Author <[email protected]> +--- + foo.c | 1 + + 1 file changed, 1 insertion(+) + +diff --git a/foo.c b/foo.c +index aaa..bbb 100644 +--- a/foo.c ++++ b/foo.c +@@ -1,3 +1,4 @@ + void foo(void) { ++ bar(); + } +""" + + +def _make_patch_msg(subject: str, from_addr: str, date: str, + body: str = '', msgid: str = '') -> email.message.EmailMessage: + """Build a minimal EmailMessage that LoreMailbox can parse as a patch.""" + msg = email.message.EmailMessage() + msg['Subject'] = subject + msg['From'] = from_addr + msg['Date'] = date + msg['Message-Id'] = msgid or f'<{abs(hash(subject + date))}@test.com>' + msg.set_payload(body or _MINIMAL_DIFF) + return msg + + +_AUTHOR = 'Author <[email protected]>' + + +class TestGetLoreSeriesVersionMismatch: + """Regression tests for the crash when the stored message-id points + to a different version's thread than the wanted revision. + + See bug cc529aa: b4 review crashes updating a series. + """ + + @staticmethod + def _v2_msgs() -> List[email.message.EmailMessage]: + return [ + _make_patch_msg( + '[PATCH v2] foo: fix bar', + _AUTHOR, + 'Thu, 19 Mar 2026 08:51:12 +0530', + msgid='<[email protected]>', + ), + ] + + @staticmethod + def _v3_msgs() -> List[email.message.EmailMessage]: + return [ + _make_patch_msg( + '[PATCH v3] foo: fix bar', + _AUTHOR, + 'Fri, 27 Mar 2026 14:51:06 +0530', + msgid='<[email protected]>', + ), + ] + + def test_correct_version_found(self) -> None: + """Requesting the version present in messages works.""" + msgs = self._v2_msgs() + lser = review._get_lore_series(msgs, wantver=2) + assert lser.revision == 2 + + def test_no_preference_picks_highest(self) -> None: + """wantver=None selects the highest available version.""" + msgs = self._v2_msgs() + self._v3_msgs() + lser = review._get_lore_series(msgs, wantver=None) + assert lser.revision == 3 + + def test_version_mismatch_shows_found(self) -> None: + """Error message lists which versions were actually found.""" + msgs = self._v2_msgs() + with pytest.raises(LookupError, match=r'found: v2'): + review._get_lore_series(msgs, wantver=3) + + def test_version_mismatch_after_extra_series(self) -> None: + """Adding the missing version's messages resolves the mismatch.""" + # Start with only v2 — requesting v3 fails + msgs = list(self._v2_msgs()) + with pytest.raises(LookupError): + review._get_lore_series(msgs, wantver=3) + + # Simulating get_extra_series adding v3 messages + msgs.extend(self._v3_msgs()) + lser = review._get_lore_series(msgs, wantver=3) + assert lser.revision == 3 + + def test_no_series_in_messages(self) -> None: + """Completely empty mailbox raises LookupError.""" + with pytest.raises(LookupError, match='No series found'): + review._get_lore_series([], wantver=1) + + +# -- Tests for collect_review_emails() ---------------------------------------- + +class TestCollectReviewEmails: + """Tests for collect_review_emails() filtering logic. + + Covers the sent-revision safety-net filter added to fix the bug where + reviews carried over from a prior revision were re-sent for the new one. + """ + + MY_EMAIL = '[email protected]' + + @staticmethod + def _make_series(reviews: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: + return { + 'revision': 1, + 'header-info': { + 'msgid': '[email protected]', + 'to': '', + 'cc': '', + 'references': '', + 'sentdate': '', + }, + 'reviews': reviews or {}, + } + + @staticmethod + def _make_patch(reviews: Optional[Dict[str, Any]] = None) -> Dict[str, Any]: + return { + 'header-info': {'msgid': '[email protected]'}, + 'reviews': reviews or {}, + } + + @staticmethod + def _review(**extra: Any) -> Dict[str, Any]: + r: Dict[str, Any] = { + 'name': 'Maintainer', + 'trailers': ['Reviewed-by: Maintainer <[email protected]>'], + } + r.update(extra) + return r + + # Use a sentinel email message so we can count how many were produced. + _FAKE_MSG = mock.sentinel.email_msg + + @mock.patch('b4.review._review._build_review_email', + return_value=_FAKE_MSG) + @mock.patch('b4.get_user_config', + return_value={'name': 'Maintainer', 'email': MY_EMAIL}) + def test_sends_normal_cover_review(self, _cfg: mock.Mock, + _build: mock.Mock) -> None: + """A cover review without sent-revision produces one email.""" + series = self._make_series({self.MY_EMAIL: self._review()}) + msgs = review.collect_review_emails(series, [], 'cover', '', []) + assert len(msgs) == 1 + + @mock.patch('b4.review._review._build_review_email', + return_value=_FAKE_MSG) + @mock.patch('b4.get_user_config', + return_value={'name': 'Maintainer', 'email': MY_EMAIL}) + def test_skips_cover_with_sent_revision(self, _cfg: mock.Mock, + _build: mock.Mock) -> None: + """Cover review stamped with sent-revision is not re-sent.""" + series = self._make_series( + {self.MY_EMAIL: self._review(**{'sent-revision': 1})}) + msgs = review.collect_review_emails(series, [], 'cover', '', []) + assert msgs == [] + + @mock.patch('b4.review._review._build_review_email', + return_value=_FAKE_MSG) + @mock.patch('b4.get_user_config', + return_value={'name': 'Maintainer', 'email': MY_EMAIL}) + def test_sends_normal_patch_review(self, _cfg: mock.Mock, + _build: mock.Mock) -> None: + """A patch review without sent-revision produces one email.""" + series = self._make_series() + patch = self._make_patch({self.MY_EMAIL: self._review()}) + msgs = review.collect_review_emails(series, [patch], 'cover', '', ['sha1']) + assert len(msgs) == 1 + + @mock.patch('b4.review._review._build_review_email', + return_value=_FAKE_MSG) + @mock.patch('b4.get_user_config', + return_value={'name': 'Maintainer', 'email': MY_EMAIL}) + def test_skips_patch_with_sent_revision(self, _cfg: mock.Mock, + _build: mock.Mock) -> None: + """Patch review stamped with sent-revision is not re-sent.""" + series = self._make_series() + patch = self._make_patch( + {self.MY_EMAIL: self._review(**{'sent-revision': 1})}) + msgs = review.collect_review_emails(series, [patch], 'cover', '', ['sha1']) + assert msgs == [] + + @mock.patch('b4.review._review._build_review_email', + return_value=_FAKE_MSG) + @mock.patch('b4.get_user_config', + return_value={'name': 'Maintainer', 'email': MY_EMAIL}) + def test_skips_patch_auto_skipped_after_upgrade(self, _cfg: mock.Mock, + _build: mock.Mock) -> None: + """Patch auto-marked skip+skip-reason during upgrade is not re-sent. + + This is the combo A+B fix: the upgrade step sets patch-state=skip + AND skip-reason on unchanged patches whose review was already sent. + Both the skip filter and the sent-revision filter independently + prevent re-sending; this test exercises the skip-state path. + """ + series = self._make_series() + patch = self._make_patch({self.MY_EMAIL: self._review( + **{'sent-revision': 1, + 'patch-state': 'skip', + 'skip-reason': 'Patch unchanged from v1; review already sent'})}) + msgs = review.collect_review_emails(series, [patch], 'cover', '', ['sha1']) + assert msgs == [] + + @mock.patch('b4.review._review._build_review_email', + return_value=_FAKE_MSG) + @mock.patch('b4.get_user_config', + return_value={'name': 'Maintainer', 'email': MY_EMAIL}) + def test_only_unsent_patches_included(self, _cfg: mock.Mock, + _build: mock.Mock) -> None: + """Mix of sent and unsent patches: only unsent ones produce emails.""" + series = self._make_series() + sent_patch = self._make_patch( + {self.MY_EMAIL: self._review(**{'sent-revision': 1})}) + fresh_patch = self._make_patch( + {self.MY_EMAIL: self._review()}) + msgs = review.collect_review_emails( + series, [sent_patch, fresh_patch], 'cover', '', ['sha1', 'sha2']) + assert len(msgs) == 1 + + @mock.patch('b4.review._review._build_review_email', + return_value=_FAKE_MSG) + @mock.patch('b4.get_user_config', + return_value={'name': 'Maintainer', 'email': MY_EMAIL}) + def test_skip_state_without_sent_revision_still_skipped( + self, _cfg: mock.Mock, _build: mock.Mock) -> None: + """Explicit skip state (manually set, no sent-revision) is honoured.""" + series = self._make_series() + patch = self._make_patch( + {self.MY_EMAIL: self._review(**{'patch-state': 'skip'})}) + msgs = review.collect_review_emails(series, [patch], 'cover', '', ['sha1']) + assert msgs == [] diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/b4-0.15.1/src/tests/test_tui_tracking.py new/b4-0.15.2/src/tests/test_tui_tracking.py --- old/b4-0.15.1/src/tests/test_tui_tracking.py 2026-03-24 15:09:05.000000000 +0100 +++ new/b4-0.15.2/src/tests/test_tui_tracking.py 2026-04-09 20:43:04.000000000 +0200 @@ -2106,7 +2106,7 @@ identifier = 'test-update-fail' change_id = 'update-fail-1' review_branch = _setup_update_test(gitdir, identifier, change_id) - upgrade_branch = f'b4/review/{change_id}-v2-upgrade' + upgrade_branch = f'b4/review/_tmp-{change_id}-v2-upgrade' # Snapshot old branch HEAD before the attempt ecode, old_head = b4.git_run_command( @@ -2157,7 +2157,7 @@ identifier = 'test-update-abort' change_id = 'update-abort-1' review_branch = _setup_update_test(gitdir, identifier, change_id) - upgrade_branch = f'b4/review/{change_id}-v2-upgrade' + upgrade_branch = f'b4/review/_tmp-{change_id}-v2-upgrade' ecode, old_head = b4.git_run_command( gitdir, ['rev-parse', review_branch]) @@ -2214,7 +2214,8 @@ identifier: Optional[str] = None, status: str = 'reviewing') -> None: """Simulate create_review_branch by making a real branch.""" - _create_review_branch(topdir, change_id + '-v2-upgrade', + branch_suffix = branch.removeprefix('b4/review/') + _create_review_branch(topdir, branch_suffix, identifier=identifier or 'test', revision=2, status='reviewing') @@ -2255,7 +2256,7 @@ # Upgrade branch should be gone (was renamed) assert not b4.git_branch_exists( - gitdir, f'b4/review/{change_id}-v2-upgrade') + gitdir, f'b4/review/_tmp-{change_id}-v2-upgrade') # Upgrade branch should have been renamed to review branch assert b4.git_branch_exists(gitdir, review_branch) @@ -2280,13 +2281,14 @@ identifier = 'test-update-archfail' change_id = 'update-archfail-1' review_branch = _setup_update_test(gitdir, identifier, change_id) - upgrade_branch = f'b4/review/{change_id}-v2-upgrade' + upgrade_branch = f'b4/review/_tmp-{change_id}-v2-upgrade' lser = _make_mock_lser() def _fake_create(topdir: str, branch: str, *args: Any, **kwargs: Any) -> None: - _create_review_branch(topdir, change_id + '-v2-upgrade', + branch_suffix = branch.removeprefix('b4/review/') + _create_review_branch(topdir, branch_suffix, identifier=identifier, revision=2, status='reviewing') ++++++ build.specials.obscpio ++++++ ++++++ build.specials.obscpio ++++++ diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/.gitignore new/.gitignore --- old/.gitignore 1970-01-01 01:00:00.000000000 +0100 +++ new/.gitignore 2026-04-10 08:31:40.000000000 +0200 @@ -0,0 +1 @@ +.osc
