Skip to content

Commit 2462790

Browse files
jpgraysonjpgrayson
committed
Repair spilling added files with path limits
When using `stg spill` with a patch containing newly added files, those newly added files were not spilled when a pathlimit was present on the command line. This issue is repaired by using a different and more robust strategy for creating the updated (spilled) index for the patch. Relevant test cases are added to t2705-spill.sh.
1 parent dd26eff commit 2462790

File tree

3 files changed

+167
-27
lines changed

3 files changed

+167
-27
lines changed

‎src/cmd/spill.rs‎

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,16 @@
22

33
//! `stg spill` implementation.
44
5-
use std::path::{Path, PathBuf};
6-
75
use anyhow::Result;
8-
use bstr::ByteSlice;
96
use clap::{Arg, ArgMatches};
107

118
use crate::{
129
color::get_color_stdout,
1310
commit::{CommitExtended, RepositoryCommitExtended},
11+
index::TemporaryIndex,
1412
repo::RepositoryExtended,
1513
stack::{Error, Stack, StackStateAccess},
14+
stupid::Stupid,
1615
};
1716

1817
pub(super) fn get_command() -> (&'static str, super::StGitCommand) {
@@ -83,30 +82,17 @@ fn run(matches: &ArgMatches) -> Result<()> {
8382
let mut index = repo.index()?;
8483

8584
let tree_id = if let Some(pathspecs) = matches.values_of_os("pathspecs") {
86-
let workdir = repo.workdir().expect("not a bare repository");
87-
let curdir = std::env::current_dir()?;
88-
let mut norm_pathspec: Vec<PathBuf> = Vec::with_capacity(pathspecs.len());
89-
for spec in pathspecs {
90-
norm_pathspec.push(crate::pathspec::normalize_pathspec(
91-
workdir,
92-
&curdir,
93-
Path::new(spec),
94-
)?);
95-
}
96-
let pathspec = git2::Pathspec::new(&norm_pathspec)?;
97-
98-
let parent_tree = parent.tree()?;
99-
let mut parent_index = git2::Index::new()?;
100-
parent_index.read_tree(&parent_tree)?;
101-
102-
for parent_entry in parent_index.iter() {
103-
if let Ok(path) = parent_entry.path.to_path() {
104-
if pathspec.matches_path(path, git2::PathspecFlags::DEFAULT) {
105-
index.add(&parent_entry)?;
106-
}
107-
}
108-
}
109-
index.write_tree()?
85+
stack.repo.with_temp_index_file(|temp_index| {
86+
let stupid = repo.stupid();
87+
let stupid_temp = stupid.with_index_path(temp_index.path().unwrap());
88+
stupid_temp.read_tree(patch_commit.tree_id())?;
89+
stupid_temp.apply_pathlimited_treediff_to_index(
90+
patch_commit.tree_id(),
91+
parent.tree_id(),
92+
pathspecs,
93+
)?;
94+
stupid_temp.write_tree()
95+
})?
11096
} else {
11197
parent.tree_id()
11298
};

‎src/stupid.rs‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,55 @@ impl<'repo, 'index> StupidContext<'repo, 'index> {
250250
Ok(apply_output.status.success())
251251
}
252252

253+
/// Apply path limited diff between to trees to specified index.
254+
pub(crate) fn apply_pathlimited_treediff_to_index<SpecIter, SpecArg>(
255+
&self,
256+
tree1: git2::Oid,
257+
tree2: git2::Oid,
258+
pathspecs: SpecIter,
259+
) -> Result<bool>
260+
where
261+
SpecIter: IntoIterator<Item = SpecArg>,
262+
SpecArg: AsRef<OsStr>,
263+
{
264+
if tree1 == tree2 {
265+
return Ok(true);
266+
}
267+
let mut diff_tree_command = self.git();
268+
diff_tree_command
269+
.args(["diff-tree", "--full-index", "--binary", "--patch"])
270+
.arg(tree1.to_string())
271+
.arg(tree2.to_string())
272+
.arg("--")
273+
.args(pathspecs);
274+
275+
let diff = diff_tree_command
276+
.stdin(Stdio::null())
277+
.stdout(Stdio::piped())
278+
.output_git()?
279+
.require_success("diff-tree")?
280+
.stdout;
281+
282+
if diff.is_empty() {
283+
return Ok(true);
284+
}
285+
286+
let apply_output = self
287+
.git_in_work_root()
288+
.args(["apply", "--cached"]) // --3way
289+
.stdin(Stdio::piped())
290+
.stdout(Stdio::null())
291+
.in_and_out(&diff)?;
292+
293+
if apply_output.status.success() {
294+
Ok(true)
295+
} else if apply_output.status.code() == Some(1) {
296+
Ok(false)
297+
} else {
298+
Err(git_command_error("apply", &apply_output.stderr))
299+
}
300+
}
301+
253302
/// Apply diff between two trees to worktree and index.
254303
///
255304
/// Pipes `git diff-tree | git apply --index`.

‎t/t2705-spill.sh‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,4 +241,109 @@ test_expect_success 'Spill with modified spillable file' '
241241
stg undo --hard
242242
'
243243

244+
cat > expected-status.txt <<EOF
245+
A dir0/dir1/new.txt
246+
EOF
247+
cat > expected-files.txt <<EOF
248+
M dir0/a.txt
249+
M dir0/dir1/e.txt
250+
EOF
251+
test_expect_success 'Spill patch with just-added files' '
252+
echo "new file" > dir0/dir1/new.txt &&
253+
stg add dir0/dir1/new.txt &&
254+
echo "modification" >> dir0/a.txt &&
255+
echo "modification" >> dir0/dir1/e.txt &&
256+
stg add dir0 &&
257+
stg new --refresh -m add-new &&
258+
(
259+
cd dir0 &&
260+
stg spill dir1/new.txt
261+
) &&
262+
stg status > status.txt &&
263+
test_cmp expected-status.txt status.txt &&
264+
stg files > files.txt &&
265+
test_cmp expected-files.txt files.txt &&
266+
grep "modification" dir0/a.txt &&
267+
grep "modification" dir0/dir1/e.txt &&
268+
stg undo --hard &&
269+
stg delete --top
270+
'
271+
272+
cat > expected-status.txt <<EOF
273+
?? dir0/dir1/new.txt
274+
EOF
275+
cat > expected-files.txt <<EOF
276+
M dir0/a.txt
277+
M dir0/dir1/e.txt
278+
EOF
279+
test_expect_success 'Spill and reset patch with just-added files' '
280+
echo "new file" > dir0/dir1/new.txt &&
281+
stg add dir0/dir1/new.txt &&
282+
echo "modification" >> dir0/a.txt &&
283+
echo "modification" >> dir0/dir1/e.txt &&
284+
stg add dir0 &&
285+
stg new --refresh -m add-new &&
286+
(
287+
cd dir0 &&
288+
stg spill --reset dir1/new.txt
289+
) &&
290+
stg status > status.txt &&
291+
test_cmp expected-status.txt status.txt &&
292+
stg files > files.txt &&
293+
test_cmp expected-files.txt files.txt &&
294+
grep "modification" dir0/a.txt &&
295+
grep "modification" dir0/dir1/e.txt &&
296+
stg undo --hard &&
297+
stg delete --top
298+
'
299+
300+
cat > expected-status.txt <<EOF
301+
M dir0/dir1/e.txt
302+
A dir0/dir1/new.txt
303+
EOF
304+
cat > expected-files.txt <<EOF
305+
M dir0/a.txt
306+
EOF
307+
test_expect_success 'Spill patch with just-added files in wildcard dir' '
308+
echo "new file" > dir0/dir1/new.txt &&
309+
stg add dir0/dir1/new.txt &&
310+
echo "modification" >> dir0/a.txt &&
311+
echo "modification" >> dir0/dir1/e.txt &&
312+
stg add dir0 &&
313+
stg new --refresh -m add-new &&
314+
(
315+
cd dir0 &&
316+
stg spill dir1
317+
) &&
318+
stg status > status.txt &&
319+
test_cmp expected-status.txt status.txt &&
320+
stg files > files.txt &&
321+
test_cmp expected-files.txt files.txt &&
322+
grep "modification" dir0/a.txt &&
323+
grep "modification" dir0/dir1/e.txt &&
324+
stg undo --hard &&
325+
stg delete --top
326+
'
327+
328+
cat > expected-status.txt <<EOF
329+
D dir0/dir1/e.txt
330+
EOF
331+
cat > expected-files.txt <<EOF
332+
M dir0/a.txt
333+
EOF
334+
test_expect_success 'Spill removed file' '
335+
echo "modification" >> dir0/a.txt &&
336+
stg add dir0/a.txt &&
337+
stg rm dir0/dir1/e.txt &&
338+
stg new -rm rm-file &&
339+
stg spill dir0/dir1 &&
340+
stg status > status.txt &&
341+
test_cmp expected-status.txt status.txt &&
342+
stg files > files.txt &&
343+
test_cmp expected-files.txt files.txt &&
344+
grep "modification" dir0/a.txt &&
345+
stg undo --hard &&
346+
stg delete --top
347+
'
348+
244349
test_done

0 commit comments

Comments
 (0)