From f9d9de006568e2fab8023f03a510d27ebc43dd41 Mon Sep 17 00:00:00 2001 From: Peter Grayson Date: Mon, 22 Aug 2022 20:11:14 -0400 Subject: [PATCH] Use stupid temporary indexes instead of git2 The new Stupid::with_temp_index() method replaces the Index::with_temp_index() and Index::with_temp_index_file() extension methods. This is motivated by sparse checkouts (#195) since git2::Index does not comprehend sparse checkouts. --- src/cmd/refresh.rs | 11 +++----- src/cmd/spill.rs | 4 +-- src/cmd/squash.rs | 11 +++----- src/index.rs | 56 ---------------------------------------- src/main.rs | 1 - src/patchedit/mod.rs | 7 ++--- src/stack/transaction.rs | 28 +++++++++----------- src/stupid/mod.rs | 46 ++++++++++++--------------------- 8 files changed, 38 insertions(+), 126 deletions(-) delete mode 100644 src/index.rs diff --git a/src/cmd/refresh.rs b/src/cmd/refresh.rs index 7fb56b7b..363f45bc 100644 --- a/src/cmd/refresh.rs +++ b/src/cmd/refresh.rs @@ -15,7 +15,6 @@ use crate::{ color::get_color_stdout, commit::{CommitMessage, RepositoryCommitExtended}, hook::run_pre_commit_hook, - index::TemporaryIndex, patchedit, patchname::PatchName, signature::SignatureExtended, @@ -280,9 +279,7 @@ fn run(matches: &ArgMatches) -> Result<()> { let ours = patch_commit.tree_id(); let theirs = temp_commit.tree_id(); - if let Some(tree_id) = repo.with_temp_index_file(|temp_index| { - let stupid = repo.stupid(); - let stupid_temp = stupid.with_index_path(temp_index.path().unwrap()); + if let Some(tree_id) = repo.stupid().with_temp_index(|stupid_temp| { stupid_temp.read_tree(ours)?; if stupid_temp.apply_treediff_to_index(base, theirs)? { let tree_id = stupid_temp.write_tree()?; @@ -412,13 +409,11 @@ fn write_tree( // may be formed into a coherent tree while leaving the default index as-is. let stupid = stack.repo.stupid(); if is_path_limiting { - let stupid_temp = stupid.get_temp_index_context(); - let stupid_temp = stupid_temp.context(); - let tree_id_result = { + let tree_id_result = stupid.with_temp_index(|stupid_temp| { stupid_temp.read_tree(stack.branch_head.tree_id())?; stupid_temp.update_index(Some(refresh_paths))?; stupid_temp.write_tree() - }; + }); stupid.update_index(Some(refresh_paths))?; tree_id_result } else { diff --git a/src/cmd/spill.rs b/src/cmd/spill.rs index 1b2e0a12..a2360e1e 100644 --- a/src/cmd/spill.rs +++ b/src/cmd/spill.rs @@ -10,7 +10,6 @@ use clap::{Arg, ArgMatches}; use crate::{ color::get_color_stdout, commit::{CommitExtended, RepositoryCommitExtended}, - index::TemporaryIndex, repo::RepositoryExtended, stack::{Error, Stack, StackStateAccess}, stupid::Stupid, @@ -86,8 +85,7 @@ fn run(matches: &ArgMatches) -> Result<()> { let mut index = repo.index()?; let tree_id = if let Some(pathspecs) = matches.get_many::("pathspecs") { - stack.repo.with_temp_index_file(|temp_index| { - let stupid_temp = stupid.with_index_path(temp_index.path().unwrap()); + stupid.with_temp_index(|stupid_temp| { stupid_temp.read_tree(patch_commit.tree_id())?; stupid_temp.apply_pathlimited_treediff_to_index( patch_commit.tree_id(), diff --git a/src/cmd/squash.rs b/src/cmd/squash.rs index b1d2ae47..95c92916 100644 --- a/src/cmd/squash.rs +++ b/src/cmd/squash.rs @@ -11,7 +11,6 @@ use clap::{Arg, ArgMatches}; use crate::{ color::get_color_stdout, commit::CommitExtended, - index::TemporaryIndex, patchedit, patchname::PatchName, patchrange, print_info_message, @@ -234,20 +233,18 @@ fn try_squash( ) -> Result> { let repo = trans.repo(); let base_commit = trans.get_patch_commit(&patchnames[0]); - if let Some(tree_id) = repo.with_temp_index_file(|temp_index| { - let stupid = repo.stupid(); - let stupid = stupid.with_index_path(temp_index.path().unwrap()); - stupid.read_tree(base_commit.tree_id())?; + if let Some(tree_id) = repo.stupid().with_temp_index(|stupid_temp| { + stupid_temp.read_tree(base_commit.tree_id())?; for commit in patchnames[1..].iter().map(|pn| trans.get_patch_commit(pn)) { let parent = commit.parent(0)?; if parent.tree_id() != commit.tree_id() - && !stupid.apply_treediff_to_index(parent.tree_id(), commit.tree_id())? + && !stupid_temp.apply_treediff_to_index(parent.tree_id(), commit.tree_id())? { return Ok(None); } } - let tree_id = stupid.write_tree()?; + let tree_id = stupid_temp.write_tree()?; Ok(Some(tree_id)) })? { if let patchedit::EditOutcome::Committed { diff --git a/src/index.rs b/src/index.rs deleted file mode 100644 index e299859d..00000000 --- a/src/index.rs +++ /dev/null @@ -1,56 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0-only - -//! Extension trait for [`git2::Index`]. - -use anyhow::Result; - -/// Extend [`git2::Index`] with methods to use temporary indexes. -pub(crate) trait TemporaryIndex { - /// Perform actions with an in-memory temporary index. - /// - /// All actions on the temporary index must be performed within the provided - /// closure. The temporary index will not be written to the filesystem by default, - /// but the caller may do so using [`git2::Index::write()`]. - fn with_temp_index(&self, f: F) -> Result - where - F: FnOnce(&mut git2::Index) -> Result; - - /// Perform actions with a temporary index file. - /// - /// The temporary index file is automatically deleted when this call returns. - fn with_temp_index_file(&self, f: F) -> Result - where - F: FnOnce(&mut git2::Index) -> Result; -} - -impl TemporaryIndex for git2::Repository { - fn with_temp_index(&self, f: F) -> Result - where - F: FnOnce(&mut git2::Index) -> Result, - { - let mut temp_index = git2::Index::new()?; - let mut orig_index = self.index()?; - self.set_index(&mut temp_index)?; - let result = f(&mut temp_index); - self.set_index(&mut orig_index) - .expect("can reset to original index"); - result - } - - fn with_temp_index_file(&self, f: F) -> Result - where - F: FnOnce(&mut git2::Index) -> Result, - { - // TODO: dynamic file name - let temp_index_path = self.path().join("index-temp-stgit"); - let mut temp_index = git2::Index::open(&temp_index_path)?; - let mut orig_index = self.index()?; - temp_index.write()?; - self.set_index(&mut temp_index)?; - let result = f(&mut temp_index); - self.set_index(&mut orig_index) - .expect("can reset to original index"); - std::fs::remove_file(&temp_index_path).expect("can remove temp index file"); - result - } -} diff --git a/src/main.rs b/src/main.rs index 1aa16d46..bebf45ea 100644 --- a/src/main.rs +++ b/src/main.rs @@ -16,7 +16,6 @@ mod cmd; mod color; mod commit; mod hook; -mod index; mod patchedit; mod patchname; mod patchrange; diff --git a/src/patchedit/mod.rs b/src/patchedit/mod.rs index 35837559..9929c2bb 100644 --- a/src/patchedit/mod.rs +++ b/src/patchedit/mod.rs @@ -25,7 +25,6 @@ use clap::{ use crate::{ commit::{CommitExtended, CommitMessage, RepositoryCommitExtended}, - index::TemporaryIndex, patchname::PatchName, signature::{self, SignatureExtended}, stack::StackStateAccess, @@ -461,6 +460,7 @@ impl<'a, 'repo> EditBuilder<'a, 'repo> { }, } = self; + let stupid = repo.stupid(); let config = repo.config()?; let default_committer = git2::Signature::default_committer(Some(&config))?; let committer = patch_commit @@ -600,7 +600,6 @@ impl<'a, 'repo> EditBuilder<'a, 'repo> { { let old_tree = repo.find_commit(parent_id)?.tree()?; let new_tree = repo.find_tree(tree_id)?; - let stupid = repo.stupid(); let diff_buf = if patch_commit.is_some() || old_tree.id() != new_tree.id() { stupid.diff_tree_patch( old_tree.id(), @@ -722,9 +721,7 @@ impl<'a, 'repo> EditBuilder<'a, 'repo> { let tree_id = if need_to_apply_diff { let diff = diff.unwrap().0; - match repo.with_temp_index_file(|temp_index| { - let stupid = repo.stupid(); - let stupid_temp = stupid.with_index_path(temp_index.path().unwrap()); + match stupid.with_temp_index(|stupid_temp| { stupid_temp.read_tree(parent_id)?; stupid_temp.apply_to_index(&diff)?; stupid_temp.write_tree() diff --git a/src/stack/transaction.rs b/src/stack/transaction.rs index e5d79132..c7d0a9ee 100644 --- a/src/stack/transaction.rs +++ b/src/stack/transaction.rs @@ -36,11 +36,10 @@ use termcolor::WriteColor; use crate::{ commit::{CommitExtended, RepositoryCommitExtended}, - index::TemporaryIndex, patchname::PatchName, signature::SignatureExtended, stack::{PatchState, Stack, StackStateAccess}, - stupid::Stupid, + stupid::{Stupid, StupidContext}, }; use super::{error::Error, state::StackState}; @@ -1034,11 +1033,12 @@ impl<'repo> StackTransaction<'repo> { where P: AsRef, { - self.stack.repo.with_temp_index_file(|temp_index| { + let stupid = self.stack.repo.stupid(); + stupid.with_temp_index(|stupid_temp| { let mut temp_index_tree_id: Option = None; let merged = if check_merged { - Some(self.check_merged(patchnames, temp_index, &mut temp_index_tree_id)?) + Some(self.check_merged(patchnames, stupid_temp, &mut temp_index_tree_id)?) } else { None }; @@ -1054,10 +1054,11 @@ impl<'repo> StackTransaction<'repo> { patchname, already_merged, is_last, - temp_index, + stupid_temp, &mut temp_index_tree_id, )?; } + Ok(()) }) } @@ -1067,7 +1068,7 @@ impl<'repo> StackTransaction<'repo> { patchname: &PatchName, already_merged: bool, is_last: bool, - temp_index: &mut git2::Index, + stupid_temp: &StupidContext, temp_index_tree_id: &mut Option, ) -> Result<()> { let repo = self.stack.repo; @@ -1096,10 +1097,6 @@ impl<'repo> StackTransaction<'repo> { (new_parent.tree_id(), patch_commit.tree_id()) }; let base = old_parent.tree_id(); - // let ours = new_parent.tree_id(); - // let theirs = patch_commit.tree_id(); - - let stupid_temp = stupid.with_index_path(temp_index.path().unwrap()); if temp_index_tree_id != &Some(ours) { stupid_temp.read_tree(ours)?; @@ -1218,20 +1215,17 @@ impl<'repo> StackTransaction<'repo> { fn check_merged<'a, P>( &self, patchnames: &'a [P], - temp_index: &mut git2::Index, + stupid_temp: &StupidContext, temp_index_tree_id: &mut Option, ) -> Result> where P: AsRef, { - let repo = self.stack.repo; - let stupid = repo.stupid(); - let stupid = stupid.with_index_path(temp_index.path().unwrap()); let head_tree_id = self.stack.branch_head.tree_id(); let mut merged: Vec<&PatchName> = vec![]; if temp_index_tree_id != &Some(head_tree_id) { - stupid.read_tree(head_tree_id)?; + stupid_temp.read_tree(head_tree_id)?; *temp_index_tree_id = Some(head_tree_id); } @@ -1245,7 +1239,9 @@ impl<'repo> StackTransaction<'repo> { let parent_commit = patch_commit.parent(0)?; - if stupid.apply_treediff_to_index(patch_commit.tree_id(), parent_commit.tree_id())? { + if stupid_temp + .apply_treediff_to_index(patch_commit.tree_id(), parent_commit.tree_id())? + { merged.push(patchname); *temp_index_tree_id = None; } diff --git a/src/stupid/mod.rs b/src/stupid/mod.rs index 72020181..60b1ba50 100644 --- a/src/stupid/mod.rs +++ b/src/stupid/mod.rs @@ -24,7 +24,7 @@ pub(crate) mod status; use std::{ ffi::{OsStr, OsString}, io::Write, - path::{Path, PathBuf}, + path::Path, process::{Command, Stdio}, }; @@ -62,32 +62,14 @@ pub(crate) struct StupidContext<'repo, 'index> { pub work_dir: Option<&'repo Path>, } -pub(crate) struct StupidTempIndexContext<'repo> { - git_dir: Option<&'repo Path>, - temp_index_path: PathBuf, - work_dir: Option<&'repo Path>, -} - -impl<'repo> StupidTempIndexContext<'repo> { - pub(crate) fn context(&self) -> StupidContext<'repo, '_> { - StupidContext { - git_dir: self.git_dir, - index_path: Some(self.temp_index_path.as_path()), - work_dir: self.work_dir, - } - } -} - impl<'repo, 'index> StupidContext<'repo, 'index> { - pub(crate) fn with_index_path(&self, index_path: &'index Path) -> StupidContext { - StupidContext { - git_dir: self.git_dir, - index_path: Some(index_path), - work_dir: self.work_dir, - } - } - - pub(crate) fn get_temp_index_context(&self) -> StupidTempIndexContext { + /// Perform actions with a temporary index file. + /// + /// The temporary index file is automatically deleted when this call returns. + pub(crate) fn with_temp_index(&self, f: F) -> Result + where + F: FnOnce(&StupidContext) -> Result, + { let temp_index_root = if let Some(git_dir) = self.git_dir { git_dir } else { @@ -96,12 +78,16 @@ impl<'repo, 'index> StupidContext<'repo, 'index> { .parent() .expect("git index path has parent") }; - - StupidTempIndexContext { + let index_tempfile = tempfile::Builder::new() + .prefix("index-temp-stg") + .tempfile_in(temp_index_root)?; + let stupid_temp = StupidContext { git_dir: self.git_dir, - temp_index_path: temp_index_root.join("index-temp-stgit"), + index_path: Some(index_tempfile.path()), work_dir: self.work_dir, - } + }; + + f(&stupid_temp) } fn git(&self) -> Command {