diff --git a/lore-revision/src/branch/push.rs b/lore-revision/src/branch/push.rs index 4ed08d9f..8244b125 100644 --- a/lore-revision/src/branch/push.rs +++ b/lore-revision/src/branch/push.rs @@ -1021,8 +1021,12 @@ async fn collect_fragments_and_push( let mut current_latest = Hash::default(); let mut fast_forward_merged = false; - for current_revision in full_local_history.iter().rev() { - let mut current_revision = *current_revision; + // The revision pushed in the previous iteration, as (signature in local + // history, signature on the server). The next revision may follow its + // parent only to a revision the server stored under a new signature. + let mut renumbered_previous: Option<(Hash, Hash)> = None; + for original_revision in full_local_history.iter().rev() { + let mut current_revision = *original_revision; let state = State::deserialize(repository.clone(), current_revision) .await @@ -1030,7 +1034,10 @@ async fn collect_fragments_and_push( push_revision_links(&repository, token, &options, &state, branch).await?; - if !current_latest.is_zero() && state.parent_self() != current_latest { + if let Some((local, stored)) = renumbered_previous + && local != stored + && state.parent_self() == local + { // Rebase on new latest revision // TODO(mjansson): This only handles revision number rewrite for now, implement proper // automatic rebase if the push resulted in a clean rebase @@ -1040,12 +1047,12 @@ async fn collect_fragments_and_push( LoreBranchPushRevisionUpdateBeginEventData { revision: state.revision(), old_parent: state.parent_self(), - new_parent: current_latest, + new_parent: stored, }, ) .send(); - state.set_parent_self(current_latest); + state.set_parent_self(stored); current_revision = state .serialize(repository.clone(), token) .await @@ -1157,6 +1164,8 @@ async fn collect_fragments_and_push( remote_latest = response.revision; current_latest = response.revision; + // The server stored what we pushed as the merge's other parent. + renumbered_previous = Some((*original_revision, current_revision)); event::LoreEvent::BranchPushRevisionPushEnd( LoreBranchPushRevisionPushEndEventData { @@ -1201,8 +1210,11 @@ async fn collect_fragments_and_push( remote_latest = response.revision; current_latest = response.revision; + // The server rewrote the revision number, which changes the signature. + renumbered_previous = Some((*original_revision, response.revision)); } else { current_latest = current_revision; + renumbered_previous = Some((*original_revision, current_revision)); } let current_number = State::deserialize(repository.clone(), current_latest) diff --git a/scripts/test/test_push.py b/scripts/test/test_push.py index 3b24f896..7976199b 100644 --- a/scripts/test/test_push.py +++ b/scripts/test/test_push.py @@ -368,6 +368,53 @@ def test_push_fast_forward_merge_non_merge(new_lore_repo): ) +@pytest.mark.smoke +def test_push_fast_forward_merge_two_local_revisions(new_lore_repo): + """A fast-forward merge push that carries more than one local revision keeps + the work another client pushed in between. The client may only follow a + parent the server renumbered, never a head the server moved for another + reason, or the second revision's stale tree replaces the branch.""" + repo: Lore = new_lore_repo() + + with repo.open_file("seed.txt", "w+") as f: + f.write("seed\n") + repo.stage(scan=True) + repo.commit("Seed", offline=True) + repo.push() + + # Another client moves the branch head + other = repo.clone() + with other.open_file("from_other.txt", "w+") as f: + f.write("from the other client\n") + other.stage(scan=True, offline=True) + other.commit("Other client publishes", offline=True) + other.push() + + # This client is still on the old head, and commits twice before it pushes + # once. The second revision is the one whose parent used to be bent onto the + # head the server had moved to, which kept its tree and dropped the other + # client's file. + with repo.open_file("first.txt", "w+") as f: + f.write("first\n") + repo.stage(scan=True, offline=True) + repo.commit("First", offline=True) + with repo.open_file("second.txt", "w+") as f: + f.write("second\n") + repo.stage(scan=True, offline=True) + repo.commit("Second", offline=True) + repo.push(fast_forward_merge=True) + + expected_files = {"seed.txt", "from_other.txt", "first.txt", "second.txt"} + + verify = repo.clone() + clone_files = _collect_repo_files(verify) + assert clone_files == expected_files, ( + f"Files on the server differ from expected.\n" + f" Extra: {clone_files - expected_files}\n" + f" Missing: {expected_files - clone_files}" + ) + + @pytest.mark.smoke def test_push_non_current_branch_preserves_anchor(new_lore_repo): """Pushing a branch that is not the current branch must not corrupt the