{"thread":{"id":"61738","subject":"[PATCH] merge-ort: fix missing early return","startedAt":"2024-07-04T20:02:24Z","lastAt":"2024-07-07T02:50:34Z","messageCount":4,"participants":["Elijah Newren via GitGitGadget","Jeff King","Junio C Hamano","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"498093","messageId":"pull.1758.git.1720123341342.gitgitgadget@gmail.com","threadId":"61738","inReplyTo":null,"subject":"[PATCH] merge-ort: fix missing early return","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-07-04T20:02:21Z","receivedAt":"2024-07-04T20:02:24Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"From: Elijah Newren \n\nOne of the conversions in 500433edf49 (\"merge-ort: convert more error()\ncases to path_msg()\", 2024-06-10) accidentally lost the early return.\nRestore it.\n\nSigned-off-by: Elijah Newren \n---\n merge-ort: fix missing early return\n \n This is a patch on top of en/ort-inner-merge-error-fix which is in next.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1758%2Fnewren%2Ffix-fix-error-cases-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1758/newren/fix-fix-error-cases-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1758\n\n merge-ort.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/merge-ort.c b/merge-ort.c\nindex 8dfe80f1009..d9ba6e3e523 100644\n--- a/merge-ort.c\n+++ b/merge-ort.c\n@@ -3618,6 +3618,7 @@ static int read_oid_strbuf(struct merge_options *opt,\n \t\tpath_msg(opt, ERROR_OBJECT_NOT_A_BLOB, 0,\n \t\t\t path, NULL, NULL, NULL,\n \t\t\t _(\"error: object %s is not a blob\"), oid_to_hex(oid));\n+\t\treturn -1;\n \t}\n \tstrbuf_attach(dst, buf, size, size + 1);\n \treturn 0;\n\nbase-commit: f19b9165351a4058832bb43560178474c7501925\n-- \ngitgitgadget\n"},{"id":"498150","messageId":"20240706062839.GD700645@coredump.intra.peff.net","threadId":"61738","inReplyTo":"pull.1758.git.1720123341342.gitgitgadget@gmail.com","subject":"Re: [PATCH] merge-ort: fix missing early return","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2024-07-06T06:28:39Z","receivedAt":"2024-07-06T06:28:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jul 04, 2024 at 08:02:21PM +0000, Elijah Newren via GitGitGadget wrote:\n\n> From: Elijah Newren \n> \n> One of the conversions in 500433edf49 (\"merge-ort: convert more error()\n> cases to path_msg()\", 2024-06-10) accidentally lost the early return.\n> Restore it.\n\nThanks, this looks good to me.\n\n-Peff\n"},{"id":"498173","messageId":"xmqqr0c6nzjl.fsf@gitster.g","threadId":"61738","inReplyTo":"pull.1758.git.1720123341342.gitgitgadget@gmail.com","subject":"Re: [PATCH] merge-ort: fix missing early return","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-07-06T18:30:38Z","receivedAt":"2024-07-06T18:30:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Elijah Newren via GitGitGadget\" writes:\n\n> From: Elijah Newren \n>\n> One of the conversions in 500433edf49 (\"merge-ort: convert more error()\n> cases to path_msg()\", 2024-06-10) accidentally lost the early return.\n\nf19b9165 (merge-ort: convert more error() cases to path_msg(),\n2024-06-19) is what I have, date is different (and object name is,\ntoo, obviously).\n\nAnd funny thing is that your base-commit points at the latter. I\nbriefly wondered if we can somehow automate the generation of\nreference in the log message, but the base is likely to be the tip\nof the topic branch that has been accepted upstream, and the commit\nbeing fixed up can be something below, not at, that tip, so it\nwouldn't be like a simple and silly \"compare base-commit and the\ncommit we talk about in the proposed log message\".\n\nA commit-msg hook that scans for names that look like commit object names in the\nmessage, and\n\n - makes sure that these commits are reachable from HEAD (the goal\n is to make sure they are reachable from the resulting commit),\n and possibly\n\n - makes sure that these commits are reachable from @{u} (the goal\n is to catch references of unpublished commits)\n\nmight be a possibility, but such criteria are probably highly\nworkflow specific, so needs to be highly customizable if we wanted\nto make such a feature as a part of what we ship.\n\n> Restore it.\n>\n> Signed-off-by: Elijah Newren \n> ---\n\nThanks. I saw Peff's earlier message and the change exactly matches\nmy expectation. Will queue (with adjustments to the log message).\n\n> merge-ort: fix missing early return\n> \n> This is a patch on top of en/ort-inner-merge-error-fix which is in next.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1758%2Fnewren%2Ffix-fix-error-cases-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1758/newren/fix-fix-error-cases-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1758\n>\n> merge-ort.c | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/merge-ort.c b/merge-ort.c\n> index 8dfe80f1009..d9ba6e3e523 100644\n> --- a/merge-ort.c\n> +++ b/merge-ort.c\n> @@ -3618,6 +3618,7 @@ static int read_oid_strbuf(struct merge_options *opt,\n> \t\tpath_msg(opt, ERROR_OBJECT_NOT_A_BLOB, 0,\n> \t\t\t path, NULL, NULL, NULL,\n> \t\t\t _(\"error: object %s is not a blob\"), oid_to_hex(oid));\n> +\t\treturn -1;\n> \t}\n> \tstrbuf_attach(dst, buf, size, size + 1);\n> \treturn 0;\n>\n> base-commit: f19b9165351a4058832bb43560178474c7501925\n"},{"id":"498187","messageId":"CABPp-BEXgZ=igvNT2RFjYTAEappx-q=8oW3RD8+FFZDx1JxfJQ@mail.gmail.com","threadId":"61738","inReplyTo":"xmqqr0c6nzjl.fsf@gitster.g","subject":"Re: [PATCH] merge-ort: fix missing early return","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2024-07-07T02:50:21Z","receivedAt":"2024-07-07T02:50:34Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"On Sat, Jul 6, 2024 at 11:30 AM Junio C Hamano wrote:\n>\n> \"Elijah Newren via GitGitGadget\" writes:\n>\n> > From: Elijah Newren \n> >\n> > One of the conversions in 500433edf49 (\"merge-ort: convert more error()\n> > cases to path_msg()\", 2024-06-10) accidentally lost the early return.\n>\n> f19b9165 (merge-ort: convert more error() cases to path_msg(),\n> 2024-06-19) is what I have, date is different (and object name is,\n> too, obviously).\n\nDoh, sorry, I forgot to update the reference after rebasing.\n\n> And funny thing is that your base-commit points at the latter. I\n> briefly wondered if we can somehow automate the generation of\n> reference in the log message, but the base is likely to be the tip\n> of the topic branch that has been accepted upstream, and the commit\n> being fixed up can be something below, not at, that tip, so it\n> wouldn't be like a simple and silly \"compare base-commit and the\n> commit we talk about in the proposed log message\".\n>\n> A commit-msg hook that scans for names that look like commit object names in the\n> message, and\n>\n> - makes sure that these commits are reachable from HEAD (the goal\n> is to make sure they are reachable from the resulting commit),\n> and possibly\n>\n> - makes sure that these commits are reachable from @{u} (the goal\n> is to catch references of unpublished commits)\n>\n> might be a possibility, but such criteria are probably highly\n> workflow specific, so needs to be highly customizable if we wanted\n> to make such a feature as a part of what we ship.\n\nRight, and a commit-hook might not catch it either. Since my original\ncommit was based on the one I referenced in my commit message, the\noriginal commit creation was fine, but I realized after creating it\nthat I should have created my commit on top of your applied copy, so I\nneeded to rebase it. Since rebase sometimes tries to avoid invoking\n`git commit` (as per sequencer.c's try_to_commit()), that'd likely\nbypass the check anyway.\n\nAn alternative here is that I've thought about adding an ability for\nrebase to update commit references in the commit messages it\nrebases...but that may not have helped either in this particular case\nsince I was only rebasing the tip commit rather than the few commits\nbehind it as well.\n\n> > Restore it.\n> >\n> > Signed-off-by: Elijah Newren \n> > ---\n>\n> Thanks. I saw Peff's earlier message and the change exactly matches\n> my expectation. Will queue (with adjustments to the log message).\n\nThanks.\n"}]}