#77 git merge fails because tkdiff exit status is 1

[shraga]

Hi
I upgraded tkdiff from 4.2 to 5.2 and found out that I cannot use tkdiff as git mergetool. Tried debugging a bit and it seems that when I finish merging and hit "save & exit", tkdiff exit with '1' .
When running with -d, I do see something that may be an error causing this: Every time I hit the shortcut keys '1' '2', 'n' or 'p' I get the following message:

Dbg: .merge.top.text_: insert: DENIED:  ReadOnly
Dbg: .merge.top.text_: see(?): DENIED:  ReadOnly

This is the full debug log:

Dbg:  2 filespecs, 0-0 revisions
Dbg:  Recovered 2 filespecs, 0 revisions
Dbg: Final: 0 revs  2 filespecs -> 2/2 pairings  0 Err
Dbg:  Assigning labels to headers
Dbg: PROPOSED wm grid is WxH(80x30) of WxH(8x15)pxls
        while PREVsz was WxH(x)       (x)
Dbg: Trim BOTH L/R to NEW computed width(80) - and detach gridding
Dbg:   NEW   wm grid is (160 X 30)   (8 x 15)
Dbg: centering(728x446) onto parent(1628x553)
Dbg: Centering has targeted at =+709+110 above parent @(259,57)
Dbg: Custom code IS in use...beware
Dbg: is3way(1)
Dbg: MergeFileSet(1): compojure_core.clj
Dbg: temp file /tmp/244036-cmderr-1
Dbg: DIFF(55) .vs. diff(55)
Dbg: temp file /tmp/244036-cmderr-2
Dbg: temp file /tmp/244036-cmderr-3
Dbg: Left  ancestral datums: 2
Dbg: Right ancestral datums: 3
Dbg: .merge.top.text_: insert: DENIED:  ReadOnly
Dbg: .merge.top.text_: see(?): DENIED:  ReadOnly
Dbg: .merge.top.text_: insert: DENIED:  ReadOnly
Dbg: .merge.top.text_: see(?): DENIED:  ReadOnly
Dbg: .merge.top.text_: insert: DENIED:  ReadOnly
Dbg: .merge.top.text_: see(?): DENIED:  ReadOnly
Dbg: .merge.top.text_: insert: DENIED:  ReadOnly
Dbg: .merge.top.text_: see(?): DENIED:  ReadOnly
Dbg: .merge.top.text_: insert: DENIED:  ReadOnly
Dbg: .merge.top.text_: see(?): DENIED:  ReadOnly
Dbg: .merge.top.text_: insert: DENIED:  ReadOnly
Dbg: .merge.top.text_: see(?): DENIED:  ReadOnly
Dbg: .merge.top.text_: insert: DENIED:  ReadOnly
Dbg: .merge.top.text_: see(?): DENIED:  ReadOnly
Dbg: .merge.top.text_: insert: DENIED:  ReadOnly
Dbg: .merge.top.text_: see(?): DENIED:  ReadOnly
Dbg: .merge.top.text_: insert: DENIED:  ReadOnly
Dbg: .merge.top.text_: see(?): DENIED:  ReadOnly
Dbg: .merge.top.text_: insert: DENIED:  ReadOnly
Dbg: .merge.top.text_: see(?): DENIED:  ReadOnly
Dbg: -> (into compojure_core.clj)

I tried deleting my tkdiff rc file but behavior doesn't change.
I am attaching a tar file with the files, and the tkdiff cmd.

[michael-m]

First, thank you for reporting this. Much of V5.2 was targeted at overhauling the internal infrastructure that dealt with the handling/reporting of "errors" and how the tool should respond which has tentacles reaching to obscure places throughout the code. Second, I wholeheartedly agree that under the usage you describe, TkDiff should not return a code indicating problems - but it has little to do with the messages you zeroed-in on (despite their ominous sounding nature).
Just to quickly explain those messages, they exist as "debug-only" indicators to note the correct operation of the text window, which TkDiff designates as read-only. They fundamentally mean that SOMETHING has tried to edit the window - but was blocked. In your case, I suspect it is simply responding to the keystroke (the "1" or "2") you used to initiate the "merge selection" which the text window (by default) assumes was meant to be ENTERED wherever the insertion cursor was positioned - however ended up blocked by the read-only implementation. We simply report such deflections (as debug messaging) to ensure that INTENDED insertions are not being incorrectly blocked.
As to your ACTUAL problem, it seems to be nothing more than a "pass- thru" of the "return code" from Diff itself (which TkDiff runs internally for you). Diff says that a ZERO return code means "identical", while ONE indicates the existence of differences. TWO is the indicator of "problems", although Diff provides some documentation about ways to adjust what constitutes a "problem".
What concerns me is that this "pass-thru" of the Diff return code, is NOT something new (although we might have become a bit more rigorous in V5.2 about ensuring it happens). Is it possible there is something missing from the integration between TkDiff and GIT? It sounds like GIT may be expecting the more conventional "Shell-like" return-code semantics where ZERO is the only "success" code. Were you using V4.2 with GIT successfully prior to your upgrade? A quick google search on "git trustexitcode" seems to suggest that GIT is prepared to deal with a merge operation where the returncode might not be to GITs default liking or expectation.

[shraga]

Thanks michael-m
I went back and find out that even tkdiff 4.3.1 doesn't play nicely with git mergetool, and the merge operation always fail. Seems strange this was not reported until now. Makes me think something might be wrong in my environment ?

[michael-m]

My point exactly.
I believe there is a Git configuration option you are missing that is called "trustexitcode".
I AM NO GIT EXPERT - but my limited understanding is that you have to "configure" Git to utilize external tools, such as TkDiff. From what I've read, there are TWO places where TkDiff could be considered useful in a Git environment:
1. as a Diff tool (just to SEE the diffs)
2. as a Merge tool (to actually make modifications)
However, Git applies different semantics when calling these two TYPEs of tools, and in this case that involves how it should treat the RETURN CODE . Apparently when invoking the "diff tool" it automatically IGNORES any return codes. But when invoking the "mergetool" it appears to test the return code for "success" (which I suspect is a zero) UNLESS TOLD NOT TO!
This is where this "trustexitcode" attribute come in. You have to ADD this into your personal Git configuration to TELL Git that the return code from the invoked MERGE tool (TkDiff) DOESN'T MATTER.
I'm unfamiliar with the EXACT way of doing this (or I'd tell you), but Git is rather well documented so it shouldn't be that hard to find (hence my earlier google search). I'm fairly confident this will solve your issue.
TkDiff really CANT change its approach to simply "return zero" as we are a graphical overlay for the ACTUAL diff program, and thus its inappropriate for us to ALTER the Diff semantics. Good Luck, and please let us know how it turns out.

[shraga]

Thanks a lot michael-m for pointing it out. After modifying my gitconfig all is OK.
For those in the future who might see the same behavior, this is the settings you need to do:

git config --global mergetool.tkdiff.trustExitCode false

I think the ticket can be closed now