{"thread":{"id":"61267","subject":"Unexpected git merge exit code when killing merge driver during ancestor merge","startedAt":"2024-04-04T16:16:17Z","lastAt":"2024-06-13T22:23:02Z","messageCount":5,"participants":["Matt Cree","brian m. carlson","Elijah Newren","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"492249","messageId":"75F8BD12-7743-4863-B4C5-049FDEC4645E@gearset.com","threadId":"61267","inReplyTo":null,"subject":"Unexpected git merge exit code when killing merge driver during ancestor merge","fromName":"Matt Cree","fromEmail":"matt.cree@gearset.com","sentAt":"2024-04-04T16:16:05Z","receivedAt":"2024-04-04T16:16:17Z","isPatch":false,"sender":{"key":"matt.cree@gearset.com","avatar":null},"body":"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.\n\nI’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.\n\nHere is my current understanding of how the ort strategy will deal with this.\n\n- Ort runs the merge driver with the parameters for the current file to be merged\n- When the driver returns exit code 0 is returned it is treated as having no conflicts\n- When the driver returns exit code 1-128 is returned it is treated as having conflicts\n- When the driver returns exit code 129+ is returned it is treated as some kind of error scenario\n\n\nThen subsequently\n- If all files returned exit code 0 during the merge git will return exit code 0 i.e. no conflicts\n- If any file returned exit code 1-128 during the merge git will return exit code 1 i.e. conflicts\n- At any time if the driver returns 129+, git will stop merging and return exit code 2 i.e. error?\n\nHowever, when setting up a criss-cross merge scenario and ‘short circuiting’ the merge during an ancestor merge, I get exit code 134\n\nHere’s a couple of quick scripts that help recreate the situation https://gist.github.com/mattcree/c6d8cc95f41e30b5d7467e9d2b01cd3d\n\nThe logs also show \n\n```\nAssertion 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\n```\n\nI 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.\nHopefully someone knows a bit more about this.\n"},{"id":"492320","messageId":"ZhBZOY1aEFW4YoD8@tapette.crustytoothpaste.net","threadId":"61267","inReplyTo":"75F8BD12-7743-4863-B4C5-049FDEC4645E@gearset.com","subject":"Re: Unexpected git merge exit code when killing merge driver during ancestor merge","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2024-04-05T20:04:09Z","receivedAt":"2024-04-05T20:04:17Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2024-04-04 at 16:16:05, Matt Cree wrote:\n> 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.\n> \n> 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.\n> \n> Here is my current understanding of how the ort strategy will deal with this.\n> \n> - Ort runs the merge driver with the parameters for the current file to be merged\n> - When the driver returns exit code 0 is returned it is treated as having no conflicts\n> - When the driver returns exit code 1-128 is returned it is treated as having conflicts\n> - When the driver returns exit code 129+ is returned it is treated as some kind of error scenario\n> \n> \n> Then subsequently\n> - If all files returned exit code 0 during the merge git will return exit code 0 i.e. no conflicts\n> - If any file returned exit code 1-128 during the merge git will return exit code 1 i.e. conflicts\n> - At any time if the driver returns 129+, git will stop merging and return exit code 2 i.e. error?\n> \n> However, when setting up a criss-cross merge scenario and ‘short circuiting’ the merge during an ancestor merge, I get exit code 134\n> \n> Here’s a couple of quick scripts that help recreate the situation https://gist.github.com/mattcree/c6d8cc95f41e30b5d7467e9d2b01cd3d\n\nThanks for the repro steps. I'm on Debian, which uses dash as /bin/sh,\nand I also use a different default branch (dev), so I was able to\nreproduce with the following patch applied:\n\n----\ndiff --git a/init-repo.sh b/init-repo.sh\nold mode 100644\nnew mode 100755\nindex e0f42a4..25d7f25\n--- a/init-repo.sh\n+++ b/init-repo.sh\n@@ -1,5 +1,5 @@\n rm -rf merge-driver-test\n mkdir merge-driver-test\n cd merge-driver-test\n-git init .\n+git init -b master .\n git commit --allow-empty -m \"Initial\"\n\\ No newline at end of file\ndiff --git a/run-merge.sh b/run-merge.sh\nold mode 100644\nnew mode 100755\ndiff --git a/run-recursive-merge.sh b/run-recursive-merge.sh\nold mode 100644\nnew mode 100755\nindex 6920720..c63d652\n--- a/run-recursive-merge.sh\n+++ b/run-recursive-merge.sh\n@@ -1,3 +1,5 @@\n+#!/bin/sh\n+\n cd merge-driver-test\n \n current_time=$(date \"+%Y%m%d-%H%M%S\");\n@@ -12,7 +14,7 @@ featureA=\"$current_time-feature-a\";\n featureB=\"$current_time-feature-b\";\n featureC=\"$current_time-feature-c\";\n \n-function writeFiles() {\n+writeFiles() {\n cat > $xmlFileName << EOM\n \n \n----\n\nI take it from the \"Abort trap\" message below, you're on macOS, but I\ndon't think that's relevant to reproduction.\n\n> The logs also show \n> \n> ```\n> 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\n> ```\n\nThis is definitely a bug because we triggered an assertion. The\nassertion asserts that that case will never happen, so if it does, we've\nmade a mistake in our code.\n\nThis also explains the 134 exit status, because on most Unix systems,\n`SIGABRT` is signal 6, and when a program exits with a signal, the shell\nreturns an exit status of 128 plus the signal number. Because a failed\nassertion calls `abort`, which raises `SIGABRT`, that would lead to an\nexit status in the shell of 134.\n\nI've CC'd Elijah Newren, who's the author of merge-ort and who wrote the\ncode. I'm not familiar at all with merge-ort, so I can't speak to what\nmight be going wrong here.\n-- \nbrian m. carlson (they/them or he/him)\nToronto, Ontario, CA\n"},{"id":"494342","messageId":"CABPp-BEogmpG7pYmC84WhQCy0tP97cV8BZ8epPeo4fzmRjU5Rg@mail.gmail.com","threadId":"61267","inReplyTo":"ZhBZOY1aEFW4YoD8@tapette.crustytoothpaste.net","subject":"Re: Unexpected git merge exit code when killing merge driver during ancestor merge","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2024-05-09T05:03:13Z","receivedAt":"2024-05-09T05:03:26Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Fri, Apr 5, 2024 at 1:04 PM brian m. carlson\n wrote:\n>\n> On 2024-04-04 at 16:16:05, Matt Cree wrote:\n> > 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.\n> >\n> > 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.\n> >\n> > Here is my current understanding of how the ort strategy will deal with this.\n> >\n> > - Ort runs the merge driver with the parameters for the current file to be merged\n> > - When the driver returns exit code 0 is returned it is treated as having no conflicts\n> > - When the driver returns exit code 1-128 is returned it is treated as having conflicts\n> > - When the driver returns exit code 129+ is returned it is treated as some kind of error scenario\n> >\n> >\n> > Then subsequently\n> > - If all files returned exit code 0 during the merge git will return exit code 0 i.e. no conflicts\n> > - If any file returned exit code 1-128 during the merge git will return exit code 1 i.e. conflicts\n> > - At any time if the driver returns 129+, git will stop merging and return exit code 2 i.e. error?\n> >\n> > However, when setting up a criss-cross merge scenario and ‘short circuiting’ the merge during an ancestor merge, I get exit code 134\n> >\n> > Here’s a couple of quick scripts that help recreate the situation https://gist.github.com/mattcree/c6d8cc95f41e30b5d7467e9d2b01cd3d\n>\n> Thanks for the repro steps. I'm on Debian, which uses dash as /bin/sh,\n> and I also use a different default branch (dev), so I was able to\n> reproduce with the following patch applied:\n>\n> ----\n> diff --git a/init-repo.sh b/init-repo.sh\n> old mode 100644\n> new mode 100755\n> index e0f42a4..25d7f25\n> --- a/init-repo.sh\n> +++ b/init-repo.sh\n> @@ -1,5 +1,5 @@\n> rm -rf merge-driver-test\n> mkdir merge-driver-test\n> cd merge-driver-test\n> -git init .\n> +git init -b master .\n> git commit --allow-empty -m \"Initial\"\n> \\ No newline at end of file\n> diff --git a/run-merge.sh b/run-merge.sh\n> old mode 100644\n> new mode 100755\n> diff --git a/run-recursive-merge.sh b/run-recursive-merge.sh\n> old mode 100644\n> new mode 100755\n> index 6920720..c63d652\n> --- a/run-recursive-merge.sh\n> +++ b/run-recursive-merge.sh\n> @@ -1,3 +1,5 @@\n> +#!/bin/sh\n> +\n> cd merge-driver-test\n>\n> current_time=$(date \"+%Y%m%d-%H%M%S\");\n> @@ -12,7 +14,7 @@ featureA=\"$current_time-feature-a\";\n> featureB=\"$current_time-feature-b\";\n> featureC=\"$current_time-feature-c\";\n>\n> -function writeFiles() {\n> +writeFiles() {\n> cat > $xmlFileName << EOM\n> \n> \n> ----\n>\n> I take it from the \"Abort trap\" message below, you're on macOS, but I\n> don't think that's relevant to reproduction.\n>\n> > The logs also show\n> >\n> > ```\n> > 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\n> > ```\n>\n> This is definitely a bug because we triggered an assertion. The\n> assertion asserts that that case will never happen, so if it does, we've\n> made a mistake in our code.\n>\n> This also explains the 134 exit status, because on most Unix systems,\n> `SIGABRT` is signal 6, and when a program exits with a signal, the shell\n> returns an exit status of 128 plus the signal number. Because a failed\n> assertion calls `abort`, which raises `SIGABRT`, that would lead to an\n> exit status in the shell of 134.\n>\n> I've CC'd Elijah Newren, who's the author of merge-ort and who wrote the\n> code. I'm not familiar at all with merge-ort, so I can't speak to what\n> might be going wrong here.\n\nbrian: Thanks for tagging me and expounding on the testcase.\nMatt: sorry for taking so long to respond.\n\nThis is just a quick note to say I'm aware of the bug and will respond\n(I think there might be a simple fix here), but for various reasons\nit's going to be a couple more weeks.\n"},{"id":"497110","messageId":"CABPp-BFrcNLA-714gw+JogtAQLhmcs_sLnkkaBiGERF+riwZLQ@mail.gmail.com","threadId":"61267","inReplyTo":"CABPp-BEogmpG7pYmC84WhQCy0tP97cV8BZ8epPeo4fzmRjU5Rg@mail.gmail.com","subject":"Re: Unexpected git merge exit code when killing merge driver during ancestor merge","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2024-06-13T20:50:04Z","receivedAt":"2024-06-13T20:50:17Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Wed, May 8, 2024 at 10:03 PM Elijah Newren wrote:\n>\n> On Fri, Apr 5, 2024 at 1:04 PM brian m. carlson\n> wrote:\n> >\n> > On 2024-04-04 at 16:16:05, Matt Cree wrote:\n> > > 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.\n> > >\n[...]\n> > This is definitely a bug because we triggered an assertion. The\n> > assertion asserts that that case will never happen, so if it does, we've\n> > made a mistake in our code.\n[...]\n> > I've CC'd Elijah Newren, who's the author of merge-ort and who wrote the\n> > code. I'm not familiar at all with merge-ort, so I can't speak to what\n> > might be going wrong here.\n>\n> brian: Thanks for tagging me and expounding on the testcase.\n> Matt: sorry for taking so long to respond.\n>\n> This is just a quick note to say I'm aware of the bug and will respond\n> (I think there might be a simple fix here), but for various reasons\n> it's going to be a couple more weeks.\n\nAgain, sorry for the delay. For those reading through the archives, I\nposted a fix to this and a few other issues found while investigating\nthis problem over here:\nhttps://lore.kernel.org/git/pull.1748.git.1718310307.gitgitgadget@gmail.com/\n"},{"id":"497112","messageId":"xmqqo784jxdu.fsf@gitster.g","threadId":"61267","inReplyTo":"CABPp-BFrcNLA-714gw+JogtAQLhmcs_sLnkkaBiGERF+riwZLQ@mail.gmail.com","subject":"Re: Unexpected git merge exit code when killing merge driver during ancestor merge","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-06-13T22:22:53Z","receivedAt":"2024-06-13T22:23:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n> Again, sorry for the delay. For those reading through the archives, I\n> posted a fix to this and a few other issues found while investigating\n> this problem over here:\n> https://lore.kernel.org/git/pull.1748.git.1718310307.gitgitgadget@gmail.com/\n\nThanks for a pleasant read. It mostly looked fine. Queued.\n\n"}]}