commit ad575c3a0f6a3c278002356c0a2cf0142cf1177a
from: James Cook <falsifian@falsifian.org>
via: Stefan Sperling <stsp@stsp.name>
date: Thu May 25 07:14:37 2023 UTC

simplify ancestry checks in checkout, update, rebase, and merge

No behaviour change as the end result of the rewritten checks should
be the same as before. We are just doing less work where possible.

Patch by James Cook

commit - c034b06627303c06e0d334eeaf3d36bc3da74325
commit + ad575c3a0f6a3c278002356c0a2cf0142cf1177a
blob - 8d34fc2c0463ba9b0482b97a427cd03c82b875d8
blob + 123dedc51cae9b07a29394e9816496121697cf7a
--- got/got.c
+++ got/got.c
@@ -2902,26 +2902,18 @@ check_linear_ancestry(struct got_object_id *commit_id,
 
 static const struct got_error *
 check_same_branch(struct got_object_id *commit_id,
-    struct got_reference *head_ref, struct got_object_id *yca_id,
-    struct got_repository *repo)
+    struct got_reference *head_ref, struct got_repository *repo)
 {
 	const struct got_error *err = NULL;
 	struct got_commit_graph *graph = NULL;
 	struct got_object_id *head_commit_id = NULL;
-	int is_same_branch = 0;
 
 	err = got_ref_resolve(&head_commit_id, repo, head_ref);
 	if (err)
 		goto done;
 
-	if (got_object_id_cmp(head_commit_id, commit_id) == 0) {
-		is_same_branch = 1;
-		goto done;
-	}
-	if (yca_id && got_object_id_cmp(commit_id, yca_id) == 0) {
-		is_same_branch = 1;
+	if (got_object_id_cmp(head_commit_id, commit_id) == 0)
 		goto done;
-	}
 
 	err = got_commit_graph_open(&graph, "/", 1);
 	if (err)
@@ -2939,23 +2931,17 @@ check_same_branch(struct got_object_id *commit_id,
 		    check_cancelled, NULL);
 		if (err) {
 			if (err->code == GOT_ERR_ITER_COMPLETED)
-				err = NULL;
+				err = got_error(GOT_ERR_ANCESTRY);
 			break;
 		}
 
-		if (yca_id && got_object_id_cmp(&id, yca_id) == 0)
-			break;
-		if (got_object_id_cmp(&id, commit_id) == 0) {
-			is_same_branch = 1;
+		if (got_object_id_cmp(&id, commit_id) == 0)
 			break;
-		}
 	}
 done:
 	if (graph)
 		got_commit_graph_close(graph);
 	free(head_commit_id);
-	if (!err && !is_same_branch)
-		err = got_error(GOT_ERR_ANCESTRY);
 	return err;
 }
 
@@ -3158,7 +3144,7 @@ cmd_checkout(int argc, char *argv[])
 			}
 			goto done;
 		}
-		error = check_same_branch(commit_id, head_ref, NULL, repo);
+		error = check_same_branch(commit_id, head_ref, repo);
 		if (error) {
 			if (error->code == GOT_ERR_ANCESTRY) {
 				error = checkout_ancestry_error(
@@ -3614,7 +3600,7 @@ cmd_update(int argc, char *argv[])
 		free(head_commit_id);
 		if (error != NULL)
 			goto done;
-		error = check_same_branch(commit_id, head_ref, NULL, repo);
+		error = check_same_branch(commit_id, head_ref, repo);
 		if (error)
 			goto done;
 		error = switch_head_ref(head_ref, commit_id, worktree, repo);
@@ -3628,7 +3614,7 @@ cmd_update(int argc, char *argv[])
 				error = got_error(GOT_ERR_BRANCH_MOVED);
 			goto done;
 		}
-		error = check_same_branch(commit_id, head_ref, NULL, repo);
+		error = check_same_branch(commit_id, head_ref, repo);
 		if (error)
 			goto done;
 	}
@@ -11324,12 +11310,7 @@ cmd_rebase(int argc, char *argv[])
 			goto done;
 		}
 
-		error = check_same_branch(base_commit_id, branch, yca_id, repo);
-		if (error) {
-			if (error->code != GOT_ERR_ANCESTRY)
-				goto done;
-			error = NULL;
-		} else {
+		if (got_object_id_cmp(base_commit_id, yca_id) == 0) {
 			struct got_pathlist_head paths;
 			printf("%s is already based on %s\n",
 			    got_ref_get_name(branch),
@@ -13323,24 +13304,16 @@ cmd_merge(int argc, char *argv[])
 		    GOT_ERR_MERGE_PATH, repo);
 		if (error)
 			goto done;
-		if (yca_id) {
-			error = check_same_branch(wt_branch_tip, branch,
-			    yca_id, repo);
-			if (error) {
-				if (error->code != GOT_ERR_ANCESTRY)
-					goto done;
-				error = NULL;
-			} else {
-				static char msg[512];
-				snprintf(msg, sizeof(msg),
-				    "cannot create a merge commit because "
-				    "%s is based on %s; %s can be integrated "
-				    "with 'got integrate' instead", branch_name,
-				    got_worktree_get_head_ref_name(worktree),
-				    branch_name);
-				error = got_error_msg(GOT_ERR_SAME_BRANCH, msg);
-				goto done;
-			}
+		if (yca_id && got_object_id_cmp(wt_branch_tip, yca_id) == 0) {
+			static char msg[512];
+			snprintf(msg, sizeof(msg),
+			    "cannot create a merge commit because %s is based "
+			    "on %s; %s can be integrated with 'got integrate' "
+			    "instead", branch_name,
+			    got_worktree_get_head_ref_name(worktree),
+			    branch_name);
+			error = got_error_msg(GOT_ERR_SAME_BRANCH, msg);
+			goto done;
 		}
 		error = got_worktree_merge_prepare(&fileindex, worktree,
 		    branch, repo);
blob - 45ed91c002a5e8a63378941757461aee195a0547
blob + 60e80af88cd8b83c0792aa2a88a615267cd87994
--- lib/commit_graph.c
+++ lib/commit_graph.c
@@ -643,6 +643,13 @@ find_yca_add_id(struct got_object_id **yca_id, struct 
 	return got_object_idset_add(commit_ids, &id, NULL);
 }
 
+/*
+ * Sets *yca_id to the youngest common ancestor of commit_id and
+ * commit_id2. Returns got_error(GOT_ERR_ANCESTRY) if they have no
+ * common ancestors.
+ *
+ * If first_parent_traversal is nonzero, only linear history is considered.
+ */
 const struct got_error *
 got_commit_graph_find_youngest_common_ancestor(struct got_object_id **yca_id,
     struct got_object_id *commit_id, struct got_object_id *commit_id2,