git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[RFC PATCH 4/7] merge-recursive: fix assumption that head tree being merged is HEAD

From
Elijah Newren <newren@gmail.com>
Date
Jun 3, 2018, 06:58 UTC
Message-ID
<20180603065810.23841-5-newren@gmail.com>
In-Reply-To
<20180603065810.23841-1-newren@gmail.com>

`git merge-recursive` does a three-way merge between user-specified trees base, head, and remote. Since the user is allowed to specify head, we can not necesarily assume that head == HEAD.

We modify index_has_changes() to take an extra argument specifying the tree to compare the index to. If NULL, it will compare to HEAD. We then use this from merge-recursive to make sure we compare to the user-specified head.

Signed-off-by: Elijah Newren <newren@gmail.com>
---

I'm really unsure where the index_has_changes() declaration should go; I stuck it in tree.h, but is there a better spot?

 builtin/am.c                             |  6 +++---
 cache.h                                  |  8 --------
 merge-recursive.c                        |  2 +-
 merge.c                                  | 10 ++++++----
 t/t6044-merge-unrelated-index-changes.sh |  2 +-
 tree.h                                   |  8 ++++++++
 6 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 2fc2d1e82c..c5b76cd095 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1763,7 +1763,7 @@ static void am_run(struct am_state *state, int resume)
 
 	refresh_and_write_cache();
 
