Re: [PATCH] merge-recursive: handle file mode changes
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 14, 2008, 10:15 UTC
- Message-ID
- <7vhcf9r4qp.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <20080313224741.GA5000@localhost>
Clemens Buchacher <drizzd@aon.at> writes:
Show 17 quoted lines
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 34e3167..d8938cc 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1025,12 +1025,21 @@ static struct merge_file_info merge_file(struct diff_filespec *o,
> hashcpy(result.sha, b->sha1);
> }
> } else {
> - if (!sha_eq(a->sha1, o->sha1) && !sha_eq(b->sha1, o->sha1))
> - result.merge = 1;
> -
> - result.mode = a->mode == o->mode ? b->mode: a->mode;
> + /*
> + * If mode changed in only one branch, keep the changed
> + * version. Otherwise, keep remote version and create a
> + * conflict.
> + */Reading the rest of the function, I notice that it consistently favor "a" over "b", when a conflict cannot be reconciled.
Show 6 quoted lines
> + if (a->mode != o->mode && b->mode != o->mode &&
> + a->mode != b->mode) {
> + result.clean = 0;
> + result.mode = b->mode;
> + } else
> + result.mode = o->mode == a->mode ? b->mode : a->mode;I think this is much easier to read:
if (a->mode == b->mode || a->mode == o->mode)
result.mode = b->mode;
else {
result.mode = a->mode;
if (b->mode != o->mode) {
result.clean = 0;
result.merge = 1;
}
}We keep "b" only if "a" hasn't touched the mode, or it happens to be the same as "a". Otherwise we take "a" anyway, but taking "a" when "b" also touched means we detected an unreconcilable conflict.
Show 6 quoted lines
> @@ -1062,6 +1071,8 @@ static struct merge_file_info merge_file(struct diff_filespec *o,
> } else {
> die("unsupported object type in the tree");
> }
> +
> + result.merge = !result.clean;As pointed out by Dscho, this is too much. We could mimick the one that is done for the contents, which is why I have "result.merge = 1" where it is in the above.
Show 11 quoted lines
> diff --git a/t/t6031-merge-recursive.sh b/t/t6031-merge-recursive.sh > new file mode 100755 > index 0000000..36cd664 > --- /dev/null > +++ b/t/t6031-merge-recursive.sh > @@ -0,0 +1,39 @@ > +#!/bin/sh > ... > + ! (git merge-recursive master -- a2 b2 || test $? -ne 1) && > + ! test -x file2 > +'
As we would favor our side in unreconsilable conflicts, the last test needs to become "test -x file2".
Also we should test ls-files -u output to make sure we have correct stages in the index.
I've done minor fixups and committed the result.