[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Allow the merge tool to abort the merge

From: Stefan Sperling <stsp_at_elego.de>
Date: Tue, 30 Dec 2008 02:22:35 +0100

On Mon, Dec 29, 2008 at 12:19:00PM -0800, John Gardiner Myers wrote:
> Stefan Sperling wrote:
> > 1) Why not just abort on exit code != 0?
> Because the current behavior of proceeding with the merge, leaving the
> conflicts is more useful in the common case.

OK, so we're talking about having three different possibilities:

(cases coded by exit code)
0) External tool merges files successfully
  -> merge proceeds, file is marked as merged
1) External tool fails merging files, leaving conflict markers
  -> merge proceeds, file remains in conflict
2) External tool fails merging files, leaving conflict markers,
   and wants to abort the merge
  -> merge is aborted, file remains in conflict

Currently, we only have case 0 and 1, right?

What do we do with tools that do exit 2 for some other reason?
Not all tools people might want to use with Subversion might be
aware of any exit code conventions we might have. Could this
be a problem?

> There are two aspects of the current behavior I have issue with:
>
> a) Why, when running "--accept launch", it doesn't try running the merge
> script on subsequent conflicting files.

Do you mean:

  ... svn doesn't try running the merge script on subsequent conflicting
  files if the tool exits unsuccessfully (!= 0)

?

> While this would make sense for
> SVN_ERR_CL_NO_EXTERNAL_MERGE_TOOL, just because the merge tool wasn't
> able to resolve conflicts for one particular file doesn't mean it
> wouldn't be able to resolve conflicts for subsequent files in the same
> merge.

This statement is confusing me. I thought the current behaviour
was just that: Leave the file in conflict and proceed with the merge
even though the tool failed to merge the file (exit != 0).

> b) Why it chooses merged instead of postpone for the (first) conflicted
> file the merge tool is unable to resolve. This appears to me to be a bug.

Hmmm... what do you mean by "unable to resolve"?
Leaving conflict markers in the file but doing exit 0?
Or doing exit 1?

Can you provide a reproduction script that shows the behaviour
you are describing? Maybe using /bin/true or /bin/false as fake
merge tools?

As far as I can tell, the current code just cares about "error" or
"success". If the tool indicates success (exit 0), it assumes the file
was merged correctly. Otherwise, it does not assume so, and the file
is left in conflict.

I get this impression from reading the code, not from running it,
so I might be wrong -- that's why I'm asking for a reproduction script.
This would help a lot to clarify what we're talking about.
 
> > 2) Could you extend your patch to document the new behaviour somewhere?
> > I don't know the appropriate place to put this information. But I
> > suppose you know where merge tool authors would be looking for this
> > kind of information?
> >
> Existing documentation on the merge tool is scarce. The only place I've
> found information useful for writing a merge tool is a rather oblique
> reference in the "Using External Differencing and Merge Tools" section
> of the Subversion book. Even that omits the SVN_MERGE environment variable.

That's bad. Please add this information, if you can.

See http://svnbook.red-bean.com/ under "Feedback/Contributing".
Among other information, it tells you where to get the book
source: svn co http://svn.red-bean.com/svnbook/trunk/ svnbook
Read src/en/HACKING.

> My understanding is that updates to the book are done through a separate
> mechanism.

Yes, as explained above, it's in a separate repository.
But it is otherwise closely tied to the Subversion project.
Quite a few Subversion developers and many other people can commit to it.
If you have patches for the book to document writing merge tools, I can
commit them. Please submit them to the svnbook-dev_at_red-bean.com mailing list.

Thanks,
Stefan

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=995399
Received on 2008-12-30 02:23:10 CET

This is an archived mail posted to the Subversion Dev mailing list.