-	if (index_has_changes(&sb)) {
+	if (index_has_changes(&sb, NULL)) {
 		write_state_bool(state, "dirtyindex", 1);
 		die(_("Dirty index: cannot apply patches (dirty: %s)"), sb.buf);
 	}
@@ -1820,7 +1820,7 @@ static void am_run(struct am_state *state, int resume)
 			 * Applying the patch to an earlier tree and merging
 			 * the result may have produced the same tree as ours.
 			 */
-			if (!apply_status && !index_has_changes(NULL)) {
+			if (!apply_status && !index_has_changes(NULL, NULL)) {
 				say(state, stdout, _("No changes -- Patch already applied."));
 				goto next;
 			}
@@ -1878,7 +1878,7 @@ static void am_resolve(struct am_state *state)
 
 	say(state, stdout, _("Applying: %.*s"), linelen(state->msg), state->msg);
 
-	if (!index_has_changes(NULL)) {
+	if (!index_has_changes(NULL, NULL)) {
 		printf_ln(_("No changes - did you forget to use 'git add'?\n"
 			"If there is nothing left to stage, chances are that something else\n"
 			"already introduced the same changes; you might want to skip this patch."));
diff --git a/cache.h b/cache.h
index 89a107a7f7..439b9d9f6e 100644
--- a/cache.h
+++ b/cache.h
@@ -634,14 +634,6 @@ extern int discard_index(struct index_state *);
 extern void move_index_extensions(struct index_state *dst, struct index_state *src);
 extern int unmerged_index(const struct index_state *);
 
-/**
- * Returns 1 if the index differs from HEAD, 0 otherwise. When on an unborn
- * branch, returns 1 if there are entries in the index, 0 otherwise. If an
- * strbuf is provided, the space-separated list of files that differ will be
- * appended to it.
- */
-extern int index_has_changes(struct strbuf *sb);
-
 extern int verify_path(const char *path, unsigned mode);
 extern int strcmp_offset(const char *s1, const char *s2, size_t *first_change);
 extern int index_dir_exists(struct index_state *istate, const char *name, int namelen);
diff --git a/merge-recursive.c b/merge-recursive.c
index b3deb7b182..762aa087d0 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -3263,7 +3263,7 @@ int merge_trees(struct merge_options *o,
 	if (oid_eq(&common->object.oid, &merge->object.oid)) {
 		struct strbuf sb = STRBUF_INIT;
 
-		if (!o->call_depth && index_has_changes(&sb)) {
+		if (!o->call_depth && index_has_changes(&sb, head)) {
 			err(o, _("Your local changes to the following files would be overwritten by merge:\n  %s"),
 			    sb.buf);
 			return -1;
diff --git a/merge.c b/merge.c
index 0783858739..965d287646 100644
--- a/merge.c
+++ b/merge.c
@@ -14,19 +14,21 @@ static const char *merge_argument(struct commit *commit)
 	return oid_to_hex(commit ? &commit->object.oid : the_hash_algo->empty_tree);
 }
 
-int index_has_changes(struct strbuf *sb)
+int index_has_changes(struct strbuf *sb, struct tree *tree)
 {
-	struct object_id head;
+	struct object_id cmp;
 	int i;
 
-	if (!get_oid_tree("HEAD", &head)) {
+	if (tree)
+		cmp = tree->object.oid;
+	if (tree || !get_oid_tree("HEAD", &cmp)) {
 		struct diff_options opt;
 
 		diff_setup(&opt);
 		opt.flags.exit_with_status = 1;
 		if (!sb)
 			opt.flags.quick = 1;
-		do_diff_cache(&head, &opt);
+		do_diff_cache(&cmp, &opt);
 		diffcore_std(&opt);
 		for (i = 0; sb && i < diff_queued_diff.nr; i++) {
 			if (i)
diff --git a/t/t6044-merge-unrelated-index-changes.sh b/t/t6044-merge-unrelated-index-changes.sh
index 3876cfa4fa..1d43712c52 100755
--- a/t/t6044-merge-unrelated-index-changes.sh
+++ b/t/t6044-merge-unrelated-index-changes.sh
@@ -126,7 +126,7 @@ test_expect_success 'recursive, when merge branch matches merge base' '
 	test_path_is_missing .git/MERGE_HEAD
 '
 
-test_expect_failure 'merge-recursive, when index==head but head!=HEAD' '
+test_expect_success 'merge-recursive, when index==head but head!=HEAD' '
 	git reset --hard &&
 	git checkout C^0 &&
 
diff --git a/tree.h b/tree.h
index e2a80be4ef..2e1d8ea720 100644
--- a/tree.h
+++ b/tree.h
@@ -37,4 +37,12 @@ extern int read_tree_recursive(struct tree *tree,
 extern int read_tree(struct tree *tree, int stage, struct pathspec *pathspec,
 		     struct index_state *istate);
 
+/**
+ * Returns 1 if the index differs from tree, 0 otherwise.  If tree is NULL,
+ * compares to HEAD.  If tree is NULL and on an unborn branch, returns 1 if
+ * there are entries in the index, 0 otherwise. If an strbuf is provided,
+ * the space-separated list of files that differ will be appended to it.
+ */
+extern int index_has_changes(struct strbuf *sb, struct tree *tree);
+
 #endif /* TREE_H */
-- 
2.18.0.rc0.49.g3c08dc0fef
Previous: Elijah NewrenNext: Ramsay Jones
Message 8 of 30 in “merge requirement: index matches head”
  1. 0/7 merge requirement: index matches headElijah Newren, Jun 3, 2018
  2. 3/7 merge-recursive: make sure when we say we abort that we actually abortElijah Newren, Jun 3, 2018
  3. 2/7 t6044: add a testcase for index matching head, when head doesn't match HEADElijah Newren, Jun 3, 2018
  4. 5/7 t6044: add more testcases with staged changes before a merge is invokedElijah Newren, Jun 3, 2018
  5. 6/7 merge-recursive: enforce rule that index matches head before mergingElijah Newren, Jun 3, 2018
  6. 7/7 merge: fix misleading pre-merge check documentationElijah Newren, Jun 3, 2018
  7. Elijah NewrenJun 7, 2018
  8. 4/7 merge-recursive: fix assumption that head tree being merged is HEADElijah Newren, Jun 3, 2018
  9. Ramsay JonesJun 3, 2018
  10. brian m. carlsonJun 3, 2018
  11. Ramsay JonesJun 4, 2018
  12. Junio C HamanoJun 4, 2018
  13. Elijah NewrenJun 5, 2018
  14. Elijah NewrenJun 11, 2018
  15. 1/7 t6044: verify that merges expected to abort actually abortElijah Newren, Jun 3, 2018
  16. 0/9 Fix merge issues with index not matching HEADElijah Newren, Jul 1, 2018
  17. 1/9 read-cache.c: move index_has_changes() from merge.cElijah Newren, Jul 1, 2018
  18. 2/9 index_has_changes(): avoid assuming operating on the_indexElijah Newren, Jul 1, 2018
  19. Junio C HamanoJul 3, 2018
  20. 3/9 t6044: verify that merges expected to abort actually abortElijah Newren, Jul 1, 2018
  21. 4/9 t6044: add a testcase for index matching head, when head doesn't match HEADElijah Newren, Jul 1, 2018
  22. SZEDER GáborJul 10, 2018
  23. Elijah NewrenJul 11, 2018
  24. 8/9 merge-recursive: enforce rule that index matches head before mergingElijah Newren, Jul 1, 2018
  25. Junio C HamanoJul 3, 2018
  26. 5/9 merge-recursive: make sure when we say we abort that we actually abortElijah Newren, Jul 1, 2018
  27. 6/9 merge-recursive: fix assumption that head tree being merged is HEADElijah Newren, Jul 1, 2018
  28. Junio C HamanoJul 3, 2018
  29. 7/9 t6044: add more testcases with staged changes before a merge is invokedElijah Newren, Jul 1, 2018
  30. 9/9 merge: fix misleading pre-merge check documentationElijah Newren, Jul 1, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.