merge-ort: fix missing early return

From: Elijah Newren <[email protected]>

One of the conversions in 500433edf49 ("merge-ort: convert more error() cases to path_msg()", 2024-06-10) accidentally lost the early return. Restore it.

---
merge-ort: fix missing early return

This is a patch on top of en/ort-inner-merge-error-fix which is in next.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1758%2Fnewren%2Ffix-fix-error-cases-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1758/newren/fix-fix-error-cases-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1758

 merge-ort.c | 1 +
 1 file changed, 1 insertion(+)

index 8dfe80f1009..d9ba6e3e523 100644
@@ -3618,6 +3618,7 @@ static int read_oid_strbuf(struct merge_options *opt,
 		path_msg(opt, ERROR_OBJECT_NOT_A_BLOB, 0,
 			 path, NULL, NULL, NULL,
 			 _("error: object %s is not a blob"), oid_to_hex(oid));
+		return -1;
 	}
 	strbuf_attach(dst, buf, size, size + 1);
 	return 0;

base-commit: f19b9165351a4058832bb43560178474c7501925

gitgitgadget

Thanks, this looks good to me.

-Peff

f19b9165 (merge-ort: convert more error() cases to path_msg(), 2024-06-19) is what I have, date is different (and object name is, too, obviously).

And funny thing is that your base-commit points at the latter. I briefly wondered if we can somehow automate the generation of reference in the log message, but the base is likely to be the tip of the topic branch that has been accepted upstream, and the commit being fixed up can be something below, not at, that tip, so it wouldn't be like a simple and silly "compare base-commit and the commit we talk about in the proposed log message".

A commit-msg hook that scans for names that look like commit object names in the message, and

- makes sure that these commits are reachable from HEAD (the goal is to make sure they are reachable from the resulting commit), and possibly

- makes sure that these commits are reachable from @{u} (the goal is to catch references of unpublished commits)

might be a possibility, but such criteria are probably highly workflow specific, so needs to be highly customizable if we wanted to make such a feature as a part of what we ship.

Thanks. I saw Peff's earlier message and the change exactly matches my expectation. Will queue (with adjustments to the log message).

Doh, sorry, I forgot to update the reference after rebasing.

Right, and a commit-hook might not catch it either. Since my original commit was based on the one I referenced in my commit message, the original commit creation was fine, but I realized after creating it that I should have created my commit on top of your applied copy, so I needed to rebase it. Since rebase sometimes tries to avoid invoking git commit (as per sequencer.c's try_to_commit()), that'd likely bypass the check anyway.

An alternative here is that I've thought about adding an ability for rebase to update commit references in the commit messages it rebases...but that may not have helped either in this particular case since I was only rebasing the tip commit rather than the few commits behind it as well.

Thanks.