As far as I can tell, the below patch doesn't change any behaviour
except maybe to save a bit of time.

I added a comment to got_commit_graph_find_youngest_ancestor mostly to
make it easier to verify that yca_id can't be NULL when the new line
11313 is reached.

The calls to check_same_branch that I replaced were equivalent to just
checking whether one commit was the youngest common ancestor of both. It
is complicated a bit by whether non-linear ancestors were considered,
i.e. the first_parent_traversal argument to
got_commit_graph_find_youngest_common_ancestor, but I believe it all
works out:

- The check in cmd_rebase only considers linear history, before and
  after this change.

- The check in cmd_merge considers full history, both before and after
  this change. (The reason it considered full history before this change
  is that one of the first things check_same_branch did was to compare
  commit_id to yca_id, and yca_id was based on full history.)

I was working on making "got merge" fast-forward when possible as
discussed in the other thread when I noticed this apparently unneeded
complexity. If this change is correct, I think it makes sense to do it
first as a separate change, because it's already tricky enough to reason
about on its own.

-- 
James


diff b156a14e478c9485a080a24f237f70b6c8984182 refs/heads/mg_simplify_br
commit - b156a14e478c9485a080a24f237f70b6c8984182
commit + 8b32aba2f499aa19f79a72f5bc04a327c6ff5c26
blob - 8d34fc2c0463ba9b0482b97a427cd03c82b875d8
blob + 123dedc51cae9b07a29394e9816496121697cf7a
--- got/got.c
+++ got/got.c
@@ -2902,26 +2902,18 @@ check_same_branch(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;
+	if (got_object_id_cmp(head_commit_id, commit_id) == 0)
 		goto done;
-	}
-	if (yca_id && got_object_id_cmp(commit_id, yca_id) == 0) {
-		is_same_branch = 1;
-		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)
+		if (got_object_id_cmp(&id, commit_id) == 0)
 			break;
-		if (got_object_id_cmp(&id, commit_id) == 0) {
-			is_same_branch = 1;
-			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 + f25ec1e90750a30eab3d01e03d67faabeaefb27b
--- lib/commit_graph.c
+++ lib/commit_graph.c
@@ -643,6 +643,12 @@ const struct got_error *
 	return got_object_idset_add(commit_ids, &id, NULL);
 }
 
+/*
+ * Sets *yca_id to the last commit that can be reached from both commit_id and
+ * commit_id2. Returns got_error(GOT_ERR_ANCESTRY) if there are none.
+ *
+ * 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,