* [PATCH 0/5] merge-tree fix-ups
@ 2012-12-26 23:03 Junio C Hamano
2012-12-26 23:03 ` [PATCH 1/5] Which merge_file() function do you mean? Junio C Hamano
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Junio C Hamano @ 2012-12-26 23:03 UTC (permalink / raw)
To: git
This is a small preparatory step to build a new merge strategy based
on the disused merge-tree proof-of-concept code. It starts with a
small set of code clean-up patches and ends with two bugfixes, at
least for now.
Junio C Hamano (5):
Which merge_file() function do you mean?
merge-tree: lose unused "flags" from merge_list
merge-tree: lose unused "resolve_directories"
merge-tree: add comments to clarify what these functions are doing
merge-tree: fix d/f conflicts
Makefile | 2 +-
builtin/merge-index.c | 4 +-
builtin/merge-tree.c | 92 +++++++++++++++++++++++--------------
merge-blobs.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++
merge-blobs.h | 8 ++++
merge-file.c | 124 --------------------------------------------------
merge-file.h | 7 ---
merge-recursive.c | 6 +--
t/t4300-merge-tree.sh | 44 ++++++++++++++++++
9 files changed, 239 insertions(+), 172 deletions(-)
create mode 100644 merge-blobs.c
create mode 100644 merge-blobs.h
delete mode 100644 merge-file.c
delete mode 100644 merge-file.h
--
1.8.1.rc3.356.g686f81c
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/5] Which merge_file() function do you mean?
2012-12-26 23:03 [PATCH 0/5] merge-tree fix-ups Junio C Hamano
@ 2012-12-26 23:03 ` Junio C Hamano
2012-12-26 23:03 ` [PATCH 2/5] merge-tree: lose unused "flags" from merge_list Junio C Hamano
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2012-12-26 23:03 UTC (permalink / raw)
To: git
There are two different static functions and one global function,
all of them called "merge_file()", with different signatures and
purposes. Rename them all to reduce confusion in "git grep" output:
* Rename the static one in merge-index to "merge_one_path(const char
*path)" as that function is about asking an external command to
resolve conflicts in one path.
* Rename the global one in merge-file.c that is only used by
merge-tree to "merge_blobs()", as the function takes three blobs and
returns the merged result only in-core, without doing anything to
the filesystem.
* Rename the one in merge-recursive to "merge_one_file()", just to be
fair.
Also rename merge-file.[ch] to merge-blobs.[ch].
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Makefile | 2 +-
builtin/merge-index.c | 4 +-
builtin/merge-tree.c | 4 +-
merge-blobs.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++
merge-blobs.h | 8 ++++
merge-file.c | 124 --------------------------------------------------
merge-file.h | 7 ---
merge-recursive.c | 6 +--
8 files changed, 140 insertions(+), 139 deletions(-)
create mode 100644 merge-blobs.c
create mode 100644 merge-blobs.h
delete mode 100644 merge-file.c
delete mode 100644 merge-file.h
diff --git a/Makefile b/Makefile
index 4ad6fbd..26ec519 100644
--- a/Makefile
+++ b/Makefile
@@ -765,7 +765,7 @@ LIB_OBJS += log-tree.o
LIB_OBJS += mailmap.o
LIB_OBJS += match-trees.o
LIB_OBJS += merge.o
-LIB_OBJS += merge-file.o
+LIB_OBJS += merge-blobs.o
LIB_OBJS += merge-recursive.o
LIB_OBJS += mergesort.o
LIB_OBJS += name-hash.o
diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index 2338832..be5e514 100644
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -42,7 +42,7 @@ static int merge_entry(int pos, const char *path)
return found;
}
-static void merge_file(const char *path)
+static void merge_one_path(const char *path)
{
int pos = cache_name_pos(path, strlen(path));
@@ -102,7 +102,7 @@ int cmd_merge_index(int argc, const char **argv, const char *prefix)
}
die("git merge-index: unknown option %s", arg);
}
- merge_file(arg);
+ merge_one_path(arg);
}
if (err && !quiet)
die("merge program failed");
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 897a563..ebd0d25 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -3,7 +3,7 @@
#include "xdiff-interface.h"
#include "blob.h"
#include "exec_cmd.h"
-#include "merge-file.h"
+#include "merge-blobs.h"
static const char merge_tree_usage[] = "git merge-tree <base-tree> <branch1> <branch2>";
static int resolve_directories = 1;
@@ -76,7 +76,7 @@ static void *result(struct merge_list *entry, unsigned long *size)
their = NULL;
if (entry)
their = entry->blob;
- return merge_file(path, base, our, their, size);
+ return merge_blobs(path, base, our, their, size);
}
static void *origin(struct merge_list *entry, unsigned long *size)
diff --git a/merge-blobs.c b/merge-blobs.c
new file mode 100644
index 0000000..57211bc
--- /dev/null
+++ b/merge-blobs.c
@@ -0,0 +1,124 @@
+#include "cache.h"
+#include "run-command.h"
+#include "xdiff-interface.h"
+#include "ll-merge.h"
+#include "blob.h"
+#include "merge-blobs.h"
+
+static int fill_mmfile_blob(mmfile_t *f, struct blob *obj)
+{
+ void *buf;
+ unsigned long size;
+ enum object_type type;
+
+ buf = read_sha1_file(obj->object.sha1, &type, &size);
+ if (!buf)
+ return -1;
+ if (type != OBJ_BLOB)
+ return -1;
+ f->ptr = buf;
+ f->size = size;
+ return 0;
+}
+
+static void free_mmfile(mmfile_t *f)
+{
+ free(f->ptr);
+}
+
+static void *three_way_filemerge(const char *path, mmfile_t *base, mmfile_t *our, mmfile_t *their, unsigned long *size)
+{
+ int merge_status;
+ mmbuffer_t res;
+
+ /*
+ * This function is only used by cmd_merge_tree, which
+ * does not respect the merge.conflictstyle option.
+ * There is no need to worry about a label for the
+ * common ancestor.
+ */
+ merge_status = ll_merge(&res, path, base, NULL,
+ our, ".our", their, ".their", NULL);
+ if (merge_status < 0)
+ return NULL;
+
+ *size = res.size;
+ return res.ptr;
+}
+
+static int common_outf(void *priv_, mmbuffer_t *mb, int nbuf)
+{
+ int i;
+ mmfile_t *dst = priv_;
+
+ for (i = 0; i < nbuf; i++) {
+ memcpy(dst->ptr + dst->size, mb[i].ptr, mb[i].size);
+ dst->size += mb[i].size;
+ }
+ return 0;
+}
+
+static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2)
+{
+ unsigned long size = f1->size < f2->size ? f1->size : f2->size;
+ void *ptr = xmalloc(size);
+ xpparam_t xpp;
+ xdemitconf_t xecfg;
+ xdemitcb_t ecb;
+
+ memset(&xpp, 0, sizeof(xpp));
+ xpp.flags = 0;
+ memset(&xecfg, 0, sizeof(xecfg));
+ xecfg.ctxlen = 3;
+ xecfg.flags = XDL_EMIT_COMMON;
+ ecb.outf = common_outf;
+
+ res->ptr = ptr;
+ res->size = 0;
+
+ ecb.priv = res;
+ return xdi_diff(f1, f2, &xpp, &xecfg, &ecb);
+}
+
+void *merge_blobs(const char *path, struct blob *base, struct blob *our, struct blob *their, unsigned long *size)
+{
+ void *res = NULL;
+ mmfile_t f1, f2, common;
+
+ /*
+ * Removed in either branch?
+ *
+ * NOTE! This depends on the caller having done the
+ * proper warning about removing a file that got
+ * modified in the other branch!
+ */
+ if (!our || !their) {
+ enum object_type type;
+ if (base)
+ return NULL;
+ if (!our)
+ our = their;
+ return read_sha1_file(our->object.sha1, &type, size);
+ }
+
+ if (fill_mmfile_blob(&f1, our) < 0)
+ goto out_no_mmfile;
+ if (fill_mmfile_blob(&f2, their) < 0)
+ goto out_free_f1;
+
+ if (base) {
+ if (fill_mmfile_blob(&common, base) < 0)
+ goto out_free_f2_f1;
+ } else {
+ if (generate_common_file(&common, &f1, &f2) < 0)
+ goto out_free_f2_f1;
+ }
+ res = three_way_filemerge(path, &common, &f1, &f2, size);
+ free_mmfile(&common);
+out_free_f2_f1:
+ free_mmfile(&f2);
+out_free_f1:
+ free_mmfile(&f1);
+out_no_mmfile:
+ return res;
+}
diff --git a/merge-blobs.h b/merge-blobs.h
new file mode 100644
index 0000000..62b569e
--- /dev/null
+++ b/merge-blobs.h
@@ -0,0 +1,8 @@
+#ifndef MERGE_BLOBS_H
+#define MERGE_BLOBS_H
+
+#include "blob.h"
+
+extern void *merge_blobs(const char *, struct blob *, struct blob *, struct blob *, unsigned long *);
+
+#endif /* MERGE_BLOBS_H */
diff --git a/merge-file.c b/merge-file.c
deleted file mode 100644
index 7845528..0000000
--- a/merge-file.c
+++ /dev/null
@@ -1,124 +0,0 @@
-#include "cache.h"
-#include "run-command.h"
-#include "xdiff-interface.h"
-#include "ll-merge.h"
-#include "blob.h"
-#include "merge-file.h"
-
-static int fill_mmfile_blob(mmfile_t *f, struct blob *obj)
-{
- void *buf;
- unsigned long size;
- enum object_type type;
-
- buf = read_sha1_file(obj->object.sha1, &type, &size);
- if (!buf)
- return -1;
- if (type != OBJ_BLOB)
- return -1;
- f->ptr = buf;
- f->size = size;
- return 0;
-}
-
-static void free_mmfile(mmfile_t *f)
-{
- free(f->ptr);
-}
-
-static void *three_way_filemerge(const char *path, mmfile_t *base, mmfile_t *our, mmfile_t *their, unsigned long *size)
-{
- int merge_status;
- mmbuffer_t res;
-
- /*
- * This function is only used by cmd_merge_tree, which
- * does not respect the merge.conflictstyle option.
- * There is no need to worry about a label for the
- * common ancestor.
- */
- merge_status = ll_merge(&res, path, base, NULL,
- our, ".our", their, ".their", NULL);
- if (merge_status < 0)
- return NULL;
-
- *size = res.size;
- return res.ptr;
-}
-
-static int common_outf(void *priv_, mmbuffer_t *mb, int nbuf)
-{
- int i;
- mmfile_t *dst = priv_;
-
- for (i = 0; i < nbuf; i++) {
- memcpy(dst->ptr + dst->size, mb[i].ptr, mb[i].size);
- dst->size += mb[i].size;
- }
- return 0;
-}
-
-static int generate_common_file(mmfile_t *res, mmfile_t *f1, mmfile_t *f2)
-{
- unsigned long size = f1->size < f2->size ? f1->size : f2->size;
- void *ptr = xmalloc(size);
- xpparam_t xpp;
- xdemitconf_t xecfg;
- xdemitcb_t ecb;
-
- memset(&xpp, 0, sizeof(xpp));
- xpp.flags = 0;
- memset(&xecfg, 0, sizeof(xecfg));
- xecfg.ctxlen = 3;
- xecfg.flags = XDL_EMIT_COMMON;
- ecb.outf = common_outf;
-
- res->ptr = ptr;
- res->size = 0;
-
- ecb.priv = res;
- return xdi_diff(f1, f2, &xpp, &xecfg, &ecb);
-}
-
-void *merge_file(const char *path, struct blob *base, struct blob *our, struct blob *their, unsigned long *size)
-{
- void *res = NULL;
- mmfile_t f1, f2, common;
-
- /*
- * Removed in either branch?
- *
- * NOTE! This depends on the caller having done the
- * proper warning about removing a file that got
- * modified in the other branch!
- */
- if (!our || !their) {
- enum object_type type;
- if (base)
- return NULL;
- if (!our)
- our = their;
- return read_sha1_file(our->object.sha1, &type, size);
- }
-
- if (fill_mmfile_blob(&f1, our) < 0)
- goto out_no_mmfile;
- if (fill_mmfile_blob(&f2, their) < 0)
- goto out_free_f1;
-
- if (base) {
- if (fill_mmfile_blob(&common, base) < 0)
- goto out_free_f2_f1;
- } else {
- if (generate_common_file(&common, &f1, &f2) < 0)
- goto out_free_f2_f1;
- }
- res = three_way_filemerge(path, &common, &f1, &f2, size);
- free_mmfile(&common);
-out_free_f2_f1:
- free_mmfile(&f2);
-out_free_f1:
- free_mmfile(&f1);
-out_no_mmfile:
- return res;
-}
diff --git a/merge-file.h b/merge-file.h
deleted file mode 100644
index 9b3b83a..0000000
--- a/merge-file.h
+++ /dev/null
@@ -1,7 +0,0 @@
-#ifndef MERGE_FILE_H
-#define MERGE_FILE_H
-
-extern void *merge_file(const char *path, struct blob *base, struct blob *our,
- struct blob *their, unsigned long *size);
-
-#endif
diff --git a/merge-recursive.c b/merge-recursive.c
index d882060..33ba5dc 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -976,7 +976,7 @@ merge_file_special_markers(struct merge_options *o,
return mfi;
}
-static struct merge_file_info merge_file(struct merge_options *o,
+static struct merge_file_info merge_file_one(struct merge_options *o,
const char *path,
const unsigned char *o_sha, int o_mode,
const unsigned char *a_sha, int a_mode,
@@ -1166,7 +1166,7 @@ static void conflict_rename_rename_1to2(struct merge_options *o,
struct merge_file_info mfi;
struct diff_filespec other;
struct diff_filespec *add;
- mfi = merge_file(o, one->path,
+ mfi = merge_file_one(o, one->path,
one->sha1, one->mode,
a->sha1, a->mode,
b->sha1, b->mode,
@@ -1450,7 +1450,7 @@ static int process_renames(struct merge_options *o,
ren1_dst, branch2);
if (o->call_depth) {
struct merge_file_info mfi;
- mfi = merge_file(o, ren1_dst, null_sha1, 0,
+ mfi = merge_file_one(o, ren1_dst, null_sha1, 0,
ren1->pair->two->sha1, ren1->pair->two->mode,
dst_other.sha1, dst_other.mode,
branch1, branch2);
--
1.8.1.rc3.356.g686f81c
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/5] merge-tree: lose unused "flags" from merge_list
2012-12-26 23:03 [PATCH 0/5] merge-tree fix-ups Junio C Hamano
2012-12-26 23:03 ` [PATCH 1/5] Which merge_file() function do you mean? Junio C Hamano
@ 2012-12-26 23:03 ` Junio C Hamano
2012-12-26 23:03 ` [PATCH 3/5] merge-tree: lose unused "resolve_directories" Junio C Hamano
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2012-12-26 23:03 UTC (permalink / raw)
To: git
Drop the unused field from the structure.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/merge-tree.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index ebd0d25..b61d811 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -12,8 +12,7 @@ struct merge_list {
struct merge_list *next;
struct merge_list *link; /* other stages for this object */
- unsigned int stage : 2,
- flags : 30;
+ unsigned int stage : 2;
unsigned int mode;
const char *path;
struct blob *blob;
--
1.8.1.rc3.356.g686f81c
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/5] merge-tree: lose unused "resolve_directories"
2012-12-26 23:03 [PATCH 0/5] merge-tree fix-ups Junio C Hamano
2012-12-26 23:03 ` [PATCH 1/5] Which merge_file() function do you mean? Junio C Hamano
2012-12-26 23:03 ` [PATCH 2/5] merge-tree: lose unused "flags" from merge_list Junio C Hamano
@ 2012-12-26 23:03 ` Junio C Hamano
2012-12-26 23:03 ` [PATCH 4/5] merge-tree: add comments to clarify what these functions are doing Junio C Hamano
2012-12-26 23:03 ` [PATCH 5/5] merge-tree: fix d/f conflicts Junio C Hamano
4 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2012-12-26 23:03 UTC (permalink / raw)
To: git
This option is always set; simplify.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/merge-tree.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index b61d811..95de162 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -6,7 +6,6 @@
#include "merge-blobs.h"
static const char merge_tree_usage[] = "git merge-tree <base-tree> <branch1> <branch2>";
-static int resolve_directories = 1;
struct merge_list {
struct merge_list *next;
@@ -198,8 +197,6 @@ static int unresolved_directory(const struct traverse_info *info, struct name_en
struct tree_desc t[3];
void *buf0, *buf1, *buf2;
- if (!resolve_directories)
- return 0;
p = n;
if (!p->mode) {
p++;
--
1.8.1.rc3.356.g686f81c
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/5] merge-tree: add comments to clarify what these functions are doing
2012-12-26 23:03 [PATCH 0/5] merge-tree fix-ups Junio C Hamano
` (2 preceding siblings ...)
2012-12-26 23:03 ` [PATCH 3/5] merge-tree: lose unused "resolve_directories" Junio C Hamano
@ 2012-12-26 23:03 ` Junio C Hamano
2012-12-26 23:03 ` [PATCH 5/5] merge-tree: fix d/f conflicts Junio C Hamano
4 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2012-12-26 23:03 UTC (permalink / raw)
To: git
Rename the "branch1" parameter given to resolve() to "ours", to
clarify what is going on. Also, annotate the unresolved_directory()
function with some comments to show what decisions are made in each
step, and highlight two bugs that need to be fixed.
Add two tests to t4300 to illustrate these bugs.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/merge-tree.c | 26 ++++++++++++++++++++++----
t/t4300-merge-tree.sh | 44 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+), 4 deletions(-)
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 95de162..5704d51 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -172,17 +172,17 @@ static char *traverse_path(const struct traverse_info *info, const struct name_e
return make_traverse_path(path, info, n);
}
-static void resolve(const struct traverse_info *info, struct name_entry *branch1, struct name_entry *result)
+static void resolve(const struct traverse_info *info, struct name_entry *ours, struct name_entry *result)
{
struct merge_list *orig, *final;
const char *path;
- /* If it's already branch1, don't bother showing it */
- if (!branch1)
+ /* If it's already ours, don't bother showing it */
+ if (!ours)
return;
path = traverse_path(info, result);
- orig = create_entry(2, branch1->mode, branch1->sha1, path);
+ orig = create_entry(2, ours->mode, ours->sha1, path);
final = create_entry(0, result->mode, result->sha1, path);
final->link = orig;
@@ -205,6 +205,15 @@ static int unresolved_directory(const struct traverse_info *info, struct name_en
}
if (!S_ISDIR(p->mode))
return 0;
+ /*
+ * NEEDSWORK: this is broken. The path can originally be a file
+ * and then one side may have turned it into a directory, in which
+ * case we return and let the three-way merge as if the tree were
+ * a regular file. If the path that was originally a tree is
+ * now a file in either branch, fill_tree_descriptor() below will
+ * die when fed a blob sha1.
+ */
+
newbase = traverse_path(info, p);
buf0 = fill_tree_descriptor(t+0, n[0].sha1);
buf1 = fill_tree_descriptor(t+1, n[1].sha1);
@@ -288,20 +297,29 @@ static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, s
/* Same in both? */
if (same_entry(entry+1, entry+2)) {
if (entry[0].sha1) {
+ /* Modified identically */
resolve(info, NULL, entry+1);
return mask;
}
+ /* "Both added the same" is left unresolved */
}
if (same_entry(entry+0, entry+1)) {
if (entry[2].sha1 && !S_ISDIR(entry[2].mode)) {
+ /* We did not touch, they modified -- take theirs */
resolve(info, entry+1, entry+2);
return mask;
}
+ /*
+ * If we did not touch a directory but they made it
+ * into a file, we fall through and unresolved()
+ * recurses down. Likewise for the opposite case.
+ */
}
if (same_entry(entry+0, entry+2)) {
if (entry[1].sha1 && !S_ISDIR(entry[1].mode)) {
+ /* We modified, they did not touch -- take ours */
resolve(info, NULL, entry+1);
return mask;
}
diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh
index 46c3fe7..03e8fca 100755
--- a/t/t4300-merge-tree.sh
+++ b/t/t4300-merge-tree.sh
@@ -254,4 +254,48 @@ EXPECTED
test_cmp expected actual
'
+test_expect_failure 'turn file to tree' '
+ git reset --hard initial &&
+ rm initial-file &&
+ mkdir initial-file &&
+ test_commit "turn-file-to-tree" "initial-file/ONE" "CCC" &&
+ git merge-tree initial initial turn-file-to-tree >actual &&
+ cat >expect <<-\EOF &&
+ added in remote
+ their 100644 43aa4fdec31eb92e1fdc2f0ce6ea9ddb7c32bcf7 initial-file/ONE
+ @@ -0,0 +1 @@
+ +CCC
+ removed in remote
+ base 100644 e79c5e8f964493290a409888d5413a737e8e5dd5 initial-file
+ our 100644 e79c5e8f964493290a409888d5413a737e8e5dd5 initial-file
+ @@ -1 +0,0 @@
+ -initial
+ EOF
+ test_cmp expect actual
+'
+
+test_expect_failure 'turn tree to file' '
+ git reset --hard initial &&
+ mkdir dir &&
+ test_commit "add-tree" "dir/path" "AAA" &&
+ test_commit "add-another-tree" "dir/another" "BBB" &&
+ rm -fr dir &&
+ test_commit "make-file" "dir" "CCC" &&
+ git merge-tree add-tree add-another-tree make-file >actual &&
+ cat >expect <<-\EOF &&
+ added in local
+ our 100644 ba629238ca89489f2b350e196ca445e09d8bb834 dir/another
+ removed in remote
+ base 100644 43d5a8ed6ef6c00ff775008633f95787d088285d dir/path
+ our 100644 43d5a8ed6ef6c00ff775008633f95787d088285d dir/path
+ @@ -1 +0,0 @@
+ -AAA
+ added in remote
+ their 100644 43aa4fdec31eb92e1fdc2f0ce6ea9ddb7c32bcf7 dir
+ @@ -0,0 +1 @@
+ +CCC
+ EOF
+ test_cmp expect actual
+'
+
test_done
--
1.8.1.rc3.356.g686f81c
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 5/5] merge-tree: fix d/f conflicts
2012-12-26 23:03 [PATCH 0/5] merge-tree fix-ups Junio C Hamano
` (3 preceding siblings ...)
2012-12-26 23:03 ` [PATCH 4/5] merge-tree: add comments to clarify what these functions are doing Junio C Hamano
@ 2012-12-26 23:03 ` Junio C Hamano
4 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2012-12-26 23:03 UTC (permalink / raw)
To: git
The previous commit documented two known breakages revolving around
a case where one side flips a tree into a blob (or vice versa),
where the original code simply gets confused and feeds a mixture of
trees and blobs into either the recursive merge-tree (and recursing
into the blob will fail) or three-way merge (and merging tree contents
together with blobs will fail).
Fix it by feeding trees (and only trees) into the recursive
merge-tree machinery and blobs (and only blobs) into the three-way
content level merge machinery separately; when this happens, the
entire merge has to be marked as conflicting at the structure level.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/merge-tree.c | 72 ++++++++++++++++++++++++++++-----------------------
t/t4300-merge-tree.sh | 4 +--
2 files changed, 42 insertions(+), 34 deletions(-)
diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
index 5704d51..e0d0b7d 100644
--- a/builtin/merge-tree.c
+++ b/builtin/merge-tree.c
@@ -25,7 +25,7 @@ static void add_merge_entry(struct merge_list *entry)
merge_result_end = &entry->next;
}
-static void merge_trees(struct tree_desc t[3], const char *base);
+static void merge_trees_recursive(struct tree_desc t[3], const char *base, int df_conflict);
static const char *explanation(struct merge_list *entry)
{
@@ -190,41 +190,35 @@ static void resolve(const struct traverse_info *info, struct name_entry *ours, s
add_merge_entry(final);
}
-static int unresolved_directory(const struct traverse_info *info, struct name_entry n[3])
+static void unresolved_directory(const struct traverse_info *info, struct name_entry n[3],
+ int df_conflict)
{
char *newbase;
struct name_entry *p;
struct tree_desc t[3];
void *buf0, *buf1, *buf2;
- p = n;
- if (!p->mode) {
- p++;
- if (!p->mode)
- p++;
+ for (p = n; p < n + 3; p++) {
+ if (p->mode && S_ISDIR(p->mode))
+ break;
}
- if (!S_ISDIR(p->mode))
- return 0;
- /*
- * NEEDSWORK: this is broken. The path can originally be a file
- * and then one side may have turned it into a directory, in which
- * case we return and let the three-way merge as if the tree were
- * a regular file. If the path that was originally a tree is
- * now a file in either branch, fill_tree_descriptor() below will
- * die when fed a blob sha1.
- */
+ if (n + 3 <= p)
+ return; /* there is no tree here */
newbase = traverse_path(info, p);
- buf0 = fill_tree_descriptor(t+0, n[0].sha1);
- buf1 = fill_tree_descriptor(t+1, n[1].sha1);
- buf2 = fill_tree_descriptor(t+2, n[2].sha1);
- merge_trees(t, newbase);
+
+#define ENTRY_SHA1(e) (((e)->mode && S_ISDIR((e)->mode)) ? (e)->sha1 : NULL)
+ buf0 = fill_tree_descriptor(t+0, ENTRY_SHA1(n + 0));
+ buf1 = fill_tree_descriptor(t+1, ENTRY_SHA1(n + 1));
+ buf2 = fill_tree_descriptor(t+2, ENTRY_SHA1(n + 2));
+#undef ENTRY_SHA1
+
+ merge_trees_recursive(t, newbase, df_conflict);
free(buf0);
free(buf1);
free(buf2);
free(newbase);
- return 1;
}
@@ -247,18 +241,26 @@ static struct merge_list *link_entry(unsigned stage, const struct traverse_info
static void unresolved(const struct traverse_info *info, struct name_entry n[3])
{
struct merge_list *entry = NULL;
+ int i;
+ unsigned dirmask = 0, mask = 0;
+
+ for (i = 0; i < 3; i++) {
+ mask |= (1 << 1);
+ if (n[i].mode && S_ISDIR(n[i].mode))
+ dirmask |= (1 << i);
+ }
+
+ unresolved_directory(info, n, dirmask && (dirmask != mask));
- if (unresolved_directory(info, n))
+ if (dirmask == mask)
return;
- /*
- * Do them in reverse order so that the resulting link
- * list has the stages in order - link_entry adds new
- * links at the front.
- */
- entry = link_entry(3, info, n + 2, entry);
- entry = link_entry(2, info, n + 1, entry);
- entry = link_entry(1, info, n + 0, entry);
+ if (n[2].mode && !S_ISDIR(n[2].mode))
+ entry = link_entry(3, info, n + 2, entry);
+ if (n[1].mode && !S_ISDIR(n[1].mode))
+ entry = link_entry(2, info, n + 1, entry);
+ if (n[0].mode && !S_ISDIR(n[0].mode))
+ entry = link_entry(1, info, n + 0, entry);
add_merge_entry(entry);
}
@@ -329,15 +331,21 @@ static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, s
return mask;
}
-static void merge_trees(struct tree_desc t[3], const char *base)
+static void merge_trees_recursive(struct tree_desc t[3], const char *base, int df_conflict)
{
struct traverse_info info;
setup_traverse_info(&info, base);
+ info.data = &df_conflict;
info.fn = threeway_callback;
traverse_trees(3, t, &info);
}
+static void merge_trees(struct tree_desc t[3], const char *base)
+{
+ merge_trees_recursive(t, base, 0);
+}
+
static void *get_tree_descriptor(struct tree_desc *desc, const char *rev)
{
unsigned char sha1[20];
diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh
index 03e8fca..d0b2a45 100755
--- a/t/t4300-merge-tree.sh
+++ b/t/t4300-merge-tree.sh
@@ -254,7 +254,7 @@ EXPECTED
test_cmp expected actual
'
-test_expect_failure 'turn file to tree' '
+test_expect_success 'turn file to tree' '
git reset --hard initial &&
rm initial-file &&
mkdir initial-file &&
@@ -274,7 +274,7 @@ test_expect_failure 'turn file to tree' '
test_cmp expect actual
'
-test_expect_failure 'turn tree to file' '
+test_expect_success 'turn tree to file' '
git reset --hard initial &&
mkdir dir &&
test_commit "add-tree" "dir/path" "AAA" &&
--
1.8.1.rc3.356.g686f81c
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-12-26 23:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-26 23:03 [PATCH 0/5] merge-tree fix-ups Junio C Hamano
2012-12-26 23:03 ` [PATCH 1/5] Which merge_file() function do you mean? Junio C Hamano
2012-12-26 23:03 ` [PATCH 2/5] merge-tree: lose unused "flags" from merge_list Junio C Hamano
2012-12-26 23:03 ` [PATCH 3/5] merge-tree: lose unused "resolve_directories" Junio C Hamano
2012-12-26 23:03 ` [PATCH 4/5] merge-tree: add comments to clarify what these functions are doing Junio C Hamano
2012-12-26 23:03 ` [PATCH 5/5] merge-tree: fix d/f conflicts Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).