From: Clemens Buchacher <drizzd@aon.at>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH] merge-recursive: handle file mode changes
Date: Fri, 14 Mar 2008 13:17:52 +0100 [thread overview]
Message-ID: <20080314121752.GB3315@localhost> (raw)
In-Reply-To: <7vhcf9r4qp.fsf@gitster.siamese.dyndns.org>
On Fri, Mar 14, 2008 at 03:15:26AM -0700, Junio C Hamano wrote:
> > 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.
Indeed. I think "b" should be favored over "a", however.
> > + 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.
Yes, but if "b" and "a" both changed in the same way, we should still set
result.merge = 1, because that's more akin to an automatic merge than a
fast-forward merge, IMO.
Clemens
next prev parent reply other threads:[~2008-03-14 12:18 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-03-08 17:17 [PATCH] fix recursive-merge of empty files with different permissions Clemens Buchacher
2008-03-08 17:51 ` Johannes Schindelin
2008-03-13 12:52 ` Clemens Buchacher
2008-03-13 15:19 ` Johannes Schindelin
2008-03-13 18:50 ` Junio C Hamano
2008-03-13 21:28 ` Johannes Schindelin
2008-03-13 19:22 ` [PATCH] merge-recursive: cause a conflict if file mode does not match Clemens Buchacher
2008-03-13 21:17 ` Johannes Schindelin
2008-03-13 22:47 ` [PATCH] merge-recursive: handle file mode changes Clemens Buchacher
2008-03-13 23:40 ` Johannes Schindelin
2008-03-14 9:21 ` [PATCH] merge-recursive: handle file mode and links similarly to file content Clemens Buchacher
2008-03-14 10:13 ` Clemens Buchacher
2008-03-14 0:08 ` [PATCH] merge-recursive: handle file mode changes Junio C Hamano
2008-03-14 0:12 ` Junio C Hamano
2008-03-14 13:02 ` Clemens Buchacher
2008-03-14 0:17 ` Jakub Narebski
2008-03-14 12:56 ` Clemens Buchacher
2008-03-14 10:15 ` Junio C Hamano
2008-03-14 12:17 ` Clemens Buchacher [this message]
2008-03-14 16:01 ` Junio C Hamano
2008-03-14 17:28 ` Clemens Buchacher
2008-03-14 17:49 ` Junio C Hamano
2008-03-14 10:07 ` [PATCH] merge-recursive: cause a conflict if file mode does not match Clemens Buchacher
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080314121752.GB3315@localhost \
--to=drizzd@aon.at \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).