Re: [PATCH] merge-recursive: handle file mode changes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Clemens Buchacher <drizzd@xxxxxx> writes:

> 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.

> +		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.

> @@ -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.

> 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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux