* [PATCH] merge-tree: fix "same file added in subdir" @ 2013-03-27 21:34 John Keeping 2013-03-27 22:42 ` Junio C Hamano 0 siblings, 1 reply; 8+ messages in thread From: John Keeping @ 2013-03-27 21:34 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, John Keeping When the same file is added with identical content at the top level, git-merge-tree prints "added in both" with the details. But if the file is added in an existing subdirectory, threeway_callback() bails out early because the two trees have been modified identically. In order to detect this, we need to fall through and recurse into the subtree in this case. Signed-off-by: John Keeping <john@keeping.me.uk> --- builtin/merge-tree.c | 9 +++++++-- t/t4300-merge-tree.sh | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c index e0d0b7d..ca97fbd 100644 --- a/builtin/merge-tree.c +++ b/builtin/merge-tree.c @@ -298,12 +298,17 @@ 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) { + if (entry[0].sha1 && !S_ISDIR(entry[0].mode)) { /* Modified identically */ resolve(info, NULL, entry+1); return mask; } - /* "Both added the same" is left unresolved */ + /* + * "Both added the same" is left unresolved. We also leave + * "Both directories modified identically" unresolved in + * order to catch changes where the same file (with the same + * content) has been added to both directories. + */ } if (same_entry(entry+0, entry+1)) { diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh index d0b2a45..be0737e 100755 --- a/t/t4300-merge-tree.sh +++ b/t/t4300-merge-tree.sh @@ -298,4 +298,21 @@ test_expect_success 'turn tree to file' ' test_cmp expect actual ' +test_expect_success 'add identical files to subdir' ' + cat >expected <<\EXPECTED && +added in both + our 100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/ONE + their 100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/ONE +EXPECTED + + git reset --hard initial && + mkdir sub && + test_commit "sub-initial" "sub/initial" "initial" && + test_commit "sub-add-a-b-same-A" "sub/ONE" "AAA" && + git reset --hard sub-initial && + test_commit "sub-add-a-b-same-B" "sub/ONE" "AAA" && + git merge-tree sub-initial sub-add-a-b-same-A sub-add-a-b-same-B >actual && + test_cmp expected actual +' + test_done -- 1.8.2.411.g65a544e ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] merge-tree: fix "same file added in subdir" 2013-03-27 21:34 [PATCH] merge-tree: fix "same file added in subdir" John Keeping @ 2013-03-27 22:42 ` Junio C Hamano 2013-03-27 22:57 ` John Keeping 0 siblings, 1 reply; 8+ messages in thread From: Junio C Hamano @ 2013-03-27 22:42 UTC (permalink / raw) To: John Keeping; +Cc: git, Linus Torvalds John Keeping <john@keeping.me.uk> writes: > When the same file is added with identical content at the top level, > git-merge-tree prints "added in both" with the details. But if the file > is added in an existing subdirectory, threeway_callback() bails out early > because the two trees have been modified identically. > > In order to detect this, we need to fall through and recurse into the > subtree in this case. > > Signed-off-by: John Keeping <john@keeping.me.uk> The rationale the above description gives is internally consistent, but it is rather sad to see this optimization go. The primary motivation behind this program, which does not use the usual unpack-trees machinery, is to allow us to cull the identical result at a shallow level of the traversal when the both sides changed (not added) a file deep in a subdirectory hierarchy. The patch makes me wonder if we should go the other way around, resolving the "both added identically" case at the top cleanly without complaint. > builtin/merge-tree.c | 9 +++++++-- > t/t4300-merge-tree.sh | 17 +++++++++++++++++ > 2 files changed, 24 insertions(+), 2 deletions(-) > > diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c > index e0d0b7d..ca97fbd 100644 > --- a/builtin/merge-tree.c > +++ b/builtin/merge-tree.c > @@ -298,12 +298,17 @@ 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) { > + if (entry[0].sha1 && !S_ISDIR(entry[0].mode)) { > /* Modified identically */ > resolve(info, NULL, entry+1); > return mask; > } > - /* "Both added the same" is left unresolved */ > + /* > + * "Both added the same" is left unresolved. We also leave > + * "Both directories modified identically" unresolved in > + * order to catch changes where the same file (with the same > + * content) has been added to both directories. > + */ > } > > if (same_entry(entry+0, entry+1)) { > diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh > index d0b2a45..be0737e 100755 > --- a/t/t4300-merge-tree.sh > +++ b/t/t4300-merge-tree.sh > @@ -298,4 +298,21 @@ test_expect_success 'turn tree to file' ' > test_cmp expect actual > ' > > +test_expect_success 'add identical files to subdir' ' > + cat >expected <<\EXPECTED && > +added in both > + our 100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/ONE > + their 100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/ONE > +EXPECTED > + > + git reset --hard initial && > + mkdir sub && > + test_commit "sub-initial" "sub/initial" "initial" && > + test_commit "sub-add-a-b-same-A" "sub/ONE" "AAA" && > + git reset --hard sub-initial && > + test_commit "sub-add-a-b-same-B" "sub/ONE" "AAA" && > + git merge-tree sub-initial sub-add-a-b-same-A sub-add-a-b-same-B >actual && > + test_cmp expected actual > +' > + > test_done ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] merge-tree: fix "same file added in subdir" 2013-03-27 22:42 ` Junio C Hamano @ 2013-03-27 22:57 ` John Keeping 2013-03-28 9:34 ` John Keeping 0 siblings, 1 reply; 8+ messages in thread From: John Keeping @ 2013-03-27 22:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Linus Torvalds On Wed, Mar 27, 2013 at 03:42:40PM -0700, Junio C Hamano wrote: > John Keeping <john@keeping.me.uk> writes: > > > When the same file is added with identical content at the top level, > > git-merge-tree prints "added in both" with the details. But if the file > > is added in an existing subdirectory, threeway_callback() bails out early > > because the two trees have been modified identically. > > > > In order to detect this, we need to fall through and recurse into the > > subtree in this case. > > > > Signed-off-by: John Keeping <john@keeping.me.uk> > > The rationale the above description gives is internally consistent, > but it is rather sad to see this optimization go. The primary > motivation behind this program, which does not use the usual > unpack-trees machinery, is to allow us to cull the identical result > at a shallow level of the traversal when the both sides changed (not > added) a file deep in a subdirectory hierarchy. > > The patch makes me wonder if we should go the other way around, > resolving the "both added identically" case at the top cleanly > without complaint. I don't use merge-tree so I have no opinion on this, just wanted to fix an inconsistency :-) I'll try to have a look at doing the other change tomorrow if no one else gets there first. > > builtin/merge-tree.c | 9 +++++++-- > > t/t4300-merge-tree.sh | 17 +++++++++++++++++ > > 2 files changed, 24 insertions(+), 2 deletions(-) > > > > diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c > > index e0d0b7d..ca97fbd 100644 > > --- a/builtin/merge-tree.c > > +++ b/builtin/merge-tree.c > > @@ -298,12 +298,17 @@ 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) { > > + if (entry[0].sha1 && !S_ISDIR(entry[0].mode)) { > > /* Modified identically */ > > resolve(info, NULL, entry+1); > > return mask; > > } > > - /* "Both added the same" is left unresolved */ > > + /* > > + * "Both added the same" is left unresolved. We also leave > > + * "Both directories modified identically" unresolved in > > + * order to catch changes where the same file (with the same > > + * content) has been added to both directories. > > + */ > > } > > > > if (same_entry(entry+0, entry+1)) { > > diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh > > index d0b2a45..be0737e 100755 > > --- a/t/t4300-merge-tree.sh > > +++ b/t/t4300-merge-tree.sh > > @@ -298,4 +298,21 @@ test_expect_success 'turn tree to file' ' > > test_cmp expect actual > > ' > > > > +test_expect_success 'add identical files to subdir' ' > > + cat >expected <<\EXPECTED && > > +added in both > > + our 100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/ONE > > + their 100644 43d5a8ed6ef6c00ff775008633f95787d088285d sub/ONE > > +EXPECTED > > + > > + git reset --hard initial && > > + mkdir sub && > > + test_commit "sub-initial" "sub/initial" "initial" && > > + test_commit "sub-add-a-b-same-A" "sub/ONE" "AAA" && > > + git reset --hard sub-initial && > > + test_commit "sub-add-a-b-same-B" "sub/ONE" "AAA" && > > + git merge-tree sub-initial sub-add-a-b-same-A sub-add-a-b-same-B >actual && > > + test_cmp expected actual > > +' > > + > > test_done ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] merge-tree: fix "same file added in subdir" 2013-03-27 22:57 ` John Keeping @ 2013-03-28 9:34 ` John Keeping 2013-04-07 20:12 ` Junio C Hamano 0 siblings, 1 reply; 8+ messages in thread From: John Keeping @ 2013-03-28 9:34 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Linus Torvalds On Wed, Mar 27, 2013 at 10:57:39PM +0000, John Keeping wrote: > On Wed, Mar 27, 2013 at 03:42:40PM -0700, Junio C Hamano wrote: > > John Keeping <john@keeping.me.uk> writes: > > > > > When the same file is added with identical content at the top level, > > > git-merge-tree prints "added in both" with the details. But if the file > > > is added in an existing subdirectory, threeway_callback() bails out early > > > because the two trees have been modified identically. > > > > > > In order to detect this, we need to fall through and recurse into the > > > subtree in this case. > > > > > > Signed-off-by: John Keeping <john@keeping.me.uk> > > > > The rationale the above description gives is internally consistent, > > but it is rather sad to see this optimization go. The primary > > motivation behind this program, which does not use the usual > > unpack-trees machinery, is to allow us to cull the identical result > > at a shallow level of the traversal when the both sides changed (not > > added) a file deep in a subdirectory hierarchy. > > > > The patch makes me wonder if we should go the other way around, > > resolving the "both added identically" case at the top cleanly > > without complaint. > > I don't use merge-tree so I have no opinion on this, just wanted to fix > an inconsistency :-) Having re-read the manpage, I think you're right that we should just resolve the "both added identically" case cleanly, but I wonder whether some of the other cases should also be resolved cleanly. git-merge-tree(1) says: the output from the command omits entries that match the <branch1> tree. so you could argue that "added in branch1", "changed in branch1, unmodified in branch2" and "removed in branch1, unchanged in branch2" should also print no output. But as I said above I don't use git-merge-tree so perhaps people who do would like to explain what they expect in these cases. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] merge-tree: fix "same file added in subdir" 2013-03-28 9:34 ` John Keeping @ 2013-04-07 20:12 ` Junio C Hamano 2013-04-07 21:07 ` [PATCH v2] merge-tree: don't print entries that match "local" John Keeping 0 siblings, 1 reply; 8+ messages in thread From: Junio C Hamano @ 2013-04-07 20:12 UTC (permalink / raw) To: John Keeping; +Cc: git, Linus Torvalds John Keeping <john@keeping.me.uk> writes: > Having re-read the manpage, I think you're right that we should just > resolve the "both added identically" case cleanly, but I wonder whether > some of the other cases should also be resolved cleanly. > > git-merge-tree(1) says: > > the output from the command omits entries that match the <branch1> > tree. > > so you could argue that "added in branch1", "changed in branch1, > unmodified in branch2" and "removed in branch1, unchanged in branch2" > should also print no output. Yes, that description explains what we wanted to achieve with merge-tree that is still an unfinished experiment and fixing these cases you identified above to match the stated goal is a good thing to do. I've been meaning to find enough time to do a new merge strategy backend based on the logic of this program, and the few patches I sent out on it recently were fallout from preparatory steps for it. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] merge-tree: don't print entries that match "local" 2013-04-07 20:12 ` Junio C Hamano @ 2013-04-07 21:07 ` John Keeping 2013-04-27 11:35 ` René Scharfe 0 siblings, 1 reply; 8+ messages in thread From: John Keeping @ 2013-04-07 21:07 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Linus Torvalds The documentation says: the output from the command omits entries that match the <branch1> tree. But currently "added in branch1" and "removed in branch1" (both while unchanged in branch2) do print output. Change this so that the behaviour matches the documentation. Signed-off-by: John Keeping <john@keeping.me.uk> --- builtin/merge-tree.c | 26 +++++++++++++------------- t/t4300-merge-tree.sh | 10 ---------- 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c index bc912e3..ed25d81 100644 --- a/builtin/merge-tree.c +++ b/builtin/merge-tree.c @@ -155,6 +155,11 @@ static int same_entry(struct name_entry *a, struct name_entry *b) a->mode == b->mode; } +static int both_empty(struct name_entry *a, struct name_entry *b) +{ + return !(a->sha1 || b->sha1); +} + static struct merge_list *create_entry(unsigned stage, unsigned mode, const unsigned char *sha1, const char *path) { struct merge_list *res = xcalloc(1, sizeof(*res)); @@ -297,13 +302,10 @@ static void unresolved(const struct traverse_info *info, struct name_entry n[3]) static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, struct name_entry *entry, struct traverse_info *info) { /* 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+1, entry+2) || both_empty(entry+0, entry+2)) { + /* Modified, added or removed identically */ + resolve(info, NULL, entry+1); + return mask; } if (same_entry(entry+0, entry+1)) { @@ -319,12 +321,10 @@ static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, s */ } - 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; - } + if (same_entry(entry+0, entry+2) || both_empty(entry+0, entry+2)) { + /* We added, modified or removed, they did not touch -- take ours */ + resolve(info, NULL, entry+1); + return mask; } unresolved(info, entry); diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh index d0b2a45..bd43b3d 100755 --- a/t/t4300-merge-tree.sh +++ b/t/t4300-merge-tree.sh @@ -26,8 +26,6 @@ EXPECTED test_expect_success 'file add !A, B' ' cat >expected <<\EXPECTED && -added in local - our 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE EXPECTED git reset --hard initial && @@ -38,9 +36,6 @@ EXPECTED test_expect_success 'file add A, B (same)' ' cat >expected <<\EXPECTED && -added in both - our 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE - their 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE EXPECTED git reset --hard initial && @@ -181,9 +176,6 @@ AAA" && test_expect_success 'file remove A, !B' ' cat >expected <<\EXPECTED && -removed in local - base 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE - their 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE EXPECTED git reset --hard initial && @@ -283,8 +275,6 @@ test_expect_success 'turn tree to file' ' 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.8.2.694.ga76e9c3.dirty ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] merge-tree: don't print entries that match "local" 2013-04-07 21:07 ` [PATCH v2] merge-tree: don't print entries that match "local" John Keeping @ 2013-04-27 11:35 ` René Scharfe 2013-04-27 12:55 ` John Keeping 0 siblings, 1 reply; 8+ messages in thread From: René Scharfe @ 2013-04-27 11:35 UTC (permalink / raw) To: John Keeping; +Cc: Junio C Hamano, git, Linus Torvalds Am 07.04.2013 23:07, schrieb John Keeping: > The documentation says: > > the output from the command omits entries that match the > <branch1> tree. > > But currently "added in branch1" and "removed in branch1" (both while > unchanged in branch2) do print output. Change this so that the > behaviour matches the documentation. > > Signed-off-by: John Keeping <john@keeping.me.uk> > --- > builtin/merge-tree.c | 26 +++++++++++++------------- > t/t4300-merge-tree.sh | 10 ---------- > 2 files changed, 13 insertions(+), 23 deletions(-) > > diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c > index bc912e3..ed25d81 100644 > --- a/builtin/merge-tree.c > +++ b/builtin/merge-tree.c > @@ -155,6 +155,11 @@ static int same_entry(struct name_entry *a, struct name_entry *b) > a->mode == b->mode; > } > > +static int both_empty(struct name_entry *a, struct name_entry *b) > +{ > + return !(a->sha1 || b->sha1); > +} > + > static struct merge_list *create_entry(unsigned stage, unsigned mode, const unsigned char *sha1, const char *path) > { > struct merge_list *res = xcalloc(1, sizeof(*res)); > @@ -297,13 +302,10 @@ static void unresolved(const struct traverse_info *info, struct name_entry n[3]) > static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, struct name_entry *entry, struct traverse_info *info) > { > /* 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+1, entry+2) || both_empty(entry+0, entry+2)) { Shouldn't this zero be a one instead? > + /* Modified, added or removed identically */ > + resolve(info, NULL, entry+1); > + return mask; > } > > if (same_entry(entry+0, entry+1)) { > @@ -319,12 +321,10 @@ static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, s > */ > } > > - 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; > - } > + if (same_entry(entry+0, entry+2) || both_empty(entry+0, entry+2)) { Otherwise the both_empty check here can be removed because we'd have already returned above if it was true. But do we actually want to resolve the removal of a file on both sides silently? The added comment above says so, but the commit message doesn't mention it. > + /* We added, modified or removed, they did not touch -- take ours */ > + resolve(info, NULL, entry+1); > + return mask; > } > > unresolved(info, entry); > diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh > index d0b2a45..bd43b3d 100755 > --- a/t/t4300-merge-tree.sh > +++ b/t/t4300-merge-tree.sh > @@ -26,8 +26,6 @@ EXPECTED > > test_expect_success 'file add !A, B' ' > cat >expected <<\EXPECTED && > -added in local > - our 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE > EXPECTED > > git reset --hard initial && > @@ -38,9 +36,6 @@ EXPECTED > > test_expect_success 'file add A, B (same)' ' > cat >expected <<\EXPECTED && > -added in both > - our 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE > - their 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE > EXPECTED > > git reset --hard initial && > @@ -181,9 +176,6 @@ AAA" && > > test_expect_success 'file remove A, !B' ' > cat >expected <<\EXPECTED && > -removed in local > - base 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE > - their 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE > EXPECTED > > git reset --hard initial && > @@ -283,8 +275,6 @@ test_expect_success 'turn tree to file' ' > 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 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] merge-tree: don't print entries that match "local" 2013-04-27 11:35 ` René Scharfe @ 2013-04-27 12:55 ` John Keeping 0 siblings, 0 replies; 8+ messages in thread From: John Keeping @ 2013-04-27 12:55 UTC (permalink / raw) To: René Scharfe; +Cc: Junio C Hamano, git, Linus Torvalds On Sat, Apr 27, 2013 at 01:35:57PM +0200, René Scharfe wrote: > Am 07.04.2013 23:07, schrieb John Keeping: > > The documentation says: > > > > the output from the command omits entries that match the > > <branch1> tree. > > > > But currently "added in branch1" and "removed in branch1" (both while > > unchanged in branch2) do print output. Change this so that the > > behaviour matches the documentation. > > > > Signed-off-by: John Keeping <john@keeping.me.uk> > > --- > > builtin/merge-tree.c | 26 +++++++++++++------------- > > t/t4300-merge-tree.sh | 10 ---------- > > 2 files changed, 13 insertions(+), 23 deletions(-) > > > > diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c > > index bc912e3..ed25d81 100644 > > --- a/builtin/merge-tree.c > > +++ b/builtin/merge-tree.c > > @@ -155,6 +155,11 @@ static int same_entry(struct name_entry *a, struct name_entry *b) > > a->mode == b->mode; > > } > > > > +static int both_empty(struct name_entry *a, struct name_entry *b) > > +{ > > + return !(a->sha1 || b->sha1); > > +} > > + > > static struct merge_list *create_entry(unsigned stage, unsigned mode, const unsigned char *sha1, const char *path) > > { > > struct merge_list *res = xcalloc(1, sizeof(*res)); > > @@ -297,13 +302,10 @@ static void unresolved(const struct traverse_info *info, struct name_entry n[3]) > > static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, struct name_entry *entry, struct traverse_info *info) > > { > > /* 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+1, entry+2) || both_empty(entry+0, entry+2)) { > > Shouldn't this zero be a one instead? Yes, that's a typo. > > + /* Modified, added or removed identically */ > > + resolve(info, NULL, entry+1); > > + return mask; > > } > > > > if (same_entry(entry+0, entry+1)) { > > @@ -319,12 +321,10 @@ static int threeway_callback(int n, unsigned long mask, unsigned long dirmask, s > > */ > > } > > > > - 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; > > - } > > + if (same_entry(entry+0, entry+2) || both_empty(entry+0, entry+2)) { > > Otherwise the both_empty check here can be removed because we'd have > already returned above if it was true. > > But do we actually want to resolve the removal of a file on both sides > silently? The added comment above says so, but the commit message > doesn't mention it. I think so. The rule is that we omit things that match the branch1 tree. So if it is removed in branch1 and there isn't a conflict with branch2 then we shouldn't be printing any output. > > + /* We added, modified or removed, they did not touch -- take ours */ > > + resolve(info, NULL, entry+1); > > + return mask; > > } > > > > unresolved(info, entry); > > diff --git a/t/t4300-merge-tree.sh b/t/t4300-merge-tree.sh > > index d0b2a45..bd43b3d 100755 > > --- a/t/t4300-merge-tree.sh > > +++ b/t/t4300-merge-tree.sh > > @@ -26,8 +26,6 @@ EXPECTED > > > > test_expect_success 'file add !A, B' ' > > cat >expected <<\EXPECTED && > > -added in local > > - our 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE > > EXPECTED > > > > git reset --hard initial && > > @@ -38,9 +36,6 @@ EXPECTED > > > > test_expect_success 'file add A, B (same)' ' > > cat >expected <<\EXPECTED && > > -added in both > > - our 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE > > - their 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE > > EXPECTED > > > > git reset --hard initial && > > @@ -181,9 +176,6 @@ AAA" && > > > > test_expect_success 'file remove A, !B' ' > > cat >expected <<\EXPECTED && > > -removed in local > > - base 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE > > - their 100644 43d5a8ed6ef6c00ff775008633f95787d088285d ONE > > EXPECTED > > > > git reset --hard initial && > > @@ -283,8 +275,6 @@ test_expect_success 'turn tree to file' ' > > 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 > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-04-27 12:55 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-03-27 21:34 [PATCH] merge-tree: fix "same file added in subdir" John Keeping 2013-03-27 22:42 ` Junio C Hamano 2013-03-27 22:57 ` John Keeping 2013-03-28 9:34 ` John Keeping 2013-04-07 20:12 ` Junio C Hamano 2013-04-07 21:07 ` [PATCH v2] merge-tree: don't print entries that match "local" John Keeping 2013-04-27 11:35 ` René Scharfe 2013-04-27 12:55 ` John Keeping
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 NNTP newsgroup(s).