All of lore.kernel.org
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: git@vger.kernel.org
Cc: jrnieder@gmail.com, Elijah Newren <newren@gmail.com>
Subject: [RFC PATCH 4/7] merge-recursive: fix assumption that head tree being merged is HEAD
Date: Sat,  2 Jun 2018 23:58:07 -0700	[thread overview]
Message-ID: <20180603065810.23841-5-newren@gmail.com> (raw)
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


  parent reply	other threads:[~2018-06-03  6:58 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-03  6:58 [RFC PATCH 0/7] merge requirement: index matches head Elijah Newren
2018-06-03  6:58 ` [RFC PATCH 1/7] t6044: verify that merges expected to abort actually abort Elijah Newren
2018-06-03  6:58 ` [RFC PATCH 2/7] t6044: add a testcase for index matching head, when head doesn't match HEAD Elijah Newren
2018-06-03  6:58 ` [RFC PATCH 3/7] merge-recursive: make sure when we say we abort that we actually abort Elijah Newren
2018-06-03  6:58 ` Elijah Newren [this message]
2018-06-03 13:52   ` [RFC PATCH 4/7] merge-recursive: fix assumption that head tree being merged is HEAD Ramsay Jones
2018-06-03 23:37     ` brian m. carlson
2018-06-04  2:26       ` Ramsay Jones
2018-06-04  3:19   ` Junio C Hamano
2018-06-05  7:14     ` Elijah Newren
2018-06-11 16:15       ` Elijah Newren
2018-06-03  6:58 ` [RFC PATCH 5/7] t6044: add more testcases with staged changes before a merge is invoked Elijah Newren
2018-06-03  6:58 ` [RFC PATCH 6/7] merge-recursive: enforce rule that index matches head before merging Elijah Newren
2018-06-03  6:58 ` [RFC PATCH 7/7] merge: fix misleading pre-merge check documentation Elijah Newren
2018-06-07  5:27   ` Elijah Newren
2018-07-01  1:24 ` [PATCH v2 0/9] Fix merge issues with index not matching HEAD Elijah Newren
2018-07-01  1:24   ` [PATCH v2 1/9] read-cache.c: move index_has_changes() from merge.c Elijah Newren
2018-07-01  1:24   ` [PATCH v2 2/9] index_has_changes(): avoid assuming operating on the_index Elijah Newren
2018-07-03 19:44     ` Junio C Hamano
2018-07-01  1:24   ` [PATCH v2 3/9] t6044: verify that merges expected to abort actually abort Elijah Newren
2018-07-01  1:24   ` [PATCH v2 4/9] t6044: add a testcase for index matching head, when head doesn't match HEAD Elijah Newren
2018-07-10 20:39     ` SZEDER Gábor
2018-07-11  3:09       ` [RFC PATCH 2/7] " Elijah Newren
2018-07-01  1:24   ` [PATCH v2 5/9] merge-recursive: make sure when we say we abort that we actually abort Elijah Newren
2018-07-01  1:25   ` [PATCH v2 6/9] merge-recursive: fix assumption that head tree being merged is HEAD Elijah Newren
2018-07-03 19:57     ` Junio C Hamano
2018-07-01  1:25   ` [PATCH v2 7/9] t6044: add more testcases with staged changes before a merge is invoked Elijah Newren
2018-07-01  1:25   ` [PATCH v2 8/9] merge-recursive: enforce rule that index matches head before merging Elijah Newren
2018-07-03 20:05     ` Junio C Hamano
2018-07-01  1:25   ` [PATCH v2 9/9] merge: fix misleading pre-merge check documentation Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180603065810.23841-5-newren@gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.