Keep per-file index flags when rebuilding the workspace index

crates/gitbutler-branch-actions/src/integration.rs

@@ -1,4 +1,4 @@
-use std::path::PathBuf;
+use std::{collections::HashMap, path::PathBuf};
 
 use anyhow::{Context as _, Result, anyhow};
 use but_ctx::{Context, access::RepoExclusive};
@@ -170,6 +170,17 @@ pub(crate) fn update_workspace_commit_with_vb_state(
         None,
     )?;
 
+    // Taken before the checkout below, which writes `.git/index` itself and strips these flags
+    // from every path whose content it updates. Reading it back afterwards would find them
+    // already gone.
+    let per_file_flags = {
+        let mut index = repo.index()?;
+        // Only re-reads when the file changed underneath us, which is what picks up a
+        // `git update-index --skip-worktree` made outside the app.
+        index.read(false)?;
+        per_file_index_flags(&index)
+    };
+
     let checkout_res = if checkout_new_worktree && prev_head_id.is_some() {
         let res = but_core::worktree::safe_checkout_from_head(
             final_commit,
@@ -200,6 +211,7 @@ pub(crate) fn update_workspace_commit_with_vb_state(
 
     let mut index = repo.index()?;
     index.read_tree(&repo.find_tree(workspace_tree.to_git2())?)?;
+    restore_per_file_index_flags(&mut index, &per_file_flags)?;
     index.write()?;
 
     // Everything is written out already, so if we fail here, we do so to surface the error
@@ -213,6 +225,62 @@ pub(crate) fn update_workspace_commit_with_vb_state(
     Ok(final_commit)
 }
 
+/// The per-file index state that lives only in the index: `skip-worktree`, which tells Git to
+/// leave a tracked file's worktree copy alone, and `assume-unchanged`.
+///
+/// A tree has nowhere to record either, so rebuilding the index from one drops them for every
+/// path. Collecting them first lets [`restore_per_file_index_flags()`] put them back.
+fn per_file_index_flags(index: &git2::Index) -> HashMap<Vec<u8>, (bool, bool)> {
+    let assume_unchanged_bit = git2::IndexEntryFlag::VALID.bits();
+    let skip_worktree_bit = git2::IndexEntryExtendedFlag::SKIP_WORKTREE.bits();
+
+    index
+        .iter()
+        .filter_map(|entry| {
+            let assume_unchanged = entry.flags & assume_unchanged_bit != 0;
+            let skip_worktree = entry.flags_extended & skip_worktree_bit != 0;
+            (assume_unchanged || skip_worktree)
+                .then(|| (entry.path.clone(), (assume_unchanged, skip_worktree)))
+        })
+        .collect()
+}
+
+/// Put the flags that [`per_file_index_flags()`] collected back onto the paths that are still
+/// tracked, leaving paths the rebuild dropped alone.
+///
+/// Only the two flag bits are carried over. The rest of each entry belongs to the tree the index
+/// was just rebuilt from.
+fn restore_per_file_index_flags(
+    index: &mut git2::Index,
+    per_file_flags: &HashMap<Vec<u8>, (bool, bool)>,
+) -> Result<()> {
+    if per_file_flags.is_empty() {
+        return Ok(());
+    }
+    let assume_unchanged_bit = git2::IndexEntryFlag::VALID.bits();
+    let skip_worktree_bit = git2::IndexEntryExtendedFlag::SKIP_WORKTREE.bits();
+
+    // Collected first because adding entries while iterating the index would invalidate it.
+    let restored: Vec<git2::IndexEntry> = index
+        .iter()
+        .filter_map(|mut entry| {
+            let (assume_unchanged, skip_worktree) = per_file_flags.get(&entry.path)?;
+            if *assume_unchanged {
+                entry.flags |= assume_unchanged_bit;
+            }
+            if *skip_worktree {
+                entry.flags_extended |= skip_worktree_bit;
+            }
+            Some(entry)
+        })
+        .collect();
+
+    for entry in restored {
+        index.add(&entry)?;
+    }
+    Ok(())
+}
+
 pub fn verify_branch(ctx: &Context, perm: &mut RepoExclusive) -> Result<()> {
     verify_current_branch_name(ctx)
         .and_then(verify_head_is_set)

crates/gitbutler-branch-actions/tests/branch-actions/workspace_commit.rs

@@ -3,7 +3,8 @@
     reason = "VirtualBranchesHandle should be replaced with ctx.workspace_* helpers"
 )]
 
-use anyhow::Result;
+use anyhow::{Context as _, Result};
+use but_testsupport::CommandExt as _;
 use but_testsupport::visualize_tree;
 use gitbutler_stack::VirtualBranchesHandle;
 use gix::prelude::ObjectIdExt;
@@ -192,3 +193,111 @@ fn update_workspace_commit_with_diverged_stacks_preserves_target_content() -> Re
 
     Ok(())
 }
+
+/// `skip-worktree` tells Git to leave a tracked file's worktree copy alone. Sparse checkouts set
+/// it, and it is also set by hand to keep local edits to a checked-in file out of the way.
+/// Rebuilding `.git/index` from the workspace tree must not quietly drop it.
+///
+/// `shared.txt` is untouched by either stack, so this also shows the flag going missing on a
+/// path the workspace commit does not change at all.
+#[test]
+fn workspace_commit_preserves_skip_worktree() -> Result<()> {
+    let (ctx, _temp_dir) = command_ctx("adjacent-stacks")?;
+    let worktree_dir = ctx
+        .repo
+        .get()?
+        .workdir()
+        .expect("fixture repo has a worktree")
+        .to_owned();
+
+    but_testsupport::git_at_dir(&worktree_dir)
+        .args(["update-index", "--skip-worktree", "shared.txt"])
+        .run();
+    assert!(
+        index_flag_is_set(&ctx, "shared.txt", gix::index::entry::Flags::SKIP_WORKTREE)?,
+        "precondition: the flag is set before the workspace commit is rebuilt"
+    );
+
+    gitbutler_branch_actions::update_workspace_commit(&ctx, false)?;
+
+    assert!(
+        index_flag_is_set(&ctx, "shared.txt", gix::index::entry::Flags::SKIP_WORKTREE)?,
+        "rebuilding the index from the workspace tree keeps per-file flags"
+    );
+    Ok(())
+}
+
+fn index_flag_is_set(ctx: &Context, path: &str, flag: gix::index::entry::Flags) -> Result<bool> {
+    let repo = ctx.repo.get()?;
+    let index = repo.index()?;
+    let entry = index
+        .entry_by_path(path.into())
+        .with_context(|| format!("{path} should be tracked"))?;
+    Ok(entry.flags.contains(flag))
+}
+
+/// `assume-unchanged` is the other per-file flag Git keeps only in the index, and the issue asks
+/// for it alongside `skip-worktree`.
+///
+/// This one uses `file`, which both stacks modify, because it is only lost when the rebuild
+/// actually replaces the entry - libgit2 keeps an entry whose blob and mode are unchanged, flags
+/// and all. `skip-worktree` above needs no such setup, being dropped either way.
+#[test]
+fn workspace_commit_preserves_assume_unchanged() -> Result<()> {
+    let (ctx, _temp_dir) = command_ctx("adjacent-stacks")?;
+    let worktree_dir = ctx
+        .repo
+        .get()?
+        .workdir()
+        .expect("fixture repo has a worktree")
+        .to_owned();
+
+    but_testsupport::git_at_dir(&worktree_dir)
+        .args(["update-index", "--assume-unchanged", "file"])
+        .run();
+    assert!(
+        index_flag_is_set(&ctx, "file", gix::index::entry::Flags::ASSUME_VALID)?,
+        "precondition: the flag is set before the workspace commit is rebuilt"
+    );
+
+    gitbutler_branch_actions::update_workspace_commit(&ctx, false)?;
+
+    assert!(
+        index_flag_is_set(&ctx, "file", gix::index::entry::Flags::ASSUME_VALID)?,
+        "rebuilding the index from the workspace tree keeps per-file flags"
+    );
+    Ok(())
+}
+
+/// The `checkout_new_worktree` leg checks the working tree out before the index is rebuilt, and
+/// that checkout writes the index itself. The flags have to be taken before it runs, or they are
+/// already gone by the time the rebuild sees them.
+#[test]
+fn workspace_commit_preserves_flags_when_checking_out() -> Result<()> {
+    let (ctx, _temp_dir) = command_ctx("adjacent-stacks")?;
+    let worktree_dir = ctx
+        .repo
+        .get()?
+        .workdir()
+        .expect("fixture repo has a worktree")
+        .to_owned();
+
+    but_testsupport::git_at_dir(&worktree_dir)
+        .args(["update-index", "--skip-worktree", "shared.txt"])
+        .run();
+    but_testsupport::git_at_dir(&worktree_dir)
+        .args(["update-index", "--assume-unchanged", "file"])
+        .run();
+
+    gitbutler_branch_actions::update_workspace_commit(&ctx, true)?;
+
+    assert!(
+        index_flag_is_set(&ctx, "shared.txt", gix::index::entry::Flags::SKIP_WORKTREE)?,
+        "skip-worktree survives the checkout leg"
+    );
+    assert!(
+        index_flag_is_set(&ctx, "file", gix::index::entry::Flags::ASSUME_VALID)?,
+        "assume-unchanged survives the checkout leg"
+    );
+    Ok(())
+}