Merge pull request #3758 from libgit2/ethomson/annotated_commit_refs

Annotated commits: differentiate between the ref names and the description

src/annotated_commit.c

@@ -20,38 +20,101 @@
 
 static int annotated_commit_init(
 	git_annotated_commit **out,
+	git_commit *commit,
+	const char *description)
+{
+	git_annotated_commit *annotated_commit;
+	int error = 0;
+
+	assert(out && commit);
+
+	*out = NULL;
+
+	annotated_commit = git__calloc(1, sizeof(git_annotated_commit));
+	GITERR_CHECK_ALLOC(annotated_commit);
+
+	annotated_commit->type = GIT_ANNOTATED_COMMIT_REAL;
+
+	if ((error = git_commit_dup(&annotated_commit->commit, commit)) < 0)
+		goto done;
+
+	git_oid_fmt(annotated_commit->id_str, git_commit_id(commit));
+	annotated_commit->id_str[GIT_OID_HEXSZ] = '\0';
+
+	if (!description)
+		description = annotated_commit->id_str;
+
+	annotated_commit->description = git__strdup(description);
+	GITERR_CHECK_ALLOC(annotated_commit->description);
+
+done:
+	if (!error)
+		*out = annotated_commit;
+
+	return error;
+}
+
+static int annotated_commit_init_from_id(
+	git_annotated_commit **out,
 	git_repository *repo,
 	const git_oid *id,
-	const char *ref_name,
-	const char *remote_url)
+	const char *description)
 {
-	git_annotated_commit *annotated_commit;
 	git_commit *commit = NULL;
 	int error = 0;
 
-	assert(out && id);
+	assert(out && repo && id);
 
 	*out = NULL;
 
-	if ((error = git_commit_lookup(&commit, repo, id)) < 0 ||
-		(error = git_annotated_commit_from_commit(&annotated_commit,
-			commit)) < 0)
+	if ((error = git_commit_lookup(&commit, repo, id)) < 0)
 		goto done;
 
-	if (ref_name) {
-		annotated_commit->ref_name = git__strdup(ref_name);
-		GITERR_CHECK_ALLOC(annotated_commit->ref_name);
-	}
+	error = annotated_commit_init(out, commit, description);
+
+done:
+	git_commit_free(commit);
+	return error;
+}
+
+int git_annotated_commit_lookup(
+	git_annotated_commit **out,
+	git_repository *repo,
+	const git_oid *id)
+{
+	return annotated_commit_init_from_id(out, repo, id, NULL);
+}
+
+int git_annotated_commit_from_commit(
+	git_annotated_commit **out,
+	git_commit *commit)
+{
+	return annotated_commit_init(out, commit, NULL);
+}
 
