feat(move): Support on-disk rebases with --fixup

[claytonrcarter]

Follow up to #545

[claytonrcarter]

The fiddly, hacky patch is in the last commit in this stack, and the test that it's patching is in its parent. To observe the issue:

❯ git switch move-fixup-on-disk
❯ cargo test test_move_fixup_multiple_disconnected_into_ancestor
# passes!
❯ git prev
❯ cargo test test_move_fixup_multiple_disconnected_into_ancestor
# fails with
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ Snapshot Summary ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
Snapshot: move_fixup_multiple_disconnected_into_ancestor-4
Source: git-branchless/tests/test_move.rs:5740
─────────────────────────────────────────────────────────────────────────────────────────────────────────────
Expression: stdout
─────────────────────────────────────────────────────────────────────────────────────────────────────────────
-old snapshot
+new results
────────────┬────────────────────────────────────────────────────────────────────────────────────────────────
    0     0 │ O f777ecc (master) create initial.txt␊
          1 │+|\␊
          2 │+| o 38caaaf create test.txt␊
          3 │+| |␊
          4 │+| o 6783c86 update 2 test.txt␊
          5 │+| |␊
          6 │+| o 7e2a64a update 4 test.txt␊
          7 │+| |␊
          8 │+| @ 472b70b update 6 test.txt␊
    1     9 │ |␊
    2       │-o 38caaaf (test) create test.txt␊
    3       │-|␊
    4       │-o 6783c86 update 2 test.txt␊
    5       │-|␊
    6       │-o 7e2a64a update 4 test.txt␊
    7       │-|␊
    8       │-@ 472b70b update 6 test.txt
         10 │+x ea32d54 (rewritten as 00000000) (test) create test.txt␊
────────────┴────────────────────────────────────────────────────────────────────────────────────────────────

In that snapshot, you can see that branch test should have been moved from ea32d54 to 38caaaf, but was not.

[arxanas]

If the issue does not regress existing functionality, then I would be okay to merge this anyways and resolve (or not resolve) the issue later.

EDIT: oh, I see, #545 was already landed and this is not a replacement for it, which is what I thought when skimming over the PR title.

[arxanas]

@claytonrcarter I was going to release a new version of git-branchless hopefully sometime in the next week. Let me know if you want me to wait to get this PR in.

[claytonrcarter]

@claytonrcarter I was going to release a new version of git-branchless hopefully sometime in the next week. Let me know if you want me to wait to get this PR in.

I'm definitely not going to get to this in the next week, so please don't wait for it.

If the issue does not regress existing functionality, then I would be okay to merge this anyways and resolve (or not resolve) the issue later.

I could go either way. There are a few known issues w/ this patch, but I'm not able to test it more at this time, and releasing it may be the best way to get more feedback. I don't think this is wrong or that it introduces defects; just that the whole on-disk fixup story feels a bit shakey still, what with all of the "manually hide every commit we generate" stuff, and now this latest patch. The on disk implementation feels like I'm abusing the rebase exec command, and leaves me feeling like I don't fully understand how branchless listens for and processes external changes.

Options, as I see them:

• leave this out, and we'll find out if anyone really even needs on-disk fixups 😄
• merge and release, and we'll find out if this really works in the wild and covers enough cases. We can always revert this PR and push a patch release.

In case it's helpful, the current status:

• move --fixup works fine for in-mem cases (that's feat(move): Add --fixup to squash moved commits into the destination #545)
• the first commit this PR works fine for most on-disk cases
• the last commit updates it to work when rebasing/squashing into a non-HEAD commit w/ a branch pointing at it
• the last commit depends on the git shell (or whatever shell runs the rebase commands) executing a for loop and manually resetting each branch ... which feels 🙄 and 😬

Any input is welcome, of course!

[claytonrcarter]

will not work for branches w/ spaces ... is that a thing?

No, it's not. At least not w/ git 2.41:

❯ git br 'foo bar'
fatal: 'foo bar' is not a valid branch name

❯ git br foo\tbar
fatal: 'foo	bar' is not a valid branch name

❯ git br foo\nbar
fatal: 'foo
bar' is not a valid branch name

[arxanas]

TIL