Skip to content

Commit

Permalink
cli rebase: fix an assumption in a comment and add a TODO
Browse files Browse the repository at this point in the history
It In Git, a commit's direct parent is allowed to also be an indirect ancestor
at the same time. `jj` currently tries to prevent this situation, but does
allow it. The correctness of `rebase -r A -d descendant_of_A` currently depends
on this jj-specific behavior; we should change that.

Cc jj-vcs#2600

This is an independent problem from the one the parent commit describes,
though it affects the same piece of code.
  • Loading branch information
ilyagr committed Nov 26, 2023
1 parent 3967f63 commit 5a64c4c
Showing 1 changed file with 9 additions and 5 deletions.
14 changes: 9 additions & 5 deletions cli/src/commands/rebase.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,14 +369,18 @@ fn rebase_revision(
let num_rebased_descendants = rebased_commit_ids.len();

// We now update `new_parents` to account for the rebase of all of
// `old_commit`'s descendants. Even if some of the original `new_parents` were
// descendants of `old_commit`, this will no longer be the case after the
// update.
// `old_commit`'s descendants. Even if some of the original `new_parents`
// were descendants of `old_commit`, this will no longer be the case after
// the update.
//
// To make the update simpler, we assume that each commit was rewritten only
// once; we don't have a situation where both `(A,B)` and `(B,C)` are in
// `rebased_commit_ids`. This assumption relies on the fact that a descendant of
// a child of `old_commit` cannot also be a direct child of `old_commit`.
// `rebased_commit_ids`.
// TODO(ilyagr): This assumption relies on the fact that, after
// `rebase_descendants`, a child of `old_commit` cannot also be a direct
// child of `old_commit`. This will likely change, see
// https://github.com/martinvonz/jj/issues/2600. So, the code needs to be
// updated before that happens.
let new_parents: Vec<_> = new_parents
.iter()
.map(|new_parent| {
Expand Down

0 comments on commit 5a64c4c

Please sign in to comment.