From: Matt Cree <matt.cree@gearset.com>
To: sandals@crustytoothpaste.net
Cc: git@vger.kernel.org, matt.cree@gearset.com, newren@gmail.com
Subject: Re: Unexpected git merge exit code when killing merge driver during ancestor merge
Date: Wed, 8 May 2024 19:14:12 +0100 [thread overview]
Message-ID: <B2E03023-18FF-453E-91F0-9A55D430EE31@gearset.com> (raw)
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.
next reply other threads:[~2024-05-08 18:14 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-08 18:14 Matt Cree [this message]
-- strict thread matches above, loose matches on Subject: below --
2024-04-04 16:16 Unexpected git merge exit code when killing merge driver during ancestor merge Matt Cree
2024-04-05 20:04 ` brian m. carlson
2024-05-09 5:03 ` Elijah Newren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=B2E03023-18FF-453E-91F0-9A55D430EE31@gearset.com \
--to=matt.cree@gearset.com \
--cc=git@vger.kernel.org \
--cc=newren@gmail.com \
--cc=sandals@crustytoothpaste.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).