History

fix(gix-tix): avoid pins during rebase continuation

Record externally amended conflict resolutions in the rewrite mapping and preserve that mapping across intermediate materialization. Use it for final continuation checkout so superseded resolution commits are recognized as rewritten instead of pinned as departures.

Codex committed 2 months ago

Commit f49faf11 parent fdd92c94 files · +83 −53Browse files

Unified · Split

4 files
  1. gix-tix/src/command/rebase.rs +26 −1
  2. gix-tix/src/edit/rebase.rs +14 −3
  3. gix-tix/src/edit/time_travel.rs +20 −30
  4. gix-tix/src/lib.rs +23 −19
gix-tix/src/command/rebase.rs+26−1
@@ -597,7 +597,7 @@ mod tests { · 596 unmodified lines
597 "the materialized conflict commit records the ours tree"597 "the materialized conflict commit records the ours tree"
598 );598 );
599 599
600 std::fs::write(fixture.path().join("file"), b"base\n")?;600 std::fs::write(fixture.path().join("file"), b"resolved first\n")?;
601 assert!(601 assert!(
602 Command::new("git")602 Command::new("git")
603 .arg("-C")603 .arg("-C")
@@ -606,10 +606,20 @@ mod tests { · 2 unmodified lines
606 .status()?606 .status()?
607 .success()607 .success()
608 );608 );
609 assert!(
610 Command::new("git")
611 .arg("-C")
612 .arg(fixture.path())
613 .args(["commit", "-q", "--amend", "--no-edit"])
614 .status()?
615 .success(),
616 "resolving through the CLI may amend the materialized conflict before continuing"
617 );
609 let repo = crate::test_repository::open_with(618 let repo = crate::test_repository::open_with(
610 fixture.path(),619 fixture.path(),
611 ["user.name=todo author", "user.email=todo@example.com"],620 ["user.name=todo author", "user.email=todo@example.com"],
612 )?;621 )?;
622 let first_resolution = repo.head_id()?.detach();
613 let next_output = output_dir.path().join("continue-again.md");623 let next_output = output_dir.path().join("continue-again.md");
614 let err = apply_document(repo, &continuation, Some(&next_output))624 let err = apply_document(repo, &continuation, Some(&next_output))
615 .expect_err("the descendant conflict stops the continuation");625 .expect_err("the descendant conflict stops the continuation");
@@ -617,6 +627,12 @@ mod tests { · 1 unmodified line
617 format!("{err:#}").contains("materialized conflict"),627 format!("{err:#}").contains("materialized conflict"),
618 "the second conflict is materialized: {err:#}"628 "the second conflict is materialized: {err:#}"
619 );629 );
630 assert!(
631 crate::history::all_pins(&crate::test_repository::open(fixture.path())?)?
632 .iter()
633 .all(|pin| pin.id != first_resolution),
634 "continuing does not pin the superseded CLI resolution"
635 );
620 std::fs::write(fixture.path().join("file"), b"resolved again\n")?;636 std::fs::write(fixture.path().join("file"), b"resolved again\n")?;
621 assert!(637 assert!(
622 Command::new("git")638 Command::new("git")
@@ -626,6 +642,15 @@ mod tests { · 3 unmodified lines
626 .status()?642 .status()?
627 .success()643 .success()
628 );644 );
645 assert!(
646 Command::new("git")
647 .arg("-C")
648 .arg(fixture.path())
649 .args(["commit", "-q", "--amend", "--no-edit"])
650 .status()?
651 .success(),
652 "the final conflict may also be amended before continuing"
653 );
629 let repo = crate::test_repository::open_with(654 let repo = crate::test_repository::open_with(
630 fixture.path(),655 fixture.path(),
631 ["user.name=todo author", "user.email=todo@example.com"],656 ["user.name=todo author", "user.email=todo@example.com"],
gix-tix/src/edit/rebase.rs+14−3
@@ -1585,6 +1585,7 @@ pub(crate) fn perform_plan_with_progress( · 1584 unmodified lines
1585 let mut conflict = None;1585 let mut conflict = None;
1586 let mut marked = false;1586 let mut marked = false;
1587 for (index, step) in plan.steps.iter().enumerate() {1587 for (index, step) in plan.steps.iter().enumerate() {
1588 let mut resolved_head = None;
1588 let parent = match step.parent {1589 let parent = match step.parent {
1589 PlanParent::Existing(id) => {1590 PlanParent::Existing(id) => {
1590 repo.find_commit(id).context("could not find a fork target")?;1591 repo.find_commit(id).context("could not find a fork target")?;
@@ -1602,11 +1603,13 @@ pub(crate) fn perform_plan_with_progress( · 11 unmodified lines
1602 .context("could not decode a picked commit")?1603 .context("could not decode a picked commit")?
1603 .into_owned()1604 .into_owned()
1604 .context("could not own a picked commit")?,1605 .context("could not own a picked commit")?,
1605 PlanCommit::Resolved(_) => {1606 PlanCommit::Resolved(planned) => {
1606 let mut commit = repo1607 let head = repo
1607 .head()?1608 .head()?
1608 .peel_to_commit()1609 .peel_to_commit()
1609 .context("could not resolve the conflicted HEAD commit")?1610 .context("could not resolve the conflicted HEAD commit")?;
1611 resolved_head = Some((*planned, head.id));
1612 let mut commit = head
1610 .decode()1613 .decode()
1611 .context("could not decode the conflicted HEAD commit")?1614 .context("could not decode the conflicted HEAD commit")?
1612 .into_owned()1615 .into_owned()
@@ -1778,6 +1781,14 @@ pub(crate) fn perform_plan_with_progress( · 165 unmodified lines
1778 {1781 {
1779 note_rewrites.push((old_id, new_id));1782 note_rewrites.push((old_id, new_id));
1780 }1783 }
1784 if let Some((planned, actual)) = resolved_head
1785 && actual != planned
1786 {
1787 rewritten.insert(actual, Some(new_id));
1788 if actual != new_id {
1789 note_rewrites.push((actual, new_id));
1790 }
1791 }
1781 for old_id in step.squash.iter().take(applied_squash) {1792 for old_id in step.squash.iter().take(applied_squash) {
1782 rewritten.insert(*old_id, Some(new_id));1793 rewritten.insert(*old_id, Some(new_id));
1783 if *old_id != new_id {1794 if *old_id != new_id {
gix-tix/src/edit/time_travel.rs+20−30
@@ -143,6 +143,12 @@ pub(crate) fn materialize_plan_conflict_reporting( · 142 unmodified lines
143 Vec<super::undo::RefChange>,143 Vec<super::undo::RefChange>,
144)> {144)> {
145 let original = conflict.original();145 let original = conflict.original();
146 let mapped_head = conflict
147 .repository()
148 .head()?
149 .id()
150 .map(gix::Id::detach)
151 .map(|id| (id, conflict.map(id)));
146 let mut conflict = conflict.into_conflict().persist()?;152 let mut conflict = conflict.into_conflict().persist()?;
147 let mut ref_changes = std::mem::take(&mut conflict.ref_changes);153 let mut ref_changes = std::mem::take(&mut conflict.ref_changes);
148 let (notice, mut checkout_changes) = move_head_to_reporting(154 let (notice, mut checkout_changes) = move_head_to_reporting(
@@ -152,7 +158,10 @@ pub(crate) fn materialize_plan_conflict_reporting( · 3 unmodified lines
152 None,158 None,
153 revisions,159 revisions,
154 include_worktrees,160 include_worktrees,
155 |id| conflict.map(id),161 |id| match mapped_head {
162 Some((head, mapped)) if head == id => mapped,
163 _ => conflict.map(id),
164 },
156 )?;165 )?;
157 ref_changes.append(&mut checkout_changes);166 ref_changes.append(&mut checkout_changes);
158 let mut deletion_changes = delete_deferred_refs(repository_path, bare, &conflict.deferred_ref_deletions)?;167 let mut deletion_changes = delete_deferred_refs(repository_path, bare, &conflict.deferred_ref_deletions)?;
@@ -178,17 +187,16 @@ pub(crate) fn checkout_without_replay( · 19 unmodified lines
178 revisions: &[OsString],187 revisions: &[OsString],
179 include_worktrees: bool,188 include_worktrees: bool,
180) -> Result<Option<String>> {189) -> Result<Option<String>> {
181 Ok(checkout_without_replay_reporting(repository_path, bare, selected, revisions, include_worktrees)?.0)190 Ok(move_head_to_reporting(
182}191 repository_path,
183 192 bare,
184pub(crate) fn checkout_without_replay_reporting(193 selected,
185 repository_path: &Path,194 None,
186 bare: bool,195 revisions,
187 selected: ObjectId,196 include_worktrees,
188 revisions: &[OsString],197 Some,
189 include_worktrees: bool,198 )?
190) -> Result<(Option<String>, Vec<super::undo::RefChange>)> {199 .0)
191 move_head(repository_path, bare, selected, revisions, include_worktrees)
192}200}
193 201
194#[cfg(test)]202#[cfg(test)]
@@ -278,24 +286,6 @@ pub(crate) fn checkout_plan_reporting( · 83 unmodified lines
278 Ok((notice, ref_changes))286 Ok((notice, ref_changes))
279}287}
280 288
281fn move_head(
282 repository_path: &Path,
283 bare: bool,
284 selected: ObjectId,
285 revisions: &[OsString],
286 include_worktrees: bool,
287) -> Result<(Option<String>, Vec<super::undo::RefChange>)> {
288 move_head_to_reporting(
289 repository_path,
290 bare,
291 selected,
292 None,
293 revisions,
294 include_worktrees,
295 Some,
296 )
297}
298
299#[cfg(test)]289#[cfg(test)]
300fn move_head_to<F>(290fn move_head_to<F>(
301 repository_path: &Path,291 repository_path: &Path,
gix-tix/src/lib.rs+23−19
@@ -2087,30 +2087,30 @@ fn event_loop( · 2086 unmodified lines
2087 })();2087 })();
2088 match result {2088 match result {
2089 Ok(edit::rebase::PlanPerform::Complete(outcome)) => {2089 Ok(edit::rebase::PlanPerform::Complete(outcome)) => {
2090 let notice = outcome2090 let checkout = if outcome.selected.is_some() {
2091 .selected2091 edit::time_travel::checkout_plan_reporting(
2092 .map(|selected| {2092 &repository_path,
2093 edit::time_travel::checkout_without_replay_reporting(2093 repository_is_bare,
2094 &repository_path,2094 &outcome,
2095 repository_is_bare,2095 &revisions,
2096 selected,2096 false,
2097 &revisions,2097 )
2098 false,2098 } else {
2099 )2099 Ok((None, outcome.ref_changes.clone()))
2100 })2100 };
2101 .transpose()
2102 .map(|result| result.unwrap_or((None, Vec::new())));
2103 app.clear_rebase_conflict();2101 app.clear_rebase_conflict();
2104 app.clear_rebase_continuation();2102 app.clear_rebase_continuation();
2105 app.set_worktree_conflicted(false);2103 app.set_worktree_conflicted(false);
2106 let mut changes = std::mem::take(&mut pending_todo_ref_changes);2104 let mut changes = std::mem::take(&mut pending_todo_ref_changes);
2107 changes.extend(outcome.ref_changes.iter().cloned());2105 let message = match checkout {
2108 let message = match notice {2106 Ok((notice, mut outcome_changes)) => {
2109 Ok((notice, mut checkout_changes)) => {2107 changes.append(&mut outcome_changes);
2110 changes.append(&mut checkout_changes);
2111 notice.unwrap_or_else(|| "rebased history".into())2108 notice.unwrap_or_else(|| "rebased history".into())
2112 }2109 }
2113 Err(err) => format!("rebase applied, checkout failed: {err:#}"),2110 Err(err) => {
2111 changes.extend(outcome.ref_changes.iter().cloned());
2112 format!("rebase applied, checkout failed: {err:#}")
2113 }
2114 };2114 };
2115 leave_recorded_success(2115 leave_recorded_success(
2116 &mut app,2116 &mut app,
@@ -8694,7 +8694,7 @@ mod tests { · 6577 unmodified lines
8694 let graph = HistoryGraph::for_commits(&repository, &plan.scope)?;8694 let graph = HistoryGraph::for_commits(&repository, &plan.scope)?;
8695 let outcome = edit::rebase::perform_plan(&repository, &graph, plan)?.complete()?;8695 let outcome = edit::rebase::perform_plan(&repository, &graph, plan)?.complete()?;
8696 let resolved = outcome.map(head).context("the conflicted commit is retained")?;8696 let resolved = outcome.map(head).context("the conflicted commit is retained")?;
8697 edit::time_travel::checkout_without_replay(fixture.path(), false, resolved, &[], false)?;8697 edit::time_travel::checkout_plan(fixture.path(), false, &outcome, &[], false)?;
8698 8698
8699 let repository = test_repository::open(fixture.path())?;8699 let repository = test_repository::open(fixture.path())?;
8700 assert_eq!(8700 assert_eq!(
@@ -8730,6 +8730,10 @@ mod tests { · 29 unmodified lines
8730 .all(|entry| entry.stage() == gix::index::entry::Stage::Unconflicted),8730 .all(|entry| entry.stage() == gix::index::entry::Stage::Unconflicted),
8731 "the final index has no conflict stages"8731 "the final index has no conflict stages"
8732 );8732 );
8733 assert!(
8734 history::all_pins(&repository)?.iter().all(history::Pin::is_head),
8735 "continuing a resolved plan creates no ordinary pin for its superseded conflict checkout"
8736 );
8733 assert!(8737 assert!(
8734 !git(&["ls-files", "--error-unmatch", "unrelated"])?.status.success(),8738 !git(&["ls-files", "--error-unmatch", "unrelated"])?.status.success(),
8735 "unrelated unstaged paths remain untouched"8739 "unrelated unstaged paths remain untouched"

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t