From 331a1838b26c3032bec27b66307a9de9b3b11509 Mon Sep 17 00:00:00 2001 From: Eyvind Bernhardsen Date: Fri, 2 Jul 2010 21:20:48 +0200 Subject: Try normalizing files to avoid delete/modify conflicts when merging If a file is modified due to normalization on one branch, and deleted on another, a merge of the two branches will result in a delete/modify conflict for that file even if it is otherwise unchanged. Try to avoid the conflict by normalizing and comparing the "base" file and the modified file when their sha1s differ. If they compare equal, the file is considered unmodified and is deleted. Signed-off-by: Eyvind Bernhardsen Signed-off-by: Junio C Hamano --- merge-recursive.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 206c103635..5ad8fc9e7e 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1056,6 +1056,53 @@ static unsigned char *stage_sha(const unsigned char *sha, unsigned mode) return (is_null_sha1(sha) || mode == 0) ? NULL: (unsigned char *)sha; } +static int read_sha1_strbuf(const unsigned char *sha1, struct strbuf *dst) +{ + void *buf; + enum object_type type; + unsigned long size; + buf = read_sha1_file(sha1, &type, &size); + if (!buf) + return error("cannot read object %s", sha1_to_hex(sha1)); + if (type != OBJ_BLOB) { + free(buf); + return error("object %s is not a blob", sha1_to_hex(sha1)); + } + strbuf_attach(dst, buf, size, size + 1); + return 0; +} + +static int blob_unchanged(const unsigned char *o_sha, + const unsigned char *a_sha, + const char *path) +{ + struct strbuf o = STRBUF_INIT; + struct strbuf a = STRBUF_INIT; + int ret = 0; /* assume changed for safety */ + + if (sha_eq(o_sha, a_sha)) + return 1; + if (!merge_renormalize) + return 0; + + assert(o_sha && a_sha); + if (read_sha1_strbuf(o_sha, &o) || read_sha1_strbuf(a_sha, &a)) + goto error_return; + /* + * Note: binary | is used so that both renormalizations are + * performed. Comparison can be skipped if both files are + * unchanged since their sha1s have already been compared. + */ + if (renormalize_buffer(path, o.buf, o.len, &o) | + renormalize_buffer(path, a.buf, o.len, &a)) + ret = (o.len == a.len && !memcmp(o.buf, a.buf, o.len)); + +error_return: + strbuf_release(&o); + strbuf_release(&a); + return ret; +} + /* Per entry merge function */ static int process_entry(struct merge_options *o, const char *path, struct stage_data *entry) @@ -1075,8 +1122,8 @@ static int process_entry(struct merge_options *o, if (o_sha && (!a_sha || !b_sha)) { /* Case A: Deleted in one */ if ((!a_sha && !b_sha) || - (sha_eq(a_sha, o_sha) && !b_sha) || - (!a_sha && sha_eq(b_sha, o_sha))) { + (!b_sha && blob_unchanged(o_sha, a_sha, path)) || + (!a_sha && blob_unchanged(o_sha, b_sha, path))) { /* Deleted in both or deleted in one and * unchanged in the other */ if (a_sha) -- cgit v1.2.1 From 3e7589b7b3ff7a7aa93bec39c40603e370c51317 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Thu, 5 Aug 2010 06:13:49 -0500 Subject: merge-trees: push choice to renormalize away from low level The merge machinery decides whether to resmudge and clean relevant entries based on the global merge_renormalize setting, which is set by "git merge" based on its configuration (and left alone by other commands). A nicer interface would make that decision a parameter to merge_trees so callers would pass in a choice made on a call-by-call basis. Start by making blob_unchanged stop examining the merge_renormalize global. In other words, this change is a trivial no-op, but it brings us closer to something good. Cc: Eyvind Bernhardsen Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- merge-recursive.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 5ad8fc9e7e..2b55fc27dd 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1074,7 +1074,7 @@ static int read_sha1_strbuf(const unsigned char *sha1, struct strbuf *dst) static int blob_unchanged(const unsigned char *o_sha, const unsigned char *a_sha, - const char *path) + int renormalize, const char *path) { struct strbuf o = STRBUF_INIT; struct strbuf a = STRBUF_INIT; @@ -1082,7 +1082,7 @@ static int blob_unchanged(const unsigned char *o_sha, if (sha_eq(o_sha, a_sha)) return 1; - if (!merge_renormalize) + if (!renormalize) return 0; assert(o_sha && a_sha); @@ -1112,6 +1112,7 @@ static int process_entry(struct merge_options *o, print_index_entry("\tpath: ", entry); */ int clean_merge = 1; + int normalize = merge_renormalize; unsigned o_mode = entry->stages[1].mode; unsigned a_mode = entry->stages[2].mode; unsigned b_mode = entry->stages[3].mode; @@ -1122,8 +1123,8 @@ static int process_entry(struct merge_options *o, if (o_sha && (!a_sha || !b_sha)) { /* Case A: Deleted in one */ if ((!a_sha && !b_sha) || - (!b_sha && blob_unchanged(o_sha, a_sha, path)) || - (!a_sha && blob_unchanged(o_sha, b_sha, path))) { + (!b_sha && blob_unchanged(o_sha, a_sha, normalize, path)) || + (!a_sha && blob_unchanged(o_sha, b_sha, normalize, path))) { /* Deleted in both or deleted in one and * unchanged in the other */ if (a_sha) -- cgit v1.2.1 From 1bc0ab7cd13af796b4ba1a5fc3ede9e92078aee4 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Thu, 5 Aug 2010 06:15:32 -0500 Subject: merge-trees: let caller decide whether to renormalize Add a "renormalize" option to struct merge_options so callers can decide on a case-by-case basis whether the merge is likely to have overlapped with a change in smudge/clean rules. The option defaults to the global merge_renormalize setting for now. No change in behavior intended. Cc: Eyvind Bernhardsen Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- merge-recursive.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 2b55fc27dd..8a49844c90 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1112,7 +1112,7 @@ static int process_entry(struct merge_options *o, print_index_entry("\tpath: ", entry); */ int clean_merge = 1; - int normalize = merge_renormalize; + int normalize = o->renormalize; unsigned o_mode = entry->stages[1].mode; unsigned a_mode = entry->stages[2].mode; unsigned b_mode = entry->stages[3].mode; @@ -1484,6 +1484,7 @@ void init_merge_options(struct merge_options *o) o->buffer_output = 1; o->diff_rename_limit = -1; o->merge_rename_limit = -1; + o->renormalize = merge_renormalize; git_config(merge_recursive_config, o); if (getenv("GIT_MERGE_VERBOSITY")) o->verbosity = -- cgit v1.2.1 From 73cf7f713da4fc797e2393a9e490ad4ec9466c53 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Thu, 5 Aug 2010 06:17:38 -0500 Subject: ll-merge: make flag easier to populate ll_merge() takes its options in a flag word, which has a few advantages: - options flags can be cheaply passed around in registers, while an option struct passed by pointer cannot; - callers can easily pass 0 without trouble for no options, while an option struct passed by value would not allow that. The downside is that code to populate and access the flag word can be somewhat opaque. Mitigate that with a few macros. Cc: Avery Pennarun Cc: Bert Wesarg Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- merge-recursive.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 8a49844c90..c0c9f0ccc4 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -647,7 +647,8 @@ static int merge_3way(struct merge_options *o, merge_status = ll_merge(result_buf, a->path, &orig, base_name, &src1, name1, &src2, name2, - (!!o->call_depth) | (favor << 1)); + ((o->call_depth ? LL_OPT_VIRTUAL_ANCESTOR : 0) | + create_ll_flag(favor))); free(name1); free(name2); -- cgit v1.2.1 From 18b037a5b61532cba7f19efdb2e75c258d87d3d7 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Thu, 5 Aug 2010 06:24:58 -0500 Subject: ll-merge: let caller decide whether to renormalize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a “renormalize” bit to the ll-merge options word so callers can decide on a case-by-case basis whether the merge is likely to have overlapped with a change in smudge/clean rules. This reveals a few commands that have not been taking that situation into account, though it does not fix them. No functional change intended. Cc: Eyvind Bernhardsen Improved-by: Junio C Hamano Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- merge-recursive.c | 1 + 1 file changed, 1 insertion(+) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index c0c9f0ccc4..23f7a4d139 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -648,6 +648,7 @@ static int merge_3way(struct merge_options *o, merge_status = ll_merge(result_buf, a->path, &orig, base_name, &src1, name1, &src2, name2, ((o->call_depth ? LL_OPT_VIRTUAL_ANCESTOR : 0) | + (o->renormalize ? LL_OPT_RENORMALIZE : 0) | create_ll_flag(favor))); free(name1); -- cgit v1.2.1 From 7610fa57e63b0acc0a66717fc2d85755634db591 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Thu, 5 Aug 2010 06:32:41 -0500 Subject: merge-recursive --renormalize Teach "git merge-recursive" a --renormalize option to enable the merge.renormalize configuration. The --no-renormalize option can be used to override it in the negative. So in the future, you might be able to, e.g.: git checkout -m -Xrenormalize otherbranch or git revert -Xrenormalize otherpatch or git pull --rebase -Xrenormalize The bad part: merge.renormalize is still not honored for most commands. And it reveals lots of places that -X has not been plumbed in (so we get "git merge -Xrenormalize" but not much else). NEEDSWORK: tests Cc: Eyvind Bernhardsen Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- merge-recursive.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'merge-recursive.c') diff --git a/merge-recursive.c b/merge-recursive.c index 23f7a4d139..762b5494d2 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1486,7 +1486,7 @@ void init_merge_options(struct merge_options *o) o->buffer_output = 1; o->diff_rename_limit = -1; o->merge_rename_limit = -1; - o->renormalize = merge_renormalize; + o->renormalize = 0; git_config(merge_recursive_config, o); if (getenv("GIT_MERGE_VERBOSITY")) o->verbosity = -- cgit v1.2.1