sync-upstream: Overhaul#341
Merged
real-or-random merged 2 commits intoBlockstreamResearch:masterfrom Mar 5, 2026
Merged
Conversation
Member
Author
|
@jonasnick Can you take a look? I think you wrote the initial version of the script including the select option. |
jonasnick
reviewed
Mar 5, 2026
| range "$@" | ||
|
|
||
| TITLE="Upstream PRs" | ||
| REPRODUCE_COMMAND="$0 -b $LOCAL_BRANCH range $RANGEEND_COMMIT" |
Contributor
There was a problem hiding this comment.
range should be removed here
I believe it was introduced to cherry-pick upstream PRs, but that simply doesn't work. Assume upstream is two PRs A and B ahead, and A has been merged before B. Then trying to cherry-picking B by merging the state of upstream's master after the merge-B commit won't do what we expect. In particular, the merge result will *include A's changes* because A had already been merged in upstream's master when B was merged. (One could think that merging the PR branch of B instead works, but this will yield the same result if B was rebased on master before it was merged.) The proper way to cherry-pick B is to create a PR that cherry-picks all commits that had been included in B. This could be done automatically, but the need to cherry-pick a PR is rare enough that we don't need tool support for it. In fact, because we want to keep cherry-picking at a minimum, there's a good chance that we'd anyway want to pick only a subset of the commits in a upstream PR, and that would need manual work anyway.
Roughly speaking, this changes (assuming 3 upstream PRs) git merge <upstream-commit1> <upstream-commit-2> <upstream-commit3> into git merge <upstream-commit3> This is more intuitive. We're merging a single upstream revision, namely <upstream-commit3>. The other two commits are simply parents of that one, i.e., they're included anyway, and git merge ignores them. (In fact, passing multiple refs looks like we're doing an octopus merge. It's just that git recognizes the fact that everything is included in the last ref anyway, and behaves as if only the last one had been passed.) This commit also makes some further clean ups and improvements.
real-or-random
force-pushed
the
202603-sync-script
branch
from
506b1bd to
656c7cc
Compare
Collaborator
|
ACK 656c7cc |
real-or-random
merged commit e363adc
into
BlockstreamResearch:master
106 of 122 checks passed
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
3 participants
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.
Please see individual commit messages for details.
Here's an example session that shows why
selectdoesn't make sense: