Merge pull request #3250 from ethomson/stash

Stash workdir correctly when added in the index, modified in the workdir

src/diff.h

@@ -123,6 +123,25 @@ extern int git_diff_find_similar__calc_similarity(
 extern int git_diff__commit(
 	git_diff **diff, git_repository *repo, const git_commit *commit, const git_diff_options *opts);
 
+/* Merge two `git_diff`s according to the callback given by `cb`. */
+
+typedef git_diff_delta *(*git_diff__merge_cb)(
+	const git_diff_delta *left,
+	const git_diff_delta *right,
+	git_pool *pool);
+
+extern int git_diff__merge(
+	git_diff *onto, const git_diff *from, git_diff__merge_cb cb);
+
+extern git_diff_delta *git_diff__merge_like_cgit(
+	const git_diff_delta *a,
+	const git_diff_delta *b,
+	git_pool *pool);
+
+/* Duplicate a `git_diff_delta` out of the `git_pool` */
+extern git_diff_delta *git_diff__delta_dup(
+	const git_diff_delta *d, git_pool *pool);
+
 /*
  * Sometimes a git_diff_file will have a zero size; this attempts to
  * fill in the size without loading the blob if possible.  If that is

src/diff_tform.c

@@ -15,7 +15,7 @@
 #include "fileops.h"
 #include "config.h"
 
-static git_diff_delta *diff_delta__dup(
+git_diff_delta *git_diff__delta_dup(
 	const git_diff_delta *d, git_pool *pool)
 {
 	git_diff_delta *delta = git__malloc(sizeof(git_diff_delta));
@@ -46,7 +46,7 @@ fail:
 	return NULL;
 }
 
-static git_diff_delta *diff_delta__merge_like_cgit(
+git_diff_delta *git_diff__merge_like_cgit(
 	const git_diff_delta *a,
 	const git_diff_delta *b,
 	git_pool *pool)
@@ -67,23 +67,24 @@ static git_diff_delta *diff_delta__merge_like_cgit(
 
 	/* If one of the diffs is a conflict, just dup it */
 	if (b->status == GIT_DELTA_CONFLICTED)
-		return diff_delta__dup(b, pool);
+		return git_diff__delta_dup(b, pool);
 	if (a->status == GIT_DELTA_CONFLICTED)
-		return diff_delta__dup(a, pool);
+		return git_diff__delta_dup(a, pool);
 
 	/* if f2 == f3 or f2 is deleted, then just dup the 'a' diff */
 	if (b->status == GIT_DELTA_UNMODIFIED || a->status == GIT_DELTA_DELETED)
-		return diff_delta__dup(a, pool);
+		return git_diff__delta_dup(a, pool);
 
 	/* otherwise, base this diff on the 'b' diff */
-	if ((dup = diff_delta__dup(b, pool)) == NULL)
+	if ((dup = git_diff__delta_dup(b, pool)) == NULL)
 		return NULL;
 
 	/* If 'a' status is uninteresting, then we're done */
-	if (a->status == GIT_DELTA_UNMODIFIED)
+	if (a->status == GIT_DELTA_UNMODIFIED ||
+		a->status == GIT_DELTA_UNTRACKED ||
+		a->status == GIT_DELTA_UNREADABLE)
 		return dup;
 
-	assert(a->status != GIT_DELTA_UNMODIFIED);
 	assert(b->status != GIT_DELTA_UNMODIFIED);
 
 	/* A cgit exception is that the diff of a file that is only in the
@@ -108,48 +109,8 @@ static git_diff_delta *diff_delta__merge_like_cgit(
 	return dup;
 }
 
-static git_diff_delta *diff_delta__merge_like_cgit_reversed(
-	const git_diff_delta *a,
-	const git_diff_delta *b,
-	git_pool *pool)
-{
-	git_diff_delta *dup;
-
-	/* reversed version of above logic */
-
-	if (a->status == GIT_DELTA_CONFLICTED)
-		return diff_delta__dup(a, pool);
-	if (b->status == GIT_DELTA_CONFLICTED)
-		return diff_delta__dup(b, pool);
-
-	if (a->status == GIT_DELTA_UNMODIFIED)
-		return diff_delta__dup(b, pool);
-
-	if ((dup = diff_delta__dup(a, pool)) == NULL)
-		return NULL;
-
-	if (b->status == GIT_DELTA_UNMODIFIED || b->status == GIT_DELTA_UNTRACKED || b->status == GIT_DELTA_UNREADABLE)
-		return dup;
-
-	if (dup->status == GIT_DELTA_DELETED) {
-		if (b->status == GIT_DELTA_ADDED) {
-			dup->status = GIT_DELTA_UNMODIFIED;
-			dup->nfiles = 2;
-		}
-	} else {
-		dup->status = b->status;
-		dup->nfiles = b->nfiles;
-	}
-
-	git_oid_cpy(&dup->old_file.id, &b->old_file.id);
-	dup->old_file.mode  = b->old_file.mode;
-	dup->old_file.size  = b->old_file.size;
-	dup->old_file.flags = b->old_file.flags;
-
-	return dup;
-}
-
-int git_diff_merge(git_diff *onto, const git_diff *from)
+int git_diff__merge(
+	git_diff *onto, const git_diff *from, git_diff__merge_cb cb)
 {
 	int error = 0;
 	git_pool onto_pool;
@@ -185,15 +146,16 @@ int git_diff_merge(git_diff *onto, const git_diff *from)
 			STRCMP_CASESELECT(ignore_case, o->old_file.path, f->old_file.path);
 
 		if (cmp < 0) {
-			delta = diff_delta__dup(o, &onto_pool);
+			delta = git_diff__delta_dup(o, &onto_pool);
 			i++;
 		} else if (cmp > 0) {
-			delta = diff_delta__dup(f, &onto_pool);
+			delta = git_diff__delta_dup(f, &onto_pool);
 			j++;
 		} else {
-			delta = reversed ?
-				diff_delta__merge_like_cgit_reversed(o, f, &onto_pool) :
-				diff_delta__merge_like_cgit(o, f, &onto_pool);
+			const git_diff_delta *left = reversed ? f : o;
+			const git_diff_delta *right = reversed ? o : f;
+
+			delta = cb(left, right, &onto_pool);
 			i++;
 			j++;
 		}
@@ -201,7 +163,7 @@ int git_diff_merge(git_diff *onto, const git_diff *from)
 		/* the ignore rules for the target may not match the source
 		 * or the result of a merged delta could be skippable...
 		 */
-		if (git_diff_delta__should_skip(&onto->opts, delta)) {
+		if (delta && git_diff_delta__should_skip(&onto->opts, delta)) {
 			git__free(delta);
 			continue;
 		}
@@ -232,6 +194,11 @@ int git_diff_merge(git_diff *onto, const git_diff *from)
 	return error;
 }
 
+int git_diff_merge(git_diff *onto, const git_diff *from)
+{
+	return git_diff__merge(onto, from, git_diff__merge_like_cgit);
+}
+
 int git_diff_find_similar__hashsig_for_file(
 	void **out, const git_diff_file *f, const char *path, void *p)
 {
@@ -380,7 +347,7 @@ static int insert_delete_side_of_split(
 	git_diff *diff, git_vector *onto, const git_diff_delta *delta)
 {
 	/* make new record for DELETED side of split */
-	git_diff_delta *deleted = diff_delta__dup(delta, &diff->pool);
+	git_diff_delta *deleted = git_diff__delta_dup(delta, &diff->pool);
 	GITERR_CHECK_ALLOC(deleted);
 
 	deleted->status = GIT_DELTA_DELETED;

src/stash.c

@@ -22,6 +22,7 @@
 #include "signature.h"
 #include "iterator.h"
 #include "merge.h"
+#include "diff.h"
 
 static int create_error(int error, const char *msg)
 {
@@ -292,6 +293,25 @@ cleanup:
 	return error;
 }
 
+static git_diff_delta *stash_delta_merge(
+	const git_diff_delta *a,
+	const git_diff_delta *b,
+	git_pool *pool)
+{
+	/* Special case for stash: if a file is deleted in the index, but exists
+	 * in the working tree, we need to stash the workdir copy for the workdir.
+	 */
+	if (a->status == GIT_DELTA_DELETED && b->status == GIT_DELTA_UNTRACKED) {
+		git_diff_delta *dup = git_diff__delta_dup(b, pool);
+
+		if (dup)
+			dup->status = GIT_DELTA_MODIFIED;
+		return dup;
+	}
+
+	return git_diff__merge_like_cgit(a, b, pool);
+}
+
 static int build_workdir_tree(
 	git_tree **tree_out,
 	git_index *index,
@@ -299,17 +319,19 @@ static int build_workdir_tree(
 {
 	git_repository *repo = git_index_owner(index);
 	git_tree *b_tree = NULL;
-	git_diff *diff = NULL;
+	git_diff *diff = NULL, *idx_to_wd = NULL;
 	git_diff_options opts = GIT_DIFF_OPTIONS_INIT;
 	struct stash_update_rules data = {0};
 	int error;
 
-	opts.flags = GIT_DIFF_IGNORE_SUBMODULES;
+	opts.flags = GIT_DIFF_IGNORE_SUBMODULES | GIT_DIFF_INCLUDE_UNTRACKED;
 
 	if ((error = git_commit_tree(&b_tree, b_commit)) < 0)
 		goto cleanup;
 
-	if ((error = git_diff_tree_to_workdir(&diff, repo, b_tree, &opts)) < 0)
+	if ((error = git_diff_tree_to_index(&diff, repo, b_tree, index, &opts)) < 0 ||
+		(error = git_diff_index_to_workdir(&idx_to_wd, repo, index, &opts)) < 0 ||
+		(error = git_diff__merge(diff, idx_to_wd, stash_delta_merge)) < 0)
 		goto cleanup;
 
 	data.include_changed = true;
@@ -320,6 +342,7 @@ static int build_workdir_tree(
 	error = build_tree_from_index(tree_out, index);
 
 cleanup:
+	git_diff_free(idx_to_wd);
 	git_diff_free(diff);
 	git_tree_free(b_tree);
 

tests/stash/foreach.c

@@ -69,16 +69,16 @@ void test_stash_foreach__enumerating_a_empty_repository_doesnt_fail(void)
 void test_stash_foreach__can_enumerate_a_repository(void)
 {
 	char *oids_default[] = {
-		"1d91c842a7cdfc25872b3a763e5c31add8816c25", NULL };
+		"493568b7a2681187aaac8a58d3f1eab1527cba84", NULL };
 
 	char *oids_untracked[] = {
 		"7f89a8b15c878809c5c54d1ff8f8c9674154017b",
-		"1d91c842a7cdfc25872b3a763e5c31add8816c25", NULL };
+		"493568b7a2681187aaac8a58d3f1eab1527cba84", NULL };
 
 	char *oids_ignored[] = {
 		"c95599a8fef20a7e57582c6727b1a0d02e0a5828",
 		"7f89a8b15c878809c5c54d1ff8f8c9674154017b",
-		"1d91c842a7cdfc25872b3a763e5c31add8816c25", NULL };
+		"493568b7a2681187aaac8a58d3f1eab1527cba84", NULL };
 
 	cl_git_pass(git_repository_init(&repo, REPO_NAME, 0));
 
@@ -96,9 +96,7 @@ void test_stash_foreach__can_enumerate_a_repository(void)
 	cl_git_pass(git_stash_foreach(repo, callback_cb, &data));
 	cl_assert_equal_i(1, data.invokes);
 
-	data.oids = oids_untracked;
-	data.invokes = 0;
-
+	/* ensure stash_foreach operates with INCLUDE_UNTRACKED */
 	cl_git_pass(git_stash_save(
 		&stash_tip_oid,
 		repo,
@@ -106,12 +104,13 @@ void test_stash_foreach__can_enumerate_a_repository(void)
 		NULL,
 		GIT_STASH_INCLUDE_UNTRACKED));
 
+	data.oids = oids_untracked;
+	data.invokes = 0;
+
 	cl_git_pass(git_stash_foreach(repo, callback_cb, &data));
 	cl_assert_equal_i(2, data.invokes);
 
-	data.oids = oids_ignored;
-	data.invokes = 0;
-
+	/* ensure stash_foreach operates with INCLUDE_IGNORED */
 	cl_git_pass(git_stash_save(
 		&stash_tip_oid,
 		repo,
@@ -119,6 +118,9 @@ void test_stash_foreach__can_enumerate_a_repository(void)
 		NULL,
 		GIT_STASH_INCLUDE_IGNORED));
 
+	data.oids = oids_ignored;
+	data.invokes = 0;
+
 	cl_git_pass(git_stash_foreach(repo, callback_cb, &data));
 	cl_assert_equal_i(3, data.invokes);
 }

tests/stash/save.c

@@ -100,12 +100,18 @@ $ git status --short
 	assert_blob_oid("refs/stash:how", "e6d64adb2c7f3eb8feb493b556cc8070dca379a3");	/* not so small and */
 	assert_blob_oid("refs/stash:who", "a0400d4954659306a976567af43125a0b1aa8595");	/* funky world */
 	assert_blob_oid("refs/stash:when", NULL);
+	assert_blob_oid("refs/stash:why", "88c2533e21f098b89c91a431d8075cbdbe422a51"); /* would anybody use stash? */
+	assert_blob_oid("refs/stash:where", "e3d6434ec12eb76af8dfa843a64ba6ab91014a0b"); /* .... */
+	assert_blob_oid("refs/stash:.gitignore", "ac4d88de61733173d9959e4b77c69b9f17a00980");
 	assert_blob_oid("refs/stash:just.ignore", NULL);
 
 	assert_blob_oid("refs/stash^2:what", "dd7e1c6f0fefe118f0b63d9f10908c460aa317a6");	/* goodbye */
 	assert_blob_oid("refs/stash^2:how", "e6d64adb2c7f3eb8feb493b556cc8070dca379a3");	/* not so small and */
 	assert_blob_oid("refs/stash^2:who", "cc628ccd10742baea8241c5924df992b5c019f71");	/* world */
 	assert_blob_oid("refs/stash^2:when", NULL);
+	assert_blob_oid("refs/stash^2:why", "88c2533e21f098b89c91a431d8075cbdbe422a51"); /* would anybody use stash? */
+	assert_blob_oid("refs/stash^2:where", "e08f7fbb9a42a0c5367cf8b349f1f08c3d56bd72"); /* ???? */
+	assert_blob_oid("refs/stash^2:.gitignore", "ac4d88de61733173d9959e4b77c69b9f17a00980");
 	assert_blob_oid("refs/stash^2:just.ignore", NULL);
 
 	assert_blob_oid("refs/stash^3", NULL);
@@ -243,11 +249,13 @@ void test_stash_save__cannot_stash_when_there_are_no_local_change(void)
 	cl_git_pass(git_repository_index(&index, repo));
 
 	/*
-	 * 'what' and 'who' are being committed.
-	 * 'when' remain untracked.
+	 * 'what', 'where' and 'who' are being committed.
+	 * 'when' remains untracked.
 	 */
 	cl_git_pass(git_index_add_bypath(index, "what"));
+	cl_git_pass(git_index_add_bypath(index, "where"));
 	cl_git_pass(git_index_add_bypath(index, "who"));
+
 	cl_repo_commit_from_index(NULL, repo, signature, 0, "Initial commit");
 	git_index_free(index);
 

tests/stash/stash_helpers.c

@@ -26,12 +26,17 @@ void setup_stash(git_repository *repo, git_signature *signature)
 	cl_git_rewritefile("stash/what", "goodbye\n");			/* dd7e1c6f0fefe118f0b63d9f10908c460aa317a6 */
 	cl_git_rewritefile("stash/how", "not so small and\n");	/* e6d64adb2c7f3eb8feb493b556cc8070dca379a3 */
 	cl_git_rewritefile("stash/who", "funky world\n");		/* a0400d4954659306a976567af43125a0b1aa8595 */
+	cl_git_mkfile("stash/why", "would anybody use stash?\n"); /* 88c2533e21f098b89c91a431d8075cbde422a51 */
+	cl_git_mkfile("stash/where", "????\n");					/* e08f7fbb9a42a0c5367cf8b349f1f08c3d56bd72 */
 
 	cl_git_pass(git_index_add_bypath(index, "what"));
 	cl_git_pass(git_index_add_bypath(index, "how"));
+	cl_git_pass(git_index_add_bypath(index, "why"));
+	cl_git_pass(git_index_add_bypath(index, "where"));
 	cl_git_pass(git_index_write(index));
 
 	cl_git_rewritefile("stash/what", "see you later\n");	/* bc99dc98b3eba0e9157e94769cd4d49cb49de449 */
+	cl_git_mkfile("stash/where", "....\n");					/* e3d6434ec12eb76af8dfa843a64ba6ab91014a0b */
 
 	git_index_free(index);
 }