Commit ff0d2220eb012a3bb8f5e583df7baad033a9397b

Stefan Sperling 2019-07-11T13:59:20

handle no-op changes during 'got rebase -c'

diff --git a/got/got.1 b/got/got.1
index 6c0eb6c..c9f0107 100644
--- a/got/got.1
+++ b/got/got.1
@@ -571,6 +571,9 @@ Alternatively, the rebase operation may be aborted which will leave
 .Ar branch
 unmodified and the work tree switched back to its original branch.
 .Pp
+If a merge conflict is resolved in a way which renders the merged
+change into a no-op change, the corresponding commit will be elided.
+.Pp
 .Cm got rebase
 will refuse to run if certain preconditions are not met.
 If the work tree contains multiple base commits it must first be updated
diff --git a/got/got.c b/got/got.c
index e6bdb5c..93c26bf 100644
--- a/got/got.c
+++ b/got/got.c
@@ -3207,9 +3207,11 @@ show_rebase_progress(struct got_commit_object *commit,
 	if (err)
 		goto done;
 
-	err = got_object_id_str(&new_id_str, new_id);
-	if (err)
-		goto done;
+	if (new_id) {
+		err = got_object_id_str(&new_id_str, new_id);
+		if (err)
+			goto done;
+	}
 
 	logmsg0 = strdup(got_object_commit_get_logmsg(commit));
 	if (logmsg0 == NULL) {
@@ -3222,7 +3224,8 @@ show_rebase_progress(struct got_commit_object *commit,
 		logmsg++;
 
 	old_id_str[12] = '\0';
-	new_id_str[12] = '\0';
+	if (new_id_str)
+		new_id_str[12] = '\0';
 	len = strlen(logmsg);
 	if (len > 42)
 		len = 42;
@@ -3230,7 +3233,8 @@ show_rebase_progress(struct got_commit_object *commit,
 	nl = strchr(logmsg, '\n');
 	if (nl)
 		*nl = '\0';
-	printf("%s -> %s: %s\n", old_id_str, new_id_str, logmsg);
+	printf("%s -> %s: %s\n", old_id_str,
+	    new_id_str ? new_id_str : "no-op change", logmsg);
 done:
 	free(old_id_str);
 	free(new_id_str);
@@ -3264,6 +3268,33 @@ rebase_complete(struct got_worktree *worktree, struct got_reference *branch,
 }
 
 static const struct got_error *
+rebase_commit(struct got_worktree *worktree, struct got_reference *tmp_branch,
+   struct got_object_id *commit_id, struct got_repository *repo)
+{
+	const struct got_error *error;
+	struct got_commit_object *commit;
+	struct got_object_id *new_commit_id;
+
+	error = got_object_open_as_commit(&commit, repo, commit_id);
+	if (error)
+		return error;
+
+	error = got_worktree_rebase_commit(&new_commit_id, worktree,
+	    tmp_branch, commit, commit_id, repo);
+	if (error) {
+		if (error->code != GOT_ERR_COMMIT_NO_CHANGES)
+			goto done;
+		error = show_rebase_progress(commit, commit_id, NULL);
+	} else {
+		error = show_rebase_progress(commit, commit_id, new_commit_id);
+		free(new_commit_id);
+	}
+done:
+	got_object_commit_close(commit);
+	return error;
+}
+
+static const struct got_error *
 cmd_rebase(int argc, char *argv[])
 {
 	const struct got_error *error = NULL;
@@ -3353,52 +3384,21 @@ cmd_rebase(int argc, char *argv[])
 	}
 
 	if (continue_rebase) {
-		struct got_object_id *new_commit_id;
-
 		error = got_worktree_rebase_continue(&resume_commit_id,
 		    &new_base_branch, &tmp_branch, &branch, worktree, repo);
 		if (error)
 			goto done;
-		error = got_commit_graph_find_youngest_common_ancestor(&yca_id,
-		    got_worktree_get_base_commit_id(worktree),
-		    resume_commit_id, repo);
-		if (error)
-			goto done;
-		if (yca_id == NULL) {
-			error = got_error_msg(GOT_ERR_ANCESTRY,
-			    "cannot determine common ancestor commit for "
-			    "continued rebase operation");
-			goto done;
-		}
-		error = got_object_open_as_commit(&commit, repo,
-		    resume_commit_id);
-		if (error)
-			goto done;
 
-		error = got_worktree_rebase_commit(&new_commit_id, worktree,
-		    tmp_branch, commit, resume_commit_id, repo);
+		error = rebase_commit(worktree, tmp_branch, resume_commit_id,
+		    repo);
 		if (error)
 			goto done;
-		error = show_rebase_progress(commit, resume_commit_id,
-		    new_commit_id);
-		free(new_commit_id);
-		free(resume_commit_id);
 
-		resume_commit_id = got_object_id_dup(SIMPLEQ_FIRST(
-		    got_object_commit_get_parent_ids(commit))->id);
-		if (resume_commit_id == NULL) {
+		yca_id = got_object_id_dup(resume_commit_id);
+		if (yca_id == NULL) {
 			error = got_error_from_errno("got_object_id_dup");
 			goto done;
 		}
-		got_object_commit_close(commit);
-		commit = NULL;
-		commit_id = resume_commit_id;
-		if (got_object_id_cmp(resume_commit_id, yca_id) == 0) {
-			error = rebase_complete(worktree, branch,
-			    new_base_branch, tmp_branch, repo);
-			/* YCA has been reached; we are done. */
-			goto done;
-		}
 	} else {
 		error = got_ref_open(&branch, repo, argv[0], 0);
 		if (error != NULL)
@@ -3416,10 +3416,13 @@ cmd_rebase(int argc, char *argv[])
 			    "is already contained in work tree's branch");
 			goto done;
 		}
+	}
 
-		error = got_ref_resolve(&branch_head_commit_id, repo, branch);
-		if (error)
-			goto done;
+	error = got_ref_resolve(&branch_head_commit_id, repo, branch);
+	if (error)
+		goto done;
+
+	if (!continue_rebase) {
 		error = got_commit_graph_find_youngest_common_ancestor(&yca_id,
 		    got_worktree_get_base_commit_id(worktree),
 		    branch_head_commit_id, repo);
@@ -3436,9 +3439,9 @@ cmd_rebase(int argc, char *argv[])
 		    &tmp_branch, worktree, branch, repo);
 		if (error)
 			goto done;
-		commit_id = branch_head_commit_id;
 	}
 
+	commit_id = branch_head_commit_id;
 	error = got_object_open_as_commit(&commit, repo, commit_id);
 	if (error)
 		goto done;
@@ -3477,14 +3480,16 @@ cmd_rebase(int argc, char *argv[])
 	}
 
 	if (SIMPLEQ_EMPTY(&commits)) {
-		error = got_error(GOT_ERR_EMPTY_REBASE);
+		if (continue_rebase)
+			error = rebase_complete(worktree, branch,
+			    new_base_branch, tmp_branch, repo);
+		else
+			error = got_error(GOT_ERR_EMPTY_REBASE);
 		goto done;
 	}
 
 	pid = NULL;
 	SIMPLEQ_FOREACH(qid, &commits, entry) {
-		struct got_object_id *new_commit_id;
-
 		commit_id = qid->id;
 		parent_id = pid ? pid->id : yca_id;
 		pid = qid;
@@ -3498,20 +3503,9 @@ cmd_rebase(int argc, char *argv[])
 		if (rebase_status == GOT_STATUS_CONFLICT)
 			break;
 
-		error = got_object_open_as_commit(&commit, repo, commit_id);
-		if (error)
-			goto done;
-		error = got_worktree_rebase_commit(&new_commit_id, worktree,
-		    tmp_branch, commit, commit_id, repo);
+		error = rebase_commit(worktree, tmp_branch, commit_id, repo);
 		if (error)
 			goto done;
-		error = show_rebase_progress(commit, commit_id, new_commit_id);
-		free(new_commit_id);
-		got_object_commit_close(commit);
-		commit = NULL;
-		if (error)
-			goto done;
-
 	}
 
 	if (rebase_status == GOT_STATUS_CONFLICT) {
diff --git a/lib/worktree.c b/lib/worktree.c
index 4d551f4..d2d0842 100644
--- a/lib/worktree.c
+++ b/lib/worktree.c
@@ -3803,8 +3803,16 @@ got_worktree_rebase_commit(struct got_object_id **new_commit_id,
 	    got_object_commit_get_committer(orig_commit),
 	    collect_rebase_commit_msg, orig_commit,
 	    rebase_status, NULL, repo);
-	if (err)
+	if (err) {
+		if (err->code == GOT_ERR_COMMIT_NO_CHANGES) {
+			/* No-op change; commit will be elided. */
+			err = got_ref_delete(commit_ref, repo);
+			if (err)
+				goto done;
+			err = got_error(GOT_ERR_COMMIT_NO_CHANGES);
+		}
 		goto done;
+	}
 
 	err = got_ref_delete(commit_ref, repo);
 	if (err)
diff --git a/regress/cmdline/rebase.sh b/regress/cmdline/rebase.sh
index ca79164..e1eaa01 100755
--- a/regress/cmdline/rebase.sh
+++ b/regress/cmdline/rebase.sh
@@ -387,7 +387,113 @@ function test_rebase_abort {
 	test_done "$testroot" "$ret"
 }
 
-run_test test_rebase_basic
-run_test test_rebase_ancestry_check
-run_test test_rebase_continue
-run_test test_rebase_abort
+function test_rebase_no_op_change {
+	local testroot=`test_init rebase_no_op_change`
+	local init_commit=`git_show_head $testroot/repo`
+
+	(cd $testroot/repo && git checkout -q -b newbranch)
+	echo "modified alpha on branch" > $testroot/repo/alpha
+	git_commit $testroot/repo -m "committing to alpha on newbranch"
+	local orig_commit1=`git_show_head $testroot/repo`
+
+	(cd $testroot/repo && git checkout -q master)
+	echo "modified alpha on master" > $testroot/repo/alpha
+	git_commit $testroot/repo -m "committing to alpha on master"
+	local master_commit=`git_show_head $testroot/repo`
+
+	got checkout $testroot/repo $testroot/wt > /dev/null
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	(cd $testroot/wt && got rebase newbranch > $testroot/stdout \
+		2> $testroot/stderr)
+
+	echo "C  alpha" > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "got: conflicts must be resolved before rebase can be resumed" \
+		> $testroot/stderr.expected
+	cmp -s $testroot/stderr.expected $testroot/stderr
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stderr.expected $testroot/stderr
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "<<<<<<< commit $orig_commit1" > $testroot/content.expected
+	echo "modified alpha on branch" >> $testroot/content.expected
+	echo "=======" >> $testroot/content.expected
+	echo "modified alpha on master" >> $testroot/content.expected
+	echo '>>>>>>> alpha' >> $testroot/content.expected
+	cat $testroot/wt/alpha > $testroot/content
+	cmp -s $testroot/content.expected $testroot/content
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/content.expected $testroot/content
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	(cd $testroot/wt && got status > $testroot/stdout)
+
+	echo "C  alpha" > $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# resolve the conflict
+	echo "modified alpha on master" > $testroot/wt/alpha
+
+	(cd $testroot/wt && got rebase -c > $testroot/stdout)
+
+	(cd $testroot/repo && git checkout -q newbranch)
+	local new_commit1=`git_show_head $testroot/repo`
+
+	local short_orig_commit1=`trim_obj_id 28 $orig_commit1`
+
+	echo -n "$short_orig_commit1 -> no-op change" \
+		> $testroot/stdout.expected
+	echo ": committing to alpha on newbranch" >> $testroot/stdout.expected
+	echo "Switching work tree to refs/heads/newbranch" \
+		>> $testroot/stdout.expected
+
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+
+	(cd $testroot/wt && got log -l2 | grep ^commit > $testroot/stdout)
+	echo "commit $master_commit (master, newbranch)" \
+		> $testroot/stdout.expected
+	echo "commit $init_commit" >> $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret="$?"
+	if [ "$ret" != "0" ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
+	test_done "$testroot" "$ret"
+}
+
+#run_test test_rebase_basic
+#run_test test_rebase_ancestry_check
+#run_test test_rebase_continue
+#run_test test_rebase_abort
+run_test test_rebase_no_op_change