diff --git a/crates/but-core/src/diff/mod.rs b/crates/but-core/src/diff/mod.rs index 172f435474..3f6dc61742 100644 --- a/crates/but-core/src/diff/mod.rs +++ b/crates/but-core/src/diff/mod.rs @@ -61,15 +61,6 @@ impl TreeChange { } } -impl std::fmt::Debug for TreeChange { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("TreeChange") - .field("path", &self.path) - .field("status", &self.status) - .finish() - } -} - impl std::fmt::Debug for IgnoredWorktreeChange { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("IgnoredWorktreeChange") diff --git a/crates/but-core/src/diff/tree_changes.rs b/crates/but-core/src/diff/tree_changes.rs index d9704dbb76..7e2591f584 100644 --- a/crates/but-core/src/diff/tree_changes.rs +++ b/crates/but-core/src/diff/tree_changes.rs @@ -81,7 +81,6 @@ impl From for TreeChange { }, is_untracked: false, }, - status_item: None, }, Change::Deletion { location, @@ -96,7 +95,6 @@ impl From for TreeChange { kind: entry_mode.kind(), }, }, - status_item: None, }, Change::Modification { location, @@ -120,7 +118,6 @@ impl From for TreeChange { state, flags: ModeFlags::calculate(&previous_state, &state), }, - status_item: None, } } Change::Rewrite { @@ -150,7 +147,6 @@ impl From for TreeChange { state, flags: ModeFlags::calculate(&previous_state, &state), }, - status_item: None, } } } diff --git a/crates/but-core/src/diff/worktree.rs b/crates/but-core/src/diff/worktree.rs index b456b038a7..5803fb2df0 100644 --- a/crates/but-core/src/diff/worktree.rs +++ b/crates/but-core/src/diff/worktree.rs @@ -97,74 +97,102 @@ fn worktree_changes_inner( let work_dir = repo.workdir().context("need non-bare repository")?; let mut tmp = Vec::new(); let mut ignored_changes = Vec::new(); + let mut index_conflicts = Vec::new(); + let mut index_changes = Vec::new(); for change in status_changes { let change = change?; - let change = match change.clone() { + let change = match change { status::Item::TreeIndex(gix::diff::index::Change::Deletion { location, + index, id, entry_mode, - .. - }) => ( - Origin::TreeIndex, - TreeChange { - status: TreeStatus::Deletion { - previous_state: ChangeState { - id: id.into_owned(), - kind: into_tree_entry_kind(entry_mode)?, + }) => { + let res = ( + Origin::TreeIndex, + TreeChange { + status: TreeStatus::Deletion { + previous_state: ChangeState { + id: id.clone().into_owned(), + kind: into_tree_entry_kind(entry_mode)?, + }, }, + path: location.clone().into_owned(), }, - path: location.into_owned(), - status_item: Some(change), - }, - ), + ); + index_changes.push(gix::diff::index::Change::Deletion { + location, + index, + id, + entry_mode, + }); + res + } status::Item::TreeIndex(gix::diff::index::Change::Addition { location, + index, entry_mode, id, - .. - }) => ( - Origin::TreeIndex, - TreeChange { - path: location.into_owned(), - status: TreeStatus::Addition { - is_untracked: false, - state: ChangeState { - id: id.into_owned(), - kind: into_tree_entry_kind(entry_mode)?, + }) => { + let res = ( + Origin::TreeIndex, + TreeChange { + path: location.clone().into_owned(), + status: TreeStatus::Addition { + is_untracked: false, + state: ChangeState { + id: id.clone().into_owned(), + kind: into_tree_entry_kind(entry_mode)?, + }, }, }, - status_item: Some(change), - }, - ), + ); + index_changes.push(gix::diff::index::ChangeRef::Addition { + location, + index, + entry_mode, + id, + }); + res + } status::Item::TreeIndex(gix::diff::index::Change::Modification { location, + previous_index, previous_entry_mode, + index, entry_mode, previous_id, id, - .. }) => { let previous_state = ChangeState { - id: previous_id.into_owned(), + id: previous_id.clone().into_owned(), kind: into_tree_entry_kind(previous_entry_mode)?, }; let state = ChangeState { - id: id.into_owned(), + id: id.clone().into_owned(), kind: into_tree_entry_kind(entry_mode)?, }; - ( + let res = ( Origin::TreeIndex, TreeChange { - path: location.into_owned(), + path: location.clone().into_owned(), status: TreeStatus::Modification { previous_state, state, flags: ModeFlags::calculate(&previous_state, &state), }, - status_item: Some(change), }, - ) + ); + index_changes.push(gix::diff::index::Change::Modification { + location, + previous_index, + previous_entry_mode, + previous_id, + index, + entry_mode, + id, + }); + res } status::Item::IndexWorktree(index_worktree::Item::Modification { rela_path, @@ -181,7 +209,6 @@ fn worktree_changes_inner( kind: into_tree_entry_kind(entry.mode)?, }, }, - status_item: Some(change), }, ), status::Item::IndexWorktree(index_worktree::Item::Modification { @@ -208,7 +235,6 @@ fn worktree_changes_inner( state, flags: ModeFlags::calculate(&previous_state, &state), }, - status_item: Some(change), }, ) } @@ -245,7 +271,6 @@ fn worktree_changes_inner( state, flags: ModeFlags::calculate(&previous_state, &state), }, - status_item: Some(change), }, ) } @@ -267,7 +292,6 @@ fn worktree_changes_inner( }, is_untracked: false, }, - status_item: Some(change), }, ), status::Item::IndexWorktree(index_worktree::Item::DirectoryContents { @@ -300,7 +324,6 @@ fn worktree_changes_inner( }, is_untracked: true, }, - status_item: Some(change), }, ) } @@ -340,7 +363,6 @@ fn worktree_changes_inner( state, flags: ModeFlags::calculate(&previous_state, &state), }, - status_item: Some(change), }, ) } @@ -396,7 +418,6 @@ fn worktree_changes_inner( state, flags: ModeFlags::calculate(&previous_state, &state), }, - status_item: Some(change), }, ) } @@ -427,19 +448,18 @@ fn worktree_changes_inner( state, flags: ModeFlags::calculate(&previous_state, &state), }, - status_item: Some(change), }, ) } status::Item::IndexWorktree(index_worktree::Item::Modification { rela_path, - status: EntryStatus::Conflict { .. }, + status: EntryStatus::Conflict { entries, .. }, .. }) => { + index_conflicts.push((rela_path.clone(), entries)); ignored_changes.push(IgnoredWorktreeChange { path: rela_path, status: IgnoredWorktreeTreeChangeStatus::Conflict, - status_item: Some(change), }); continue; } @@ -508,17 +528,15 @@ fn worktree_changes_inner( changes.push(merged); IgnoredWorktreeTreeChangeStatus::TreeIndex } - [Some(mut first), Some(mut second)] => { + [Some(first), Some(second)] => { ignored_changes.push(IgnoredWorktreeChange { path: first.path.clone(), status: IgnoredWorktreeTreeChangeStatus::TreeIndex, - status_item: first.status_item.take(), }); changes.push(first); ignored_changes.push(IgnoredWorktreeChange { path: second.path.clone(), status: IgnoredWorktreeTreeChangeStatus::TreeIndex, - status_item: second.status_item.take(), }); changes.push(second); continue; @@ -527,7 +545,6 @@ fn worktree_changes_inner( ignored_changes.push(IgnoredWorktreeChange { path: change_path, status, - status_item: None, }); continue; } @@ -538,6 +555,8 @@ fn worktree_changes_inner( Ok(WorktreeChanges { changes, ignored_changes, + index_changes, + index_conflicts, }) } @@ -696,8 +715,6 @@ fn merge_changes( status: TreeStatus::Deletion { previous_state: *previous_state, }, - // NOTE: not relevant, as renames are disabled for snapshots where this is used. - status_item: None, })); } ( @@ -717,8 +734,6 @@ fn merge_changes( // It's just in the index, which to us doesn't exist. is_untracked: true, }, - // NOTE: not relevant, as renames are disabled for snapshots where this is used. - status_item: None, }), Some(TreeChange { path: index_wt.path, @@ -730,8 +745,6 @@ fn merge_changes( // read the initial state of a file from. is_untracked: true, }, - // NOTE: not relevant, as renames are disabled for snapshots where this is used. - status_item: None, }), ]); } @@ -935,3 +948,12 @@ impl TreeChange { } } } + +impl std::fmt::Debug for WorktreeChanges { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("WorktreeChanges") + .field("changes", &self.changes) + .field("ignored_changes", &self.ignored_changes) + .finish() + } +} diff --git a/crates/but-core/src/lib.rs b/crates/but-core/src/lib.rs index b477bc9272..469fe7a6cb 100644 --- a/crates/but-core/src/lib.rs +++ b/crates/but-core/src/lib.rs @@ -44,6 +44,7 @@ use bstr::BString; use gix::object::tree::EntryKind; use gix::refs::FullNameRef; +use gix::status::plumbing::index_as_worktree::ConflictIndexEntry; use serde::Serialize; use std::any::Any; use std::ops::{Deref, DerefMut}; @@ -233,16 +234,12 @@ pub fn open_repo(path: impl Into) -> anyhow::Result { /// /// For simplicity, copy-tracking is not representable right now, but `copy: bool` could be added /// if needed. Copy-tracking is deactivated as well. -#[derive(Clone)] +#[derive(Debug, Clone)] pub struct TreeChange { /// The *relative* path in the worktree where the entry can be found. pub path: BString, /// The specific information about this change. pub status: TreeStatus, - /// The status item that this change is derived from, used in places that need detailed information. - /// This is only set if this instance was created from a worktree-status, and is `None` when created - /// from a tree-diff. - pub status_item: Option, } /// Specifically defines a [`TreeChange`]. @@ -339,19 +336,19 @@ pub struct IgnoredWorktreeChange { pub path: BString, /// The status that caused this change to be ignored. pub status: IgnoredWorktreeTreeChangeStatus, - /// The status item that this change is derived from, used in places that need detailed information. - /// It's `None` if the status item is already present in non-ignored changes. - #[serde(skip)] - pub status_item: Option, } /// The type returned by [`worktree_changes()`](diff::worktree_changes). -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct WorktreeChanges { /// Changes that could be committed. pub changes: Vec, /// Changes that were in the index that we can't handle. The user can see them and interact with them to clear them out before a commit can be made. pub ignored_changes: Vec, + /// All unprocessed changes to the index. + pub index_changes: Vec, + /// The conflicting index entries, along with their relative path `(rela_path, [Entries(base, ours, theirs)])`. + pub index_conflicts: Vec<(BString, Box<[Option; 3]>)>, } /// Computed using the file kinds/modes of two [`ChangeState`] instances to represent diff --git a/crates/but-core/src/ui.rs b/crates/but-core/src/ui.rs index 135c3f3b06..f6015993c7 100644 --- a/crates/but-core/src/ui.rs +++ b/crates/but-core/src/ui.rs @@ -29,6 +29,8 @@ impl From for WorktreeChanges { crate::WorktreeChanges { changes, ignored_changes, + index_changes: _, + index_conflicts: _, }: crate::WorktreeChanges, ) -> Self { WorktreeChanges { @@ -269,20 +271,12 @@ impl From for crate::TreeChange { crate::TreeChange { path: path_bytes, status: status.into(), - // Lossy conversion, but this is fine. - status_item: None, } } } impl From for TreeChange { - fn from( - crate::TreeChange { - path, - status, - status_item: _, - }: crate::TreeChange, - ) -> Self { + fn from(crate::TreeChange { path, status }: crate::TreeChange) -> Self { TreeChange { path: path.clone().into(), path_bytes: path, diff --git a/crates/but-workspace/src/snapshot/create_tree.rs b/crates/but-workspace/src/snapshot/create_tree.rs index 1e28698051..5833592a2a 100644 --- a/crates/but-workspace/src/snapshot/create_tree.rs +++ b/crates/but-workspace/src/snapshot/create_tree.rs @@ -64,7 +64,6 @@ pub(super) mod function { use but_core::{ChangeState, RefMetadata}; use gix::diff::index::Change; use gix::object::tree::EntryKind; - use gix::status::plumbing::index_as_worktree::EntryStatus; use std::collections::BTreeSet; use tracing::instrument; @@ -98,8 +97,9 @@ pub(super) mod function { /// even though it might be that the respective objects aren't written to disk yet. /// - Note that this tree may contain files with conflict markers as it will pick up the conflicting state visible on disk. /// * `index` - /// - A representation of the non-conflicting and changed portions of the index, without its meta-data. - /// - may be empty if only conflicts exist. + /// - A representation of the non-conflicting portions of the index, without its meta-data, + /// ready to be read back as an index. + /// - It's present only if there is at least one modification compared to the `HEAD^{tree}`. /// * `index-conflicts` /// - `/[1,2,3]` - the blobs at their respective stages. #[instrument(skip(changes, _workspace_and_meta), err(Debug))] @@ -121,37 +121,24 @@ pub(super) mod function { .filter(|c| selection.contains(&c.path)) .map(|c| Ok(DiffSpec::from(c))) .collect(); - changes_to_apply.extend( - changes - .ignored_changes - .iter() - .filter_map(|c| match &c.status_item { - Some(gix::status::Item::IndexWorktree( - gix::status::index_worktree::Item::Modification { - status: EntryStatus::Conflict { .. }, - rela_path, - .. - }, - )) => Some(rela_path), - _ => None, - }) - .filter(|rela_path| selection.contains(rela_path.as_bstr())) - .map(|rela_path| { - // Create a pretend-addition to pick up conflicted paths as well. - Ok(DiffSpec::from(but_core::TreeChange { - path: rela_path.to_owned(), - status: but_core::TreeStatus::Addition { - state: ChangeState { - id: repo.object_hash().null(), - // This field isn't relevant when entries are read from disk. - kind: EntryKind::Tree, - }, - is_untracked: true, - }, - status_item: None, - })) - }), - ); + changes_to_apply.extend(changes.index_conflicts.iter().filter_map(|(rela_path, _)| { + if !selection.contains(rela_path.as_bstr()) { + return None; + } + // Create a pretend-addition to pick up conflicted paths as well. + Ok(DiffSpec::from(but_core::TreeChange { + path: rela_path.to_owned(), + status: but_core::TreeStatus::Addition { + state: ChangeState { + id: repo.object_hash().null(), + // This field isn't relevant when entries are read from disk. + kind: EntryKind::Tree, + }, + is_untracked: true, + }, + })) + .into() + })); let (new_tree, base_tree) = commit_engine::tree::apply_worktree_changes( head_tree_id.into(), @@ -212,31 +199,14 @@ pub(super) mod function { changes: but_core::WorktreeChanges, selection: BTreeSet, ) -> anyhow::Result, Option)>> { - let mut conflicts = Vec::new(); + let conflicts: Vec<_> = changes + .index_conflicts + .into_iter() + .filter(|(rela_path, _)| selection.contains(rela_path)) + .collect(); let changes: Vec<_> = changes - .changes + .index_changes .into_iter() - .filter_map(|c| c.status_item) - .chain( - changes - .ignored_changes - .into_iter() - .filter_map(|c| c.status_item), - ) - .filter_map(|item| match item { - gix::status::Item::IndexWorktree( - gix::status::index_worktree::Item::Modification { - status: EntryStatus::Conflict { entries, .. }, - rela_path, - .. - }, - ) => { - conflicts.push((rela_path, entries)); - None - } - gix::status::Item::TreeIndex(c) => Some(c), - _ => None, - }) .filter(|c| selection.iter().any(|path| path == c.location())) .collect(); diff --git a/crates/but-workspace/tests/fixtures/generated-archives/.gitignore b/crates/but-workspace/tests/fixtures/generated-archives/.gitignore index 7736578fb2..2f8bfd7165 100644 --- a/crates/but-workspace/tests/fixtures/generated-archives/.gitignore +++ b/crates/but-workspace/tests/fixtures/generated-archives/.gitignore @@ -28,4 +28,5 @@ /with-remotes-and-workspace.tar /with-conflict.tar /journey*.tar -/single-branch*.tar \ No newline at end of file +/single-branch*.tar +/index*.tar \ No newline at end of file diff --git a/crates/but-workspace/tests/fixtures/scenario/index-modified-added-deleted.sh b/crates/but-workspace/tests/fixtures/scenario/index-modified-added-deleted.sh new file mode 100644 index 0000000000..efb92b23a0 --- /dev/null +++ b/crates/but-workspace/tests/fixtures/scenario/index-modified-added-deleted.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash + +### Description +# A commit with an executable, a normal file, and an untracked fifo, +# which subsequently get modified and deleted in the index, with the symlink being added to it. +set -eu -o pipefail + +git init +echo content > modified-content +echo change-exe-bit > modified-exe +echo "soon deleted in index but left on disk" > deleted-exe && chmod +x deleted-exe +mkfifo fifo-should-be-ignored + +git add . && git commit -m "init" + +echo index-content >modified-content && \ + git add modified-content && \ + echo content >modified-content + +ln -s only-in-index link && \ + git add link && \ + rm link + +git rm --cached deleted-exe + +chmod +x modified-exe && \ + git add modified-exe && \ + chmod -x modified-exe + diff --git a/crates/but-workspace/tests/workspace/snapshot/index_create_and_resolve.rs b/crates/but-workspace/tests/workspace/snapshot/index_create_and_resolve.rs index 8ed8215331..3512806cc9 100644 --- a/crates/but-workspace/tests/workspace/snapshot/index_create_and_resolve.rs +++ b/crates/but-workspace/tests/workspace/snapshot/index_create_and_resolve.rs @@ -7,9 +7,9 @@ use gix::prelude::ObjectIdExt; #[test] fn unborn_added_to_index() -> anyhow::Result<()> { let repo = read_only_in_memory_scenario("unborn-all-file-types-added-to-index")?; - let (head_tree_id, state, no_workspace_and_meta) = args_for_worktree_changes(&repo)?; + let (head_tree_id, mut state, no_workspace_and_meta) = args_for_worktree_changes(&repo)?; - let out = snapshot::create_tree(head_tree_id, state, no_workspace_and_meta)?; + let out = snapshot::create_tree(head_tree_id, state.clone(), no_workspace_and_meta)?; insta::assert_snapshot!(visualize_tree(out.snapshot_tree.attach(&repo)), @r#" 085f2bf ├── HEAD:4b825dc @@ -62,15 +62,20 @@ fn unborn_added_to_index() -> anyhow::Result<()> { res_out.workspace_references.is_none(), "didn't ask to store this" ); + + state.selection.clear(); + let out = snapshot::create_tree(head_tree_id, state, no_workspace_and_meta)?; + // An empty selection always means there is no effective change. + insta::assert_snapshot!(visualize_tree(out.snapshot_tree.attach(&repo)), @"4b825dc"); Ok(()) } #[test] fn with_conflicts() -> anyhow::Result<()> { let repo = read_only_in_memory_scenario("merge-with-two-branches-conflict")?; - let (head_tree_id, state, no_workspace_and_meta) = args_for_worktree_changes(&repo)?; + let (head_tree_id, mut state, no_workspace_and_meta) = args_for_worktree_changes(&repo)?; - let out = snapshot::create_tree(head_tree_id, state, no_workspace_and_meta)?; + let out = snapshot::create_tree(head_tree_id, state.clone(), no_workspace_and_meta)?; insta::assert_snapshot!(visualize_tree(out.snapshot_tree.attach(&repo)), @r#" 60bd065 ├── HEAD:429a9b9 @@ -118,6 +123,59 @@ fn with_conflicts() -> anyhow::Result<()> { 100644:e33f5e9 file:3 "); + assert!(res_out.metadata.is_none()); + assert!( + res_out.workspace_references.is_none(), + "didn't ask to store this" + ); + + state.selection.clear(); + let out = snapshot::create_tree(head_tree_id, state, no_workspace_and_meta)?; + // An empty selection always means there is no effective change. + insta::assert_snapshot!(visualize_tree(out.snapshot_tree.attach(&repo)), @"4b825dc"); + Ok(()) +} + +#[test] +fn index_added_modified_deleted() -> anyhow::Result<()> { + let repo = read_only_in_memory_scenario("index-modified-added-deleted")?; + let (head_tree_id, state, no_workspace_and_meta) = args_for_worktree_changes(&repo)?; + + let out = snapshot::create_tree(head_tree_id, state, no_workspace_and_meta)?; + insta::assert_snapshot!(visualize_tree(out.snapshot_tree.attach(&repo)), @r#" + 449404d + └── index:3fd7ead + ├── link:120000:e940347 "only-in-index" + ├── modified-content:100644:70c2547 "index-content\n" + └── modified-exe:100755:ef1943d "change-exe-bit\n" + "#); + insta::assert_debug_snapshot!(out, @r" + Outcome { + snapshot_tree: Sha1(449404df68c6041fc4f9b2f1cd30725bdb7ba329), + head_tree: Sha1(632babec715de181e63036ee0cc3686efa67528d), + worktree: None, + index: Some( + Sha1(3fd7ead2468a4def3db4c946c0f3b933eb8f2682), + ), + index_conflicts: None, + workspace_references: None, + head_references: None, + metadata: None, + } + "); + + let res_out = snapshot::resolve_tree( + out.snapshot_tree.attach(&repo), + out.head_tree, + snapshot::resolve_tree::Options::default(), + )?; + let index = res_out.index.expect("the index was altered"); + insta::assert_snapshot!(visualize_index(&index), @r" + 120000:e940347 link + 100644:70c2547 modified-content + 100755:ef1943d modified-exe + "); + assert!(res_out.metadata.is_none()); assert!( res_out.workspace_references.is_none(), diff --git a/crates/but-workspace/tests/workspace/snapshot/worktree_create_and_resolve.rs b/crates/but-workspace/tests/workspace/snapshot/worktree_create_and_resolve.rs index e4da7607b6..a041bf35f5 100644 --- a/crates/but-workspace/tests/workspace/snapshot/worktree_create_and_resolve.rs +++ b/crates/but-workspace/tests/workspace/snapshot/worktree_create_and_resolve.rs @@ -49,9 +49,9 @@ fn unborn_empty() -> anyhow::Result<()> { #[test] fn unborn_untracked() -> anyhow::Result<()> { let repo = read_only_in_memory_scenario("unborn-untracked-all-file-types")?; - let (head_tree_id, state, no_workspace_and_meta) = args_for_worktree_changes(&repo)?; + let (head_tree_id, mut state, no_workspace_and_meta) = args_for_worktree_changes(&repo)?; - let out = snapshot::create_tree(head_tree_id, state, no_workspace_and_meta)?; + let out = snapshot::create_tree(head_tree_id, state.clone(), no_workspace_and_meta)?; assert!(!out.is_empty(), "it picks up the untracked files"); insta::assert_snapshot!(visualize_tree(out.snapshot_tree.attach(&repo)), @r#" a863d4e @@ -96,15 +96,20 @@ fn unborn_untracked() -> anyhow::Result<()> { res_out.workspace_references.is_none(), "didn't ask to store this" ); + + state.selection.clear(); + let out = snapshot::create_tree(head_tree_id, state, no_workspace_and_meta)?; + // An empty selection always means there is no effective change. + insta::assert_snapshot!(visualize_tree(out.snapshot_tree.attach(&repo)), @"4b825dc"); Ok(()) } #[test] fn worktree_all_filetypes() -> anyhow::Result<()> { let repo = read_only_in_memory_scenario("all-file-types-renamed-and-modified")?; - let (head_tree_id, state, no_workspace_and_meta) = args_for_worktree_changes(&repo)?; + let (head_tree_id, mut state, no_workspace_and_meta) = args_for_worktree_changes(&repo)?; - let out = snapshot::create_tree(head_tree_id, state, no_workspace_and_meta)?; + let out = snapshot::create_tree(head_tree_id, state.clone(), no_workspace_and_meta)?; insta::assert_snapshot!(visualize_tree(out.snapshot_tree.attach(&repo)), @r#" 9d274f3 ├── HEAD:3fd29f0 @@ -152,5 +157,10 @@ fn worktree_all_filetypes() -> anyhow::Result<()> { res_out.workspace_references.is_none(), "didn't ask to store this" ); + + state.selection.clear(); + let out = snapshot::create_tree(head_tree_id, state, no_workspace_and_meta)?; + // An empty selection always means there is no effective change. + insta::assert_snapshot!(visualize_tree(out.snapshot_tree.attach(&repo)), @"4b825dc"); Ok(()) }