diff --git a/include/git2/commit.h b/include/git2/commit.h index 8b03df0a9..834330b5d 100644 --- a/include/git2/commit.h +++ b/include/git2/commit.h @@ -242,8 +242,8 @@ GIT_EXTERN(int) git_commit_nth_gen_ancestor( /** * Create new commit in the repository from a list of `git_object` pointers * - * The message will not be cleaned up automatically. You can do that with - * the `git_message_prettify()` function. + * The message will **not** be cleaned up automatically. You can do that + * with the `git_message_prettify()` function. * * @param id Pointer in which to store the OID of the newly created commit * @@ -291,20 +291,20 @@ GIT_EXTERN(int) git_commit_create( const char *message_encoding, const char *message, const git_tree *tree, - int parent_count, + size_t parent_count, const git_commit *parents[]); /** * Create new commit in the repository using a variable argument list. * - * The message will be cleaned up from excess whitespace and it will be made - * sure that the last line ends with a '\n'. + * The message will **not** be cleaned up automatically. You can do that + * with the `git_message_prettify()` function. * * The parents for the commit are specified as a variable list of pointers * to `const git_commit *`. Note that this is a convenience method which may * not be safe to export for certain languages or compilers * - * All other parameters remain the same at `git_commit_create()`. + * All other parameters remain the same as `git_commit_create()`. * * @see git_commit_create */ @@ -317,9 +317,40 @@ GIT_EXTERN(int) git_commit_create_v( const char *message_encoding, const char *message, const git_tree *tree, - int parent_count, + size_t parent_count, ...); +/** + * Amend an existing commit by replacing only non-NULL values. + * + * This creates a new commit that is exactly the same as the old commit, + * except that any non-NULL values will be updated. The new commit has + * the same parents as the old commit. + * + * The `update_ref` value works as in the regular `git_commit_create()`, + * updating the ref to point to the newly rewritten commit. If you want + * to amend a commit that is not currently the HEAD of the branch and then + * rewrite the following commits to reach a ref, pass this as NULL and + * update the rest of the commit chain and ref separately. + * + * Unlike `git_commit_create()`, the `author`, `committer`, `message`, + * `message_encoding`, and `tree` parameters can be NULL in which case this + * will use the values from the original `commit_to_amend`. + * + * All parameters have the same meanings as in `git_commit_create()`. + * + * @see git_commit_create + */ +GIT_EXTERN(int) git_commit_amend( + git_oid *id, + const git_commit *commit_to_amend, + const char *update_ref, + const git_signature *author, + const git_signature *committer, + const char *message_encoding, + const char *message, + const git_tree *tree); + /** @} */ GIT_END_DECL #endif diff --git a/include/git2/sys/commit.h b/include/git2/sys/commit.h index c8ed56b66..627d3ae2e 100644 --- a/include/git2/sys/commit.h +++ b/include/git2/sys/commit.h @@ -21,16 +21,18 @@ GIT_BEGIN_DECL /** - * Create new commit in the repository from a list of `git_oid` values + * Create new commit in the repository from a list of `git_oid` values. * * See documentation for `git_commit_create()` for information about the * parameters, as the meaning is identical excepting that `tree` and * `parents` now take `git_oid`. This is a dangerous API in that nor * the `tree`, neither the `parents` list of `git_oid`s are checked for * validity. + * + * @see git_commit_create */ GIT_EXTERN(int) git_commit_create_from_ids( - git_oid *oid, + git_oid *id, git_repository *repo, const char *update_ref, const git_signature *author, @@ -38,9 +40,41 @@ GIT_EXTERN(int) git_commit_create_from_ids( const char *message_encoding, const char *message, const git_oid *tree, - int parent_count, + size_t parent_count, const git_oid *parents[]); +/** + * Callback function to return parents for commit. + * + * This is invoked with the count of the number of parents processed so far + * along with the user supplied payload. This should return a git_oid of + * the next parent or NULL if all parents have been provided. + */ +typedef const git_oid *(*git_commit_parent_callback)(size_t idx, void *payload); + +/** + * Create a new commit in the repository with an callback to supply parents. + * + * See documentation for `git_commit_create()` for information about the + * parameters, as the meaning is identical excepting that `tree` takes a + * `git_oid` and doesn't check for validity, and `parent_cb` is invoked + * with `parent_payload` and should return `git_oid` values or NULL to + * indicate that all parents are accounted for. + * + * @see git_commit_create + */ +GIT_EXTERN(int) git_commit_create_from_callback( + git_oid *id, + git_repository *repo, + const char *update_ref, + const git_signature *author, + const git_signature *committer, + const char *message_encoding, + const char *message, + const git_oid *tree, + git_commit_parent_callback parent_cb, + void *parent_payload); + /** @} */ GIT_END_DECL #endif diff --git a/src/commit.c b/src/commit.c index d66cf0c6a..de50e772e 100644 --- a/src/commit.c +++ b/src/commit.c @@ -36,41 +36,8 @@ void git_commit__free(void *_commit) git__free(commit); } -int git_commit_create_v( - git_oid *oid, - git_repository *repo, - const char *update_ref, - const git_signature *author, - const git_signature *committer, - const char *message_encoding, - const char *message, - const git_tree *tree, - int parent_count, - ...) -{ - va_list ap; - int i, res; - const git_commit **parents; - - parents = git__malloc(parent_count * sizeof(git_commit *)); - GITERR_CHECK_ALLOC(parents); - - va_start(ap, parent_count); - for (i = 0; i < parent_count; ++i) - parents[i] = va_arg(ap, const git_commit *); - va_end(ap); - - res = git_commit_create( - oid, repo, update_ref, author, committer, - message_encoding, message, - tree, parent_count, parents); - - git__free((void *)parents); - return res; -} - -int git_commit_create_from_ids( - git_oid *oid, +int git_commit_create_from_callback( + git_oid *id, git_repository *repo, const char *update_ref, const git_signature *author, @@ -78,19 +45,20 @@ int git_commit_create_from_ids( const char *message_encoding, const char *message, const git_oid *tree, - int parent_count, - const git_oid *parents[]) + git_commit_parent_callback parent_cb, + void *parent_payload) { git_buf commit = GIT_BUF_INIT; - int i; + size_t i = 0; git_odb *odb; + const git_oid *parent; - assert(oid && repo && tree && parent_count >= 0); + assert(id && repo && tree && parent_cb); git_oid__writebuf(&commit, "tree ", tree); - for (i = 0; i < parent_count; ++i) - git_oid__writebuf(&commit, "parent ", parents[i]); + while ((parent = parent_cb(i++, parent_payload)) != NULL) + git_oid__writebuf(&commit, "parent ", parent); git_signature__writebuf(&commit, "author ", author); git_signature__writebuf(&commit, "committer ", committer); @@ -106,7 +74,7 @@ int git_commit_create_from_ids( if (git_repository_odb__weakptr(&odb, repo) < 0) goto on_error; - if (git_odb_write(oid, odb, commit.ptr, commit.size, GIT_OBJ_COMMIT) < 0) + if (git_odb_write(id, odb, commit.ptr, commit.size, GIT_OBJ_COMMIT) < 0) goto on_error; git_buf_free(&commit); @@ -117,7 +85,7 @@ int git_commit_create_from_ids( const char *shortmsg; git_buf reflog_msg = GIT_BUF_INIT; - if (git_commit_lookup(&c, repo, oid) < 0) + if (git_commit_lookup(&c, repo, id) < 0) goto on_error; shortmsg = git_commit_summary(c); @@ -126,7 +94,7 @@ int git_commit_create_from_ids( shortmsg); git_commit_free(c); - error = git_reference__update_terminal(repo, update_ref, oid, + error = git_reference__update_terminal(repo, update_ref, id, committer, git_buf_cstr(&reflog_msg)); git_buf_free(&reflog_msg); @@ -141,8 +109,23 @@ on_error: return -1; } -int git_commit_create( - git_oid *oid, +typedef struct { + size_t total; + va_list args; +} commit_parent_varargs; + +static const git_oid *commit_parent_from_varargs(size_t curr, void *payload) +{ + commit_parent_varargs *data = payload; + const git_commit *commit; + if (curr >= data->total) + return NULL; + commit = va_arg(data->args, const git_commit *); + return commit ? git_commit_id(commit) : NULL; +} + +int git_commit_create_v( + git_oid *id, git_repository *repo, const char *update_ref, const git_signature *author, @@ -150,31 +133,144 @@ int git_commit_create( const char *message_encoding, const char *message, const git_tree *tree, - int parent_count, + size_t parent_count, + ...) +{ + int error = 0; + commit_parent_varargs data; + + assert(tree && git_tree_owner(tree) == repo); + + data.total = parent_count; + va_start(data.args, parent_count); + + error = git_commit_create_from_callback( + id, repo, update_ref, author, committer, + message_encoding, message, git_tree_id(tree), + commit_parent_from_varargs, &data); + + va_end(data.args); + return error; +} + +typedef struct { + size_t total; + const git_oid **parents; +} commit_parent_oids; + +static const git_oid *commit_parent_from_ids(size_t curr, void *payload) +{ + commit_parent_oids *data = payload; + return (curr < data->total) ? data->parents[curr] : NULL; +} + +int git_commit_create_from_ids( + git_oid *id, + git_repository *repo, + const char *update_ref, + const git_signature *author, + const git_signature *committer, + const char *message_encoding, + const char *message, + const git_oid *tree, + size_t parent_count, + const git_oid *parents[]) +{ + commit_parent_oids data = { parent_count, parents }; + + return git_commit_create_from_callback( + id, repo, update_ref, author, committer, + message_encoding, message, tree, + commit_parent_from_ids, &data); +} + +typedef struct { + size_t total; + const git_commit **parents; + git_repository *repo; +} commit_parent_data; + +static const git_oid *commit_parent_from_array(size_t curr, void *payload) +{ + commit_parent_data *data = payload; + const git_commit *commit; + if (curr >= data->total) + return NULL; + commit = data->parents[curr]; + if (git_commit_owner(commit) != data->repo) + return NULL; + return git_commit_id(commit); +} + +int git_commit_create( + git_oid *id, + git_repository *repo, + const char *update_ref, + const git_signature *author, + const git_signature *committer, + const char *message_encoding, + const char *message, + const git_tree *tree, + size_t parent_count, const git_commit *parents[]) { - int retval, i; - const git_oid **parent_oids; + commit_parent_data data = { parent_count, parents, repo }; - assert(parent_count >= 0); - assert(git_object_owner((const git_object *)tree) == repo); + assert(tree && git_tree_owner(tree) == repo); - parent_oids = git__malloc(parent_count * sizeof(git_oid *)); - GITERR_CHECK_ALLOC(parent_oids); + return git_commit_create_from_callback( + id, repo, update_ref, author, committer, + message_encoding, message, git_tree_id(tree), + commit_parent_from_array, &data); +} - for (i = 0; i < parent_count; ++i) { - assert(git_object_owner((const git_object *)parents[i]) == repo); - parent_oids[i] = git_object_id((const git_object *)parents[i]); +static const git_oid *commit_parent_for_amend(size_t curr, void *payload) +{ + const git_commit *commit_to_amend = payload; + if (curr >= git_array_size(commit_to_amend->parent_ids)) + return NULL; + return git_array_get(commit_to_amend->parent_ids, curr); +} + +int git_commit_amend( + git_oid *id, + const git_commit *commit_to_amend, + const char *update_ref, + const git_signature *author, + const git_signature *committer, + const char *message_encoding, + const char *message, + const git_tree *tree) +{ + git_repository *repo; + git_oid tree_id; + + assert(id && commit_to_amend); + + repo = git_commit_owner(commit_to_amend); + + if (!author) + author = git_commit_author(commit_to_amend); + if (!committer) + committer = git_commit_committer(commit_to_amend); + if (!message_encoding) + message_encoding = git_commit_message_encoding(commit_to_amend); + if (!message) + message = git_commit_message(commit_to_amend); + + if (!tree) { + git_tree *old_tree; + GITERR_CHECK_ERROR( git_commit_tree(&old_tree, commit_to_amend) ); + git_oid_cpy(&tree_id, git_tree_id(old_tree)); + git_tree_free(old_tree); + } else { + assert(git_tree_owner(tree) == repo); + git_oid_cpy(&tree_id, git_tree_id(tree)); } - retval = git_commit_create_from_ids( - oid, repo, update_ref, author, committer, - message_encoding, message, - git_object_id((const git_object *)tree), parent_count, parent_oids); - - git__free((void *)parent_oids); - - return retval; + return git_commit_create_from_callback( + id, repo, update_ref, author, committer, message_encoding, message, + &tree_id, commit_parent_for_amend, (void *)commit_to_amend); } int git_commit__parse(void *_commit, git_odb_object *odb_obj) @@ -314,10 +410,9 @@ const char *git_commit_summary(git_commit *commit) git_buf_putc(&summary, *msg); } - if (summary.asize == 0) + commit->summary = git_buf_detach(&summary); + if (!commit->summary) commit->summary = git__strdup(""); - else - commit->summary = git_buf_detach(&summary); } return commit->summary; diff --git a/src/signature.c b/src/signature.c index f658d6035..f501cd8b6 100644 --- a/src/signature.c +++ b/src/signature.c @@ -230,6 +230,8 @@ void git_signature__writebuf(git_buf *buf, const char *header, const git_signatu int offset, hours, mins; char sign; + assert(buf && sig); + offset = sig->when.offset; sign = (sig->when.offset < 0) ? '-' : '+'; diff --git a/tests/object/commit/commitstagedfile.c b/tests/object/commit/commitstagedfile.c index fbeeccbbd..3e7b3c02c 100644 --- a/tests/object/commit/commitstagedfile.c +++ b/tests/object/commit/commitstagedfile.c @@ -132,3 +132,79 @@ void test_object_commit_commitstagedfile__generate_predictable_object_ids(void) git_tree_free(tree); git_index_free(index); } + +static void assert_commit_tree_has_n_entries(git_commit *c, int count) +{ + git_tree *tree; + cl_git_pass(git_commit_tree(&tree, c)); + cl_assert_equal_i(count, git_tree_entrycount(tree)); + git_tree_free(tree); +} + +static void assert_commit_is_head_(git_commit *c, const char *file, int line) +{ + git_commit *head; + cl_git_pass(git_revparse_single((git_object **)&head, repo, "HEAD")); + clar__assert(git_oid_equal(git_commit_id(c), git_commit_id(head)), file, line, "Commit is not the HEAD", NULL, 1); + git_commit_free(head); +} +#define assert_commit_is_head(C) assert_commit_is_head_((C),__FILE__,__LINE__) + +void test_object_commit_commitstagedfile__amend_commit(void) +{ + git_index *index; + git_oid old_oid, new_oid, tree_oid; + git_commit *old_commit, *new_commit; + git_tree *tree; + + /* make a commit */ + + cl_git_mkfile("treebuilder/myfile", "This is a file\n"); + cl_git_pass(git_repository_index(&index, repo)); + cl_git_pass(git_index_add_bypath(index, "myfile")); + cl_repo_commit_from_index(&old_oid, repo, NULL, 0, "first commit"); + + cl_git_pass(git_commit_lookup(&old_commit, repo, &old_oid)); + + cl_assert_equal_i(0, git_commit_parentcount(old_commit)); + assert_commit_tree_has_n_entries(old_commit, 1); + assert_commit_is_head(old_commit); + + /* let's amend the message of the HEAD commit */ + + cl_git_pass(git_commit_amend( + &new_oid, old_commit, "HEAD", NULL, NULL, NULL, "Initial commit", NULL)); + + cl_git_pass(git_commit_lookup(&new_commit, repo, &new_oid)); + + cl_assert_equal_i(0, git_commit_parentcount(new_commit)); + assert_commit_tree_has_n_entries(new_commit, 1); + assert_commit_is_head(new_commit); + + git_commit_free(old_commit); + old_commit = new_commit; + + /* let's amend the tree of that last commit */ + + cl_git_mkfile("treebuilder/anotherfile", "This is another file\n"); + cl_git_pass(git_index_add_bypath(index, "anotherfile")); + cl_git_pass(git_index_write_tree(&tree_oid, index)); + cl_git_pass(git_tree_lookup(&tree, repo, &tree_oid)); + cl_assert_equal_i(2, git_tree_entrycount(tree)); + + cl_git_pass(git_commit_amend( + &new_oid, old_commit, "HEAD", NULL, NULL, NULL, "Initial commit", tree)); + git_tree_free(tree); + + cl_git_pass(git_commit_lookup(&new_commit, repo, &new_oid)); + + cl_assert_equal_i(0, git_commit_parentcount(new_commit)); + assert_commit_tree_has_n_entries(new_commit, 2); + assert_commit_is_head(new_commit); + + /* cleanup */ + + git_commit_free(old_commit); + git_commit_free(new_commit); + git_index_free(index); +}