git branchless crashes on empty commits

[samueltardieu]

Description of the bug

git branchless refuses to rewrite an empty commit and crashes:

$ git init; git branchless init --main-branch main
$ echo foo > foo
$ git add foo
$ git commit -am "First commit"
$ git commit --allow-empty -m "Second commit with no changes"
$ git reword HEAD^ -f -m "Fix message of first commit"  
The application panicked (crashed).
Message:  assertion failed: !dest_oids.is_empty()
Location: git-branchless-lib/src/core/rewrite/plan.rs:1023

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ SPANTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

   0: git_branchless_reword::reword with effects=<Output fancy=true> revsets=[Revset("HEAD^")] resolve_revset_options=ResolveRevsetOptions { show_hidden_commits: false } messages=Messages(["Fix message of first commit"]) git_run_info=<GitRunInfo path_to_git="git" working_directory="/tmp/z" env=not shown> force_rewrite_public_commits=true
      at git-branchless-reword/src/lib.rs:62

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.

Expected behavior

No error should happen. git branchless should acknowledge the existence of empty commits, or at least accept to rewrite empty commits as empty commits when restacking.

This is a behaviour I encountered when working with patch series created by b4: b4 creates an empty commit at the beginning of the patch series to track the cover letter and the changelog of the patch series.

While b4 + git-branchless could be the perfect combination for working with email-based git workflows, the systematic rejection of empty commits by git-branchless (even when an empty commit is being rewritten) makes it difficult to use.

Actual behavior

A crash happened

Version of rustc

No response

Automated bug report

Software version

git-branchless 0.8.0

Operating system

Linux 6.1.74

Command-line

/home/sam/.nix-profile/bin/git-branchless bug-report 

Environment variables

SHELL=/run/current-system/sw/bin/zsh
EDITOR=vim

Git version

> git version 
git version 2.43.0

Hooks

Show 7 hooks

Hook post-applypatch

#!/bin/sh
## START BRANCHLESS CONFIG

git branchless hook post-applypatch "$@"

## END BRANCHLESS CONFIG

Hook post-checkout

#!/bin/sh
## START BRANCHLESS CONFIG

git branchless hook post-checkout "$@"

## END BRANCHLESS CONFIG

Hook post-commit

#!/bin/sh
## START BRANCHLESS CONFIG

git branchless hook post-commit "$@"

## END BRANCHLESS CONFIG

Hook post-merge

#!/bin/sh
## START BRANCHLESS CONFIG

git branchless hook post-merge "$@"

## END BRANCHLESS CONFIG

Hook post-rewrite

#!/bin/sh
## START BRANCHLESS CONFIG

git branchless hook post-rewrite "$@"

## END BRANCHLESS CONFIG

Hook pre-auto-gc

#!/bin/sh
## START BRANCHLESS CONFIG

git branchless hook pre-auto-gc "$@"

## END BRANCHLESS CONFIG

Hook reference-transaction

#!/bin/sh
## START BRANCHLESS CONFIG

# Avoid canceling the reference transaction in the case that `branchless` fails
# for whatever reason.
git branchless hook reference-transaction "$@" || (
echo 'branchless: Failed to process reference transaction!'
echo 'branchless: Some events (e.g. branch updates) may have been lost.'
echo 'branchless: This is a bug. Please report it.'
)

## END BRANCHLESS CONFIG

Events

Show 5 events

Event ID: 7, transaction ID: 5 (message: reword)

1. RefUpdateEvent { timestamp: 1706552567.2591848, event_tx_id: EventTransactionId(5), ref_name: ReferenceName("refs/heads/main"), old_oid: 9202ffd46a48d38c42c8843eb6f22cb3dcc9f677, new_oid: ad2fd8941a2bb37f80c44b2ba7a25949a9489c7f, message: None }
2. RewriteEvent { timestamp: 1706552567.2686825, event_tx_id: EventTransactionId(5), old_commit_oid: 9202ffd46a48d38c42c8843eb6f22cb3dcc9f677, new_commit_oid: ad2fd8941a2bb37f80c44b2ba7a25949a9489c7f }
3. WorkingCopySnapshot { timestamp: 1706552567.2729092, event_tx_id: EventTransactionId(5), head_oid: 9202ffd46a48d38c42c8843eb6f22cb3dcc9f677, commit_oid: NonZeroOid(8dcb0ad411c163cc1eaeef0b84e5faf34e404812), ref_name: None }
4. RefUpdateEvent { timestamp: 1706552567.2799516, event_tx_id: EventTransactionId(5), ref_name: ReferenceName("HEAD"), old_oid: 9202ffd46a48d38c42c8843eb6f22cb3dcc9f677, new_oid: ad2fd8941a2bb37f80c44b2ba7a25949a9489c7f, message: None }

