Git Mailing List Archive mirror
 help / color / mirror / Atom feed
* Re: Unexpected git merge exit code when killing merge driver during ancestor merge
@ 2024-05-08 18:14 Matt Cree
  0 siblings, 0 replies; 6+ messages in thread
From: Matt Cree @ 2024-05-08 18:14 UTC (permalink / raw)
  To: sandals; +Cc: git, matt.cree, newren



As time has gone on, I’ve discovered when we do have this crash during an ancestor merge, somewhat unsurprisingly there is an index.lock file left over in the .git directory.


I’ve taken a look at the code and I believe I’ve spotted the issue — I have v2.44.0 checked out atm.


In the function `merge_ort_internal` function the ancestor merges occur using recursion. If there’s an error during this process, a negative number must be set as the `result->clean` value and this is used to terminate the recursion early https://github.com/git/git/blob/184c3b4c735f1c1f0f811fddcf3c2676e7ad613f/merge-ort.c#L5052

In terminating here, it means the code does not continue on to the final merge which uses the merge_ort_nonrecursive_internal which would have been called in the non recursive case anyway https://github.com/git/git/blob/184c3b4c735f1c1f0f811fddcf3c2676e7ad613f/merge-ort.c#L5068 

From what I can see, that function will correctly reinitialise/reset the opt data structure if the call depth is 0? I am guessing that is an indicator that the merge is supposed to be finished? If we did not clear it here, I’d guess we’d run into the same bug I identified during a non-recursive merge. Therefore, back at the point we first found a negative `result->clean` value, I think we probably want to do a similar reinitialisation of the opts i.e.

>		if (result->clean < 0)
>			/*
>			 * When there are ancestor merge failures,
>			 * the merge_ort_nonrecursive_internal() cleans these up
>			 * so we must do it here too
>			 */
>			result->priv = opt->priv;
>			result->_properly_initialized = RESULT_INITIALIZED;
>			opt->priv = NULL;
>			return;


I’m really not familiar with C or the git source code so this might be wrong, but it would be useful to have this case handled by git rather than myself as the time between an ancestor merge and a final merge can vary wildly and any kind of ‘short circuit’ logic I write would prefer heavily to use discover of conflicts during an ancestor merge.

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Unexpected git merge exit code when killing merge driver during ancestor merge
@ 2024-04-04 16:16 Matt Cree
  2024-04-05 20:04 ` brian m. carlson
  0 siblings, 1 reply; 6+ messages in thread
From: Matt Cree @ 2024-04-04 16:16 UTC (permalink / raw)
  To: git

Hello all. I have observed some strange behaviour when exiting a custom merge driver that I was wondering if there’s any reason for — I think it may be a bug but I’ll leave that to you to decide.

I’m configuring that merge driver to exit during a merge at the first sign of conflicts — the exact nature of the rules for the decision to exit early isn’t too important I think though so given it’s ‘work stuff’ I’ll leave some details out.

Here is my current understanding of how the ort strategy will deal with this.

- Ort runs the merge driver with the parameters for the current file to be merged
- When the driver returns exit code 0 is returned it is treated as having no conflicts
- When the driver returns exit code 1-128 is returned it is treated as having conflicts
- When the driver returns exit code 129+ is returned it is treated as some kind of error scenario


Then subsequently
- If all files returned exit code 0 during the merge git will return exit code 0 i.e. no conflicts
- If any file returned exit code 1-128 during the merge git will return exit code 1 i.e. conflicts
- At any time if the driver returns 129+, git will stop merging and return exit code 2 i.e. error?

However, when setting up a criss-cross merge scenario and ‘short circuiting’ the merge during an ancestor merge, I get exit code 134

Here’s a couple of quick scripts that help recreate the situation https://gist.github.com/mattcree/c6d8cc95f41e30b5d7467e9d2b01cd3d

The logs also show 

```
Assertion failed: (opt->priv == NULL), function merge_switch_to_result, file merge-ort.c, line 4661. ./run-recursive-merge.sh: line 162: 78797 Abort trap: 6 git merge $featureC --no-ff --no-commit
```

I thought it might be worth raising as a bug here but I’m not too sure really — I suppose the short circuiting logic I have introduced may not be a desirable use case from the git elders point of view, but I reckon the difference in behaviour depending on whether it’s an ancestor merge or a final merge seems to indicate to me that this is not intentional.
Hopefully someone knows a bit more about this.

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-06-13 22:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-08 18:14 Unexpected git merge exit code when killing merge driver during ancestor merge Matt Cree
  -- strict thread matches above, loose matches on Subject: below --
2024-04-04 16:16 Matt Cree
2024-04-05 20:04 ` brian m. carlson
2024-05-09  5:03   ` Elijah Newren
2024-06-13 20:50     ` Elijah Newren
2024-06-13 22:22       ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).