-	if (remote_url) {
-		annotated_commit->remote_url = git__strdup(remote_url);
-		GITERR_CHECK_ALLOC(annotated_commit->remote_url);
+int git_annotated_commit_from_revspec(
+	git_annotated_commit **out,
+	git_repository *repo,
+	const char *revspec)
+{
+	git_object *obj, *commit;
+	int error;
+
+	assert(out && repo && revspec);
+
+	if ((error = git_revparse_single(&obj, repo, revspec)) < 0)
+		return error;
+
+	if ((error = git_object_peel(&commit, obj, GIT_OBJ_COMMIT))) {
+		git_object_free(obj);
+		return error;
 	}
 
-	*out = annotated_commit;
+	error = annotated_commit_init(out, (git_commit *)commit, revspec);
+
+	git_object_free(obj);
+	git_object_free(commit);
 
-done:
-	git_commit_free(commit);
 	return error;
 }
 
@@ -70,8 +133,15 @@ int git_annotated_commit_from_ref(
 	if ((error = git_reference_resolve(&resolved, ref)) < 0)
 		return error;
 	
-	error = annotated_commit_init(out, repo, git_reference_target(resolved),
-		git_reference_name(ref), NULL);
+	error = annotated_commit_init_from_id(out,
+		repo,
+		git_reference_target(resolved),
+		git_reference_name(ref));
+
+	if (!error) {
+		(*out)->ref_name = git__strdup(git_reference_name(ref));
+		GITERR_CHECK_ALLOC((*out)->ref_name);
+	}
 
 	git_reference_free(resolved);
 	return error;
@@ -97,41 +167,6 @@ int git_annotated_commit_from_head(
 	return error;
 }
 
-int git_annotated_commit_from_commit(
-	git_annotated_commit **out,
-	git_commit *commit)
-{
-	git_annotated_commit *annotated_commit;
-
-	assert(out && commit);
-
-	*out = NULL;
-
-	annotated_commit = git__calloc(1, sizeof(git_annotated_commit));
-	GITERR_CHECK_ALLOC(annotated_commit);
-
-	annotated_commit->type = GIT_ANNOTATED_COMMIT_REAL;
-
-	git_cached_obj_incref(commit);
-	annotated_commit->commit = commit;
-
-	git_oid_fmt(annotated_commit->id_str, git_commit_id(commit));
-	annotated_commit->id_str[GIT_OID_HEXSZ] = '\0';
-
-	*out = annotated_commit;
-	return 0;
-}
-
-int git_annotated_commit_lookup(
-	git_annotated_commit **out,
-	git_repository *repo,
-	const git_oid *id)
-{
-	assert(out && repo && id);
-
-	return annotated_commit_init(out, repo, id, NULL, NULL);
-}
-
 int git_annotated_commit_from_fetchhead(
 	git_annotated_commit **out,
 	git_repository *repo,
@@ -141,33 +176,16 @@ int git_annotated_commit_from_fetchhead(
 {
 	assert(repo && id && branch_name && remote_url);
 
-	return annotated_commit_init(out, repo, id, branch_name, remote_url);
-}
-
-int git_annotated_commit_from_revspec(
-	git_annotated_commit **out,
-	git_repository *repo,
-	const char *revspec)
-{
-	git_object *obj, *commit;
-	int error;
-
-	assert(out && repo && revspec);
-
-	if ((error = git_revparse_single(&obj, repo, revspec)) < 0)
-		return error;
-
-	if ((error = git_object_peel(&commit, obj, GIT_OBJ_COMMIT))) {
-		git_object_free(obj);
-		return error;
-	}
+	if (annotated_commit_init_from_id(out, repo, id, branch_name) < 0)
+		return -1;
 
-	error = annotated_commit_init(out, repo, git_object_id(commit), revspec, NULL);
+	(*out)->ref_name = git__strdup(branch_name);
+	GITERR_CHECK_ALLOC((*out)->ref_name);
 
-	git_object_free(obj);
-	git_object_free(commit);
+	(*out)->remote_url = git__strdup(remote_url);
+	GITERR_CHECK_ALLOC((*out)->remote_url);
 
-	return error;
+	return 0;
 }
 
 
@@ -187,8 +205,9 @@ void git_annotated_commit_free(git_annotated_commit *annotated_commit)
 		case GIT_ANNOTATED_COMMIT_REAL:
 			git_commit_free(annotated_commit->commit);
 			git_tree_free(annotated_commit->tree);
-			git__free(annotated_commit->ref_name);
-			git__free(annotated_commit->remote_url);
+			git__free((char *)annotated_commit->description);
+			git__free((char *)annotated_commit->ref_name);
+			git__free((char *)annotated_commit->remote_url);
 			break;
 		case GIT_ANNOTATED_COMMIT_VIRTUAL:
 			git_index_free(annotated_commit->index);

src/annotated_commit.h

@@ -33,8 +33,11 @@ struct git_annotated_commit {
 	git_index *index;
 	git_array_oid_t parents;
 
-	char *ref_name;
-	char *remote_url;
+	/* how this commit was looked up */
+	const char *description;
+
+	const char *ref_name;
+	const char *remote_url;
 
 	char id_str[GIT_OID_HEXSZ+1];
 };

src/branch.c

@@ -121,7 +121,8 @@ int git_branch_create_from_annotated(
 	const git_annotated_commit *commit,
 	int force)
 {
-	return create_branch(ref_out, repository, branch_name, commit->commit, commit->ref_name, force);
+	return create_branch(ref_out,
+		repository, branch_name, commit->commit, commit->description, force);
 }
 
 int git_branch_delete(git_reference *branch)

src/repository.c

@@ -2162,7 +2162,7 @@ int git_repository_set_head_detached_from_annotated(
 {
 	assert(repo && commitish);
 
-	return detach(repo, git_annotated_commit_id(commitish), commitish->ref_name);
+	return detach(repo, git_annotated_commit_id(commitish), commitish->description);
 }
 
 int git_repository_detach_head(git_repository* repo)

src/reset.c

@@ -195,5 +195,5 @@ int git_reset_from_annotated(
 	git_reset_t reset_type,
 	const git_checkout_options *checkout_opts)
 {
-	return reset(repo, (git_object *) commit->commit, commit->ref_name, reset_type, checkout_opts);
+	return reset(repo, (git_object *) commit->commit, commit->description, reset_type, checkout_opts);
 }

tests/rebase/abort.c

@@ -145,6 +145,25 @@ void test_rebase_abort__merge_by_id(void)
 	git_rebase_free(rebase);
 }
 
+void test_rebase_abort__merge_by_revspec(void)
+{
+	git_rebase *rebase;
+	git_annotated_commit *branch_head, *onto_head;
+
+	cl_git_pass(git_annotated_commit_from_revspec(&branch_head, repo, "b146bd7"));
+	cl_git_pass(git_annotated_commit_from_revspec(&onto_head, repo, "efad0b1"));
+	
+	cl_git_pass(git_rebase_init(&rebase, repo, branch_head, NULL, onto_head, NULL));
+	cl_assert_equal_i(GIT_REPOSITORY_STATE_REBASE_MERGE, git_repository_state(repo));
+
+	test_abort(branch_head, onto_head);
+
+	git_annotated_commit_free(branch_head);
+	git_annotated_commit_free(onto_head);
+
+	git_rebase_free(rebase);
+}
+
 void test_rebase_abort__merge_by_id_immediately_after_init(void)
 {
 	git_rebase *rebase;