:
@ ad2fd89 4m (> main) xxxxxx xxxxxx xxxx xx xxxxxxx

Event ID: 6, transaction ID: 4 (message: post-commit)

1. CommitEvent { timestamp: 1706552548.0, event_tx_id: EventTransactionId(4), commit_oid: NonZeroOid(9202ffd46a48d38c42c8843eb6f22cb3dcc9f677) }

:
@ ad2fd89 4m (> main) xxxxxx xxxxxx xxxx xx xxxxxxx

Event ID: 4, transaction ID: 3 (message: reference-transaction)

1. RefUpdateEvent { timestamp: 1706552548.8607051, event_tx_id: EventTransactionId(3), ref_name: ReferenceName("HEAD"), old_oid: e12c3c1de973ed76b458946ac976eb0c28d30698, new_oid: 9202ffd46a48d38c42c8843eb6f22cb3dcc9f677, message: None }
2. RefUpdateEvent { timestamp: 1706552548.8607051, event_tx_id: EventTransactionId(3), ref_name: ReferenceName("refs/heads/main"), old_oid: e12c3c1de973ed76b458946ac976eb0c28d30698, new_oid: 9202ffd46a48d38c42c8843eb6f22cb3dcc9f677, message: None }

:
@ ad2fd89 4m (> main) xxxxxx xxxxxx xxxx xx xxxxxxx

Event ID: 3, transaction ID: 2 (message: post-commit)

1. CommitEvent { timestamp: 1706552541.0, event_tx_id: EventTransactionId(2), commit_oid: NonZeroOid(e12c3c1de973ed76b458946ac976eb0c28d30698) }

:
@ ad2fd89 4m (> main) xxxxxx xxxxxx xxxx xx xxxxxxx

Event ID: 1, transaction ID: 1 (message: reference-transaction)

1. RefUpdateEvent { timestamp: 1706552541.0822334, event_tx_id: EventTransactionId(1), ref_name: ReferenceName("HEAD"), old_oid: 0000000000000000000000000000000000000000, new_oid: e12c3c1de973ed76b458946ac976eb0c28d30698, message: None }
2. RefUpdateEvent { timestamp: 1706552541.0822334, event_tx_id: EventTransactionId(1), ref_name: ReferenceName("refs/heads/main"), old_oid: 0000000000000000000000000000000000000000, new_oid: e12c3c1de973ed76b458946ac976eb0c28d30698, message: None }

:
@ ad2fd89 4m (> main) xxxxxx xxxxxx xxxx xx xxxxxxx

Version of git-branchless

No response

Version of git

No response

[arxanas]

This behavior makes sense for rebases (in the case that a change was applied upstream), but not for rewords. It looks like it happens here for the in-memory code path:

git-branchless/git-branchless-lib/src/core/rewrite/execute.rs

Line 698 in f1c1aa5

.is_empty()

The on-disk rebase would have to use --keep-empty to accomplish the same thing.

We would add an option here to toggle it (probably skip_empty_commits):

git-branchless/git-branchless-lib/src/core/rewrite/execute.rs

Line 1195 in f1c1aa5

pub struct ExecuteRebasePlanOptions {

I don't plan to implement this myself but feel free to try it. You can also try https://github.com/martinvonz/jj as you can use it alongside Git (for b4) and it handles empty commits as part of its core workflow (I forget if you've already tried it).

[samueltardieu]

This behavior makes sense for rebases (in the case that a change was applied upstream), but not for rewords.

While it makes sense to delete commits which were not empty but become empty when rebasing (because the change was applied upstream), I think a commit which was purposefully empty to start with should be kept as-is, as its presence is there for a reason unrelated to the content.

I forget if you've already tried it

Yeah, I even contributed to it. I like jj a lot, especially the ability to move chunks from one commit to the other or the complex graph manipulation, but there are several things I like more about git-branchless for the time being:

• the fact that the branch tip advances as it does in git in presence of new commits
• the better integration with other tooling such as magit in Emacs and pre-commit
• b4 and git-publish also use git, and using jj requires the use of a colocated repository whose use is less natural than a jj-only repository or a git repository managed using git-branchless

[arxanas]

I think a commit which was purposefully empty to start with should be kept as-is,

That makes sense, and is also probably easier to implement. We would just modify the line linked in #1205 (comment), and adjust the on-disk rebase to not emit the action to check for empty commits, and it should work. (The on-disk rebase already passes the equivalent of --keep-empty to git rebase and handles empty commit detection itself.)