threads / discuss / 61267

Unexpected git merge exit code when killing merge driver during ancestor merge

Subject: Unexpected git merge exit code when killing merge driver during ancestor merge

## tl;dr

5 messages between Apr 4, 2024 and Jun 13, 2024.

replies: 4people: 4as markdown or json

Matt Cree· Apr 4, 2024, 16:16 UTC · lore
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.

brian m. carlson· Apr 5, 2024, 20:04 UTC · re: Matt Cree · lore

Re: Unexpected git merge exit code when killing merge driver during ancestor merge

On 2024-04-04 at 16:16:05, Matt Cree wrote:
Show 20 quoted lines
> 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

Thanks for the repro steps. I'm on Debian, which uses dash as /bin/sh, and I also use a different default branch (dev), so I was able to reproduce with the following patch applied:

----
diff --git a/init-repo.sh b/init-repo.sh
old mode 100644
new mode 100755
index e0f42a4..25d7f25
--- a/init-repo.sh
+++ b/init-repo.sh
@@ -1,5 +1,5 @@
 rm -rf merge-driver-test
 mkdir merge-driver-test
 cd merge-driver-test
-git init .
+git init -b master .
 git commit --allow-empty -m "Initial"
\ No newline at end of file
diff --git a/run-merge.sh b/run-merge.sh
old mode 100644
new mode 100755
diff --git a/run-recursive-merge.sh b/run-recursive-merge.sh
old mode 100644
new mode 100755
index 6920720..c63d652
--- a/run-recursive-merge.sh
+++ b/run-recursive-merge.sh
@@ -1,3 +1,5 @@
+#!/bin/sh
+
 cd merge-driver-test
 
 current_time=$(date "+%Y%m%d-%H%M%S");
@@ -12,7 +14,7 @@ featureA="$current_time-feature-a";
 featureB="$current_time-feature-b";
 featureC="$current_time-feature-c";
 
-function writeFiles() {
+writeFiles() {
 cat > $xmlFileName << EOM
 <?xml version="1.0" encoding="UTF-8"?>
 <CustomLabels xmlns="http://soap.sforce.com/2006/04/metadata">
----

I take it from the "Abort trap" message below, you're on macOS, but I
don't think that's relevant to reproduction.

> 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
> ```

This is definitely a bug because we triggered an assertion.  The
assertion asserts that that case will never happen, so if it does, we've
made a mistake in our code.

This also explains the 134 exit status, because on most Unix systems,
`SIGABRT` is signal 6, and when a program exits with a signal, the shell
returns an exit status of 128 plus the signal number.  Because a failed
assertion calls `abort`, which raises `SIGABRT`, that would lead to an
exit status in the shell of 134.

I've CC'd Elijah Newren, who's the author of merge-ort and who wrote the
code.  I'm not familiar at all with merge-ort, so I can't speak to what
might be going wrong here.
-- 
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA
Elijah Newren· May 9, 2024, 05:03 UTC · re: brian m. carlson · lore

Re: Unexpected git merge exit code when killing merge driver during ancestor merge

On Fri, Apr 5, 2024 at 1:04 PM brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 90 quoted lines
>
> On 2024-04-04 at 16:16:05, Matt Cree wrote:
> > 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
>
> Thanks for the repro steps.  I'm on Debian, which uses dash as /bin/sh,
> and I also use a different default branch (dev), so I was able to
> reproduce with the following patch applied:
>
> ----
> diff --git a/init-repo.sh b/init-repo.sh
> old mode 100644
> new mode 100755
> index e0f42a4..25d7f25
> --- a/init-repo.sh
> +++ b/init-repo.sh
> @@ -1,5 +1,5 @@
>  rm -rf merge-driver-test
>  mkdir merge-driver-test
>  cd merge-driver-test
> -git init .
> +git init -b master .
>  git commit --allow-empty -m "Initial"
> \ No newline at end of file
> diff --git a/run-merge.sh b/run-merge.sh
> old mode 100644
> new mode 100755
> diff --git a/run-recursive-merge.sh b/run-recursive-merge.sh
> old mode 100644
> new mode 100755
> index 6920720..c63d652
> --- a/run-recursive-merge.sh
> +++ b/run-recursive-merge.sh
> @@ -1,3 +1,5 @@
> +#!/bin/sh
> +
>  cd merge-driver-test
>
>  current_time=$(date "+%Y%m%d-%H%M%S");
> @@ -12,7 +14,7 @@ featureA="$current_time-feature-a";
>  featureB="$current_time-feature-b";
>  featureC="$current_time-feature-c";
>
> -function writeFiles() {
> +writeFiles() {
>  cat > $xmlFileName << EOM
>  <?xml version="1.0" encoding="UTF-8"?>
>  <CustomLabels xmlns="http://soap.sforce.com/2006/04/metadata">
> ----
>
> I take it from the "Abort trap" message below, you're on macOS, but I
> don't think that's relevant to reproduction.
>
> > 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
> > ```
>
> This is definitely a bug because we triggered an assertion.  The
> assertion asserts that that case will never happen, so if it does, we've
> made a mistake in our code.
>
> This also explains the 134 exit status, because on most Unix systems,
> `SIGABRT` is signal 6, and when a program exits with a signal, the shell
> returns an exit status of 128 plus the signal number.  Because a failed
> assertion calls `abort`, which raises `SIGABRT`, that would lead to an
> exit status in the shell of 134.
>
> I've CC'd Elijah Newren, who's the author of merge-ort and who wrote the
> code.  I'm not familiar at all with merge-ort, so I can't speak to what
> might be going wrong here.
brian: Thanks for tagging me and expounding on the testcase.
Matt: sorry for taking so long to respond.

This is just a quick note to say I'm aware of the bug and will respond (I think there might be a simple fix here), but for various reasons it's going to be a couple more weeks.

Elijah Newren· Jun 13, 2024, 20:50 UTC · re: Elijah Newren · lore

Re: Unexpected git merge exit code when killing merge driver during ancestor merge

On Wed, May 8, 2024 at 10:03 PM Elijah Newren <newren@gmail.com> wrote:
Show 7 quoted lines
>
> On Fri, Apr 5, 2024 at 1:04 PM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> >
> > On 2024-04-04 at 16:16:05, Matt Cree wrote:
> > > 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.
> > >
[...]
> > This is definitely a bug because we triggered an assertion.  The
> > assertion asserts that that case will never happen, so if it does, we've
> > made a mistake in our code.
[...]
Show 10 quoted lines
> > I've CC'd Elijah Newren, who's the author of merge-ort and who wrote the
> > code.  I'm not familiar at all with merge-ort, so I can't speak to what
> > might be going wrong here.
>
> brian: Thanks for tagging me and expounding on the testcase.
> Matt: sorry for taking so long to respond.
>
> This is just a quick note to say I'm aware of the bug and will respond
> (I think there might be a simple fix here), but for various reasons
> it's going to be a couple more weeks.

Again, sorry for the delay. For those reading through the archives, I posted a fix to this and a few other issues found while investigating this problem over here: https://lore.kernel.org/git/pull.1748.git.1718310307.gitgitgadget@gmail.com/

Junio C Hamano· Jun 13, 2024, 22:22 UTC · re: Elijah Newren · lore

Re: Unexpected git merge exit code when killing merge driver during ancestor merge

Elijah Newren <newren@gmail.com> writes:
> Again, sorry for the delay.  For those reading through the archives, I
> posted a fix to this and a few other issues found while investigating
> this problem over here:
> https://lore.kernel.org/git/pull.1748.git.1718310307.gitgitgadget@gmail.com/
Thanks for a pleasant read.  It mostly looked fine.  Queued.

← back to recent threads