feat(series): Carry patch/ entries onto the buffer as part of porting - #2660
Open
krlmlr wants to merge 4 commits into
Open
feat(series): Carry patch/ entries onto the buffer as part of porting#2660krlmlr wants to merge 4 commits into
patch/ entries onto the buffer as part of porting#2660krlmlr wants to merge 4 commits into
Conversation
…the buffer `vendor-one.sh` applies the *buffer's* `patch/*.patch` to every tree it regenerates, so an entry that never reached `<S>-build` is one the next vendor run will not apply. Stage 3 of the series loop already says to commit an entry onto the buffer in the same firing, but that only binds the entries a firing writes: stage 4's port carries whole `main` commits onto `-dev`, `patch/` files included, so an entry born as a pull request against `main` reaches every `-dev` without any firing deciding to put it there, and reaches no buffer at all. Today that is five entries on `main`, `main-fwd`, `v1.5-variegata` and `v1.5-variegata-fwd` -- `0035`, `0036`, `0037`, `0038`, and `0003` under a name the buffers still carry in its older spelling. The drift is quiet while upstream leaves the patched files alone, because the buffer is then internally consistent and its commits carry no delta for them; it bites the first time upstream touches one, and the fix is reverted on `-dev` by a commit that reads as an ordinary vendor. Report it rather than close it: which entry belongs on which buffer, and what its effect on that buffer's tree is, is the repair stage's judgement. The line is printed in both directions, because a renamed entry is one of each and neither half alone says so.
Both were written on a series buffer, and neither is series-specific: they apply cleanly to `main`'s vendored tree today, which is the test the loop's stage 3 sets for routing a fix through `main` rather than keeping it on the series. Kept under the names the buffers already use, so the stacks match rather than growing a third spelling of the same diff. - `0034-Guard-explicit-producer-token-in-concurrent-queue`: `inner_enqueue()` and `inner_enqueue_bulk()` dereference the producer a token carries without checking it. - `0035-Silence-deprecated-Catalog-GetEntry-self-delegation`: the deprecated `Catalog::GetEntry()` overloads delegate to one another, which the check reports as a significant warning. Measured rather than assumed. Against every buffer and `-dev` of every series, each entry was test-applied, reverse-applied and neither: entry main main-* v1.5-* 0034-Guard-explicit... APPLY in APPLY 0035-Silence-deprecat... APPLY in APPLY `in` means the effect is already in that tree. The three entries born the same way that do *not* appear here -- `0036-Guard-assert-only-plan-verifiers`, `0037-Cast-to-void-in-the-default-aggregate-state-memset` and `0039-Return-past-the-exhaustive-DatePart-specifier-switches` -- do not apply to `main`'s engine at all, so they stay series-specific by engine version and reach `main` when `main`'s engine reaches the code they answer.
A `patch/` entry does nothing on `-dev`. `vendor-one.sh` applies the *buffer's*
stack to every tree it regenerates, so `<S>-build` is where an entry has to live
to affect what the series vendors next. Entries reach `-dev` two ways and only
one of them puts them there: a firing writing one during a repair is told by
stage 3 to commit it onto the buffer in the same firing, but stage 4's port
brings whole `main` commits over, `patch/` files included, so an entry born as a
pull request against `main` arrives without any firing deciding to put it there
-- and reaches no buffer at all, `-build` taking no ports by design.
So carry it where the port already runs, rather than detecting the gap
afterwards: `series-port.sh --apply` now finishes by calling the new
`series-patch-sync.sh <S> --apply`, which is a read-only report on its own.
`series-check.sh`'s PATCH DRIFT line stays as the backstop it should have been
from the start -- after a port it names what the carry deliberately declined.
What carries is decided by test-applying against the buffer's own tree, never by
the file list. A buffer runs ahead of `main`, so an entry `main` needs may not
fit the engine the buffer has vendored, and committing it there regardless
breaks the next vendor run outright rather than helping anything:
carry applies cleanly -- taken with its effect on the tree
satisfied reverse-applies, so the effect is already there -- entry taken
alone, to keep the two stacks identical
stale neither: upstream moved the code out from under it, so on this
engine the entry answers nothing
Two shapes are reported and never carried, because both are judgement and both
break the next vendor run if guessed at. A candidate whose files an entry the
buffer has and `-dev` lacks also touches is a *supersession*, not an addition --
`0003-Fix-clang-warnings-in-re2` displacing `0003-Try-to-ignore-clang-warnings`,
the real fix replacing the pragma that stood in for it. And an entry `main`
edited in place, which the port brings to `-dev` under a name the buffer already
has: candidates are therefore compared by blob, since a name-only comparison
calls those two stacks level. `0009-Remove-stderr-for-zstd` is that one today.
… the carry An entry `main` edits in place reaches `-dev` through the port under a name the buffer already has, so a name-only comparison calls the two stacks level while they differ -- `0009-Remove-stderr-for-zstd` today. The advice line now names `series-patch-sync.sh` rather than telling a reader to bring the buffer level by hand: after a port, what this reports is precisely what the carry declined.
-dev and not the bufferpatch/ entries onto the buffer as part of porting
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A
patch/entry does nothing on-dev.vendor-one.shapplies the buffer's stack to every tree it regenerates, so<S>-buildis where an entry has to live to affect what the series vendors next.Entries reach
-devtwo ways and only one of them puts them there. A firing writing one during a repair is told by stage 3 to commit it onto the buffer in the same firing — that half works. But stage 4's port brings wholemaincommits over,patch/files included, so an entry born as a pull request againstmainarrives on every-devwithout any firing deciding to put it there, and reaches no buffer at all,-buildtaking no ports by design.The drift is quiet while upstream leaves the patched file alone, because the buffer is then internally consistent and its commits carry no delta for that file. It bites the first time upstream touches one: the buffer regenerates that file without the fix, and the commit that reaches
-devfrom it reverts the fix while reading as an ordinary vendor commit.What this changes
Carry it where the port already runs, rather than detecting the gap afterwards.
series-port.sh --applynow finishes by calling the newseries-patch-sync.sh <S> --apply, which is a read-only report on its own.What carries is decided by test-applying against the buffer's own tree, never by the file list — a buffer runs ahead of
main, so an entrymainneeds may not fit the engine the buffer has vendored, and committing it there regardless breaks the next vendor run outright rather than helping anything:carry— applies cleanly, taken with its effect on the treesatisfied— reverse-applies, so the effect is already there; the entry is taken alone, to keep the two stacks identicalstale— neither: upstream moved the code out from under it, so on this engine the entry answers nothingTwo shapes are reported and never carried, because both are judgement and both break the next vendor run if guessed at:
-devlacks also touches.0003-Fix-clang-warnings-in-re2displacing0003-Try-to-ignore-clang-warningsis the standing example: the real fix, hoisting and naming the anonymous struct, replacing the two#pragma clang diagnostic ignoredlines that stood in for it.mainedited in place, which the port brings to-devunder a name the buffer already has. Candidates are therefore compared by blob; a name-only comparison calls those two stacks level.0009-Remove-stderr-for-zstdis that one today.series-check.shgains aPATCH DRIFTline per series, printed beside the verdict likeCUTOVERis, naming what the two stacks differ by in all three directions. It is the backstop, not the mechanism: after a port, what it reports is precisely what the carry declined, which is stage 3 work.Finally, two entries born on a series buffer are routed to
main, which is where the loop says a fix belongs when it is not series-specific:0034-Guard-explicit-producer-token-in-concurrent-queueand0035-Silence-deprecated-Catalog-GetEntry-self-delegation. Both apply cleanly tomain's vendored tree today, and both keep the names the buffers already use, so the stacks match rather than growing a third spelling of the same diff.Measured, not assumed
Every entry in the union of the stacks was test-applied, reverse-applied and neither, against every buffer and every
-dev. That is what separated the four cases above, and it caught one that a file-list carry would have got wrong:0036-Mark-two-assert-only-bindings-usedand0038-Initialize-every-member-of-DependencySubjectare onmain, and on themainseries' newer engine they fit nothing. Committing them to that buffer would have failed its next vendor run withvendor.sh's "patches moved" exit.The three entries born on a buffer that do not appear in the routing commit —
0036-Guard-assert-only-plan-verifiers,0037-Cast-to-void-in-the-default-aggregate-state-memsetand0039-Return-past-the-exhaustive-DatePart-specifier-switches— do not apply tomain's engine at all, so they stay series-specific by engine version and reachmainwhenmain's engine reaches the code they answer.Effect on the branches
Run against all six series, the carry closed the drift on the four that had it (
main,main-fwd,v1.5-variegata,v1.5-variegata-fwd; the twov1.4-andiumseries are frozen, take no ports, and were already level). The0003supersession was then resolved by hand as stage 3 work, reversing the entry being replaced and applying the replacement, withsrc/duckdb/third_party/re2verified byte-identical to each-devafterwards.What remains is
staleonly, and correctly so. No-devbranch was touched: buffers carry no CI, so closing all of this cost no check runs.🤖 Generated with Claude Code
https://claude.ai/code/session_01D36f3J68XGNUAu4o1DX5xD