On Mon, Apr 04, 2016 at 10:34:34AM -0700, Junio C Hamano wrote: > Jeff King <peff@xxxxxxxx> writes: > > > Imagine a merge where one side changes the content of a path and the > > other changes the mode. Here's a minimal reproduction: > > > > git init repo && cd repo && > > > > echo base >file && > > git add file && > > git commit -m base && > > > > echo changed >file && > > git commit -am content && > > > > git checkout -b side HEAD^ > > chmod +x file && > > git commit -am mode > > ... > > This is a leftover from my experiments with merge-resolve versus > > merge-recursive last fall, which resulted in a few actual bug-fixes. I > > looked into fixing this case, too, at that time. It seemed possible, but > > a little more involved than you might think (because the logic is driven > > by a bunch of case statements, and this adds a multiplicative layer to > > the cases; we might need to resolve the permissions, and _then_ see if > > the content can be resolved). > > Perhaps I am missing some other codepath in the "multiplicative" > layer, but is this not sufficient? Sorry for the super-slow reply; just cleaning out my "to respond to" pile, which has gotten pretty deep. Looking at it again, I think you are right. I seemed to recall that there were multiple case arms where we dealt with the permissions, but I cannot find such a spot now. So I think the solution you outlined looks good. > - if test "$6" != "$7" > + # Three-way merge of the permissions > + perm= ;# assume the result is the same from stage #2, i.e. $6 > + if test "$6" = "$7" || test "$5" = "$7" > + then > + : nothing > + elif test "$5" = "$6" > then > + case "$7" in > + 100644) perm=-x ;; > + 100755) perm=+x ;; > + *) echo "ERROR: $7: funny filemode not handled." >&2 ;; > + esac We reject symlinks and submodules earlier, so I think this "funny filemode" error really should only be truly-funny entries. Good. -Peff -- 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