Stefan Sperling wrote:
> 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
>
Correct.
> 1) External tool fails merging files, leaving conflict markers
> -> merge proceeds, file remains in conflict
>
This would be reasonable behavior, but is not the current behavior.
> 2) External tool fails merging files, leaving conflict markers,
> and wants to abort the merge
> -> merge is aborted, file remains in conflict
>
This is the case I am attempting to add.
> 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?
>
According to the book, Subversion treats 0 and 1 differently than any
other error code for external diff and diff3 programs. I believe this
is ample precedent for treating 2 differently than 1.
>
>> 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)
>
> ?
>
>
That sentence was in the context of the exit code being != 0, so yes.
>> 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).
>
No, the behavior is to leave the conflict markers in the file, mark that
file as being resolved, and handle any subsequent conflicted files as if
the merge had been run with "--accept postpone".
In item (a), I am taking issue with the behavior of handling any
subsequent conflicted files as if the merge had been run with "--accept
postpone". While that may be reasonable for the case when no merge tool
was specified, that makes less sense in the case where the merge tool
returns an exit code of 1.
>> 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?
>
By "unable to resolve", I mean doing anything other than exit 0.
> Can you provide a reproduction script that shows the behaviour
> you are describing? Maybe using /bin/true or /bin/false as fake
> merge tools?
>
I reproduce by creating a merge script that writes a line to stderr and
does an exit 1 and then doing a "svn merge --accept launch" that creates
two or more conflicted files.
> 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.
>
The first conflicting file is left with conflict markers, but is left in
"M" status. As I mentioned previously, I believe this to be a bug.
The remaining conflicting files are left in "C" status. The merge
script is only ever invoked once.
> 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.
>
None of the existing test scripts cover "svn merge" in combination with
a merge tool. I am unfamiliar with your test harness framework, so
creating a whole new class of tests is daunting.
> That's bad. Please add this information, if you can.
I would be willing to do so once my patches were accepted, but as the
book uses a separate update procedure, I am unable to include such
changes in the patches that I submit here.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=995507
Received on 2008-12-30 10:03:29 CET