Hi Warren
On 08/09/2019 00:44, Warren He wrote:
Everyone in this thread, thanks for your support and encouragement.
[...]
It should not really imply `--interactive`, but `--rebase-merges`.
`imply_interactive` doesn't fully switch on `--interactive`, i.e., causing the
editor to open. It only selects the backend, which I think we're saying is the
right thing. I've dropped the `-i` from the test description.
And we don't really have to imply --rebase-merges, in case someone would prefer
to linearize things, which who knows? Running that non-rebase-merges command in
the example scenario from my original post should give something like this:
I think it would probably be less confusing to default to preserving
merges and having an option to turn that off - people are going to be
surprised if their history is linearized.
```
A - B (master)
\
F (feat-f)
\
E (feat-e)
\
H (my-dev)
```
So for now I haven't moved the implementation into `make_script_with_merges`.
> [...]>
[handling `fixup`/`squash` chains]
I've moved `todo_list_add_branch_updates` to run before
`todo_list_rearrange_squash`. The rearranging pulls fixups out, causing the
branch update to "fall" onto the items before, and reinserts them between a
commit and its branch update, casing them to be included in the updated branch.
which is my opinion of the right thing to do. I've added a test about this with
the following scenario:
```
A - B (master)
\
I - J - fixup! I (fixup-early)
\
K - fixup! J (fixup-late)
```
which results in the following todo list with `--autosquash`:
```
pick 9eadc32 I
fixup 265fa32 fixup! I
pick a0754fc J
fixup e7d1999 fixup! J
exec git branch -f fixup-early
pick c8bc4af K
```
That makes sense I think
I'd like to suggest [`test_cmp_rev`] instead
I've updated the test to use `test_cmp_rev`. It's not with your suggested
invocation though. We don't update the `C` tag. I've referred to the rebased `C`
with `test_cmp_rev linear-early HEAD^` and similar for the other checks.
That sounds right
* * *
And then there's the discussion about using `exec git branch -f`. To summarize
the issues collected from the entire thread:
1. the changes aren't atomically applied at the end of the rebase
2. it fails when the branch is checked out in a worktree
3. it clobbers the branch if anything else updates it during the rebase
4. the way we prepare the unprefixed branch doesn't work right some exotic cases
5. the reflog message it leaves is uninformative
For #4, I think we've lucked out actually. The `load_ref_decorations` routine we
use determines that a ref is `DECORATION_REF_LOCAL` under the condition
`starts_with(refname, "refs/heads/")` (log-tree.c:114, add_ref_decoration), so
`prettify_refname` will find the prefix and skip it. But that's an invariant
maintained by two pieces of code pretty far away from each other.
For #5, for the convenience of readers, the reflog entry it leaves looks like
this:
```
00873f2 feat-e@{0}: branch: Reset to HEAD
```
Not great.
I haven't made any changes to this yet, but I've thought about what I want. My
favorite so far is to add a new todo command that just does everything right. It
would make a temparary ref `refs/rewritten-heads/xxx` (or something), and update
`refs/heads/xxx` at the end.
I think that's the best way to do it. If we had a command like 'branch
<branch-name>' that creates a ref to remember the current HEAD and saves
the current branch head. Then at the end rebase can update the branches
to point to the saved commits if the branch is unchanged. If the rebase
is aborted then we don't end up with some branches updated and others not.
Side Note
I'd avoid creating another worktree local ref refs/rewritten-heads/.
Either store them under refs/rewritten/ or refs/worktree/
Best Wishes
Phillip
I agree that requiring a separate update-ref step at the end of the todo list is
unfriendly. Manually putting in some branch update commands and then realizing
that they weren't applied would be extremely frustrating. I don't see the option
of using existing tools as the easiest-to-use solution.
I'm reluctant to combine this with the existing `label` command. So far it
sounds like we generally want to be more willing to skip branch updates while
performing the rebase, with aforementioned scenarios where something else
updates the branch before we do, or if the branch becomes checked out in a
worktree. We don't want to mess up the structure of a `rebase -r` as a result of
skipping some branch updates. I think it would be conceptually simpler and
implementation-wise less tricky if we didn't combine it with the `label` and
`reset` system.
Warren He (1):
rebase: introduce --update-branches option
Documentation/git-rebase.txt | 9 ++++
builtin/rebase.c | 11 ++++-
sequencer.c | 58 +++++++++++++++++++++++-
sequencer.h | 6 ++-
t/t3431-rebase-update-branches.sh | 94 +++++++++++++++++++++++++++++++++++++++
5 files changed, 173 insertions(+), 5 deletions(-)
create mode 100755 t/t3431-rebase-update-branches.sh
