Add the missing tests for index-snapshot handling.

Author: Byron

crates/but-core/src/diff/worktree.rs

@@ -479,6 +479,7 @@ fn worktree_changes_inner(
     let mut path_check = gix::status::plumbing::SymlinkCheck::new(
         repo.workdir().map(ToOwned::to_owned).context("non-bare")?,
     );
+    let original_changes = tmp.iter().map(|(_, change)| change.clone()).collect();
     for (_origin, change) in tmp {
         // At this point we know that the current `change` is the tree/index variant
         // of a prior change between index/worktree.
@@ -538,6 +539,7 @@ fn worktree_changes_inner(
     Ok(WorktreeChanges {
         changes,
         ignored_changes,
+        original_changes,
     })
 }
 
@@ -935,3 +937,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()
+    }
+}

crates/but-core/src/lib.rs

@@ -346,12 +346,16 @@ pub struct IgnoredWorktreeChange {
 }
 
 /// 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<TreeChange>,
     /// 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<IgnoredWorktreeChange>,
+    /// The unprocessed (but sorted, by path) changes which were used to produce `changes` and `ignored_changes`.
+    /// This doesn't include index conflicts!
+    /// TODO: can this be specific index changes? After all that's the only thing we need.
+    pub original_changes: Vec<TreeChange>,
 }
 
 /// Computed using the file kinds/modes of two [`ChangeState`] instances to represent

crates/but-core/src/ui.rs

@@ -29,6 +29,7 @@ impl From<crate::WorktreeChanges> for WorktreeChanges {
         crate::WorktreeChanges {
             changes,
             ignored_changes,
+            original_changes: _,
         }: crate::WorktreeChanges,
     ) -> Self {
         WorktreeChanges {

crates/but-workspace/src/snapshot/create_tree.rs

@@ -98,8 +98,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`
     ///     - `<entry-path>/[1,2,3]` - the blobs at their respective stages.
     #[instrument(skip(changes, _workspace_and_meta), err(Debug))]
@@ -212,29 +213,26 @@ pub(super) mod function {
         changes: but_core::WorktreeChanges,
         selection: BTreeSet<BString>,
     ) -> anyhow::Result<Option<(Option<gix::ObjectId>, Option<gix::ObjectId>)>> {
-        let mut conflicts = Vec::new();
-        let changes: Vec<_> = changes
-            .changes
+        let conflicts: Vec<_> = changes
+            .ignored_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(
+            .filter_map(|c| match c.status_item {
+                Some(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),
+                )) => Some((rela_path, entries)),
+                _ => None,
+            })
+            .collect();
+
+        let changes: Vec<_> = changes
+            .original_changes
+            .into_iter()
+            .filter_map(|c| match c.status_item {
+                Some(gix::status::Item::TreeIndex(c)) => Some(c),
                 _ => None,
             })
             .filter(|c| selection.iter().any(|path| path == c.location()))

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
+/single-branch*.tar
+/index*.tar

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
+

crates/but-workspace/tests/workspace/snapshot/index_create_and_resolve.rs

@@ -125,3 +125,51 @@ fn with_conflicts() -> anyhow::Result<()> {
     );
     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(),
+        "didn't ask to store this"
+    );
+    Ok(())
+}