Junio C Hamano wrote:
> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
>
> > Junio C Hamano wrote:
> >> Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:
> >>
> >> > +auto_merge () {
> >> > + git merge-file --diff3 --marker-size=7 -q -p "$LOCAL" "$BASE" "$REMOTE" >"$DIFF3"
> >> > + if test -s "$DIFF3"
> >> > + then
> >>
> >> We do not want to ignore the exit status from the command. IOW, I
> >> think the above wants to be rather
> >>
> >> if git merge-file ... >"$DIFF3" &&
> >> test -s "$DIFF3"
> >> then
> >> ...
> >
> > That doesn't work.
> >
> > "git merge-file" always returns non-zero status when it succeeds (it's
> > the number of conflicts generated).
>
> Ah, I forgot about that one. I think "the number of conflicts" was
> a UI mistake (the original that it mimics is "merge" from RCS suite,
> which uses 1 and 2 for "conflicts" and "trouble") but we know we
> will get conflicts, so it is wrong to expect success from the
> command. Deliberately ignoring the return status is the right thing
> to do.
I agree. My bet is that nobody is checking the return status of "git
merge-file" to find out the number of conflicts. Plus, how can you check
the difference between 255 conflicts and error -1?
But that's the situation we are in now.
> > What if the original file does have these markers?
> >
> > Which is probably something we should be checking beforehand and not
> > attempt an automerge in those cases.
>
> Yes, that is a much better approach to avoid unnecessary work.
>
> When we made the conflict marker length configurable, we were hoping
> that we no longer have to worry about the cases where payload files
> (original or ours or theirs) have lines that are confusingly similar
> to the conflict markers, but because we are interfacing external tools
> that are unaware of the facility, it probably would not help us in
> this case all that much.
>
> FWIW, we use a fiarly large size for our own files in t/ and
> Documentation/ directories ourselves, and it does help topic branch
> merges somewhat frequently.
We could do something like --marker-size=13 to minimize the chances of
that happening.
In that case I would prefer '/^<\{13\} /' (to avoid too many
characters). I see those regexes used elsewhere in git, but I don't know
how portable that is.
If we wanted to make sure none of those markers remain it's not enough
to check for '^[<|=>]{13}', what follows up should be a space, or some
delimiter, not another < for example. So maybe '^[<|=>]{13}[^<|=>]'?
So, do we want those three things?
1. A non-standard marker-size
2. Check beforehand the existence of those markers and disable
automerge
3. Check afterwards the existence of those markers and disable
automerge
Cheers.
--
Felipe Contreras
