From 40c75652d075f87f20ddfbb715667f82644bc760 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Sat, 21 Jul 2012 12:33:46 +0200 Subject: [PATCH] reflog: prevent git_reflog_append() from persisting the reflog back to disk --- include/git2/reflog.h | 13 +-- src/reflog.c | 138 ++++++++++++++------------------ tests-clar/refs/reflog/reflog.c | 84 ++++++++----------- 3 files changed, 97 insertions(+), 138 deletions(-) diff --git a/include/git2/reflog.h b/include/git2/reflog.h index ae8bb8657..a314f94c2 100644 --- a/include/git2/reflog.h +++ b/include/git2/reflog.h @@ -46,22 +46,17 @@ GIT_EXTERN(int) git_reflog_read(git_reflog **reflog, git_reference *ref); GIT_EXTERN(int) git_reflog_write(git_reflog *reflog); /** - * Add a new entry to the reflog for the given reference - * - * If there is no reflog file for the given - * reference yet, it will be created. - * - * `oid_old` may be NULL in case it's a new reference. + * Add a new entry to the reflog. * * `msg` is optional and can be NULL. * - * @param ref the changed reference - * @param oid_old the OID the reference was pointing to + * @param reflog an existing reflog object + * @param new_oid the OID the reference is now pointing to * @param committer the signature of the committer * @param msg the reflog message * @return 0 or an error code */ -GIT_EXTERN(int) git_reflog_append(git_reference *ref, const git_oid *oid_old, const git_signature *committer, const char *msg); +GIT_EXTERN(int) git_reflog_append(git_reflog *reflog, const git_oid *new_oid, const git_signature *committer, const char *msg); /** * Rename the reflog for the given reference diff --git a/src/reflog.c b/src/reflog.c index 9007bd379..ef0aa7eca 100644 --- a/src/reflog.c +++ b/src/reflog.c @@ -59,18 +59,8 @@ static int serialize_reflog_entry( git_buf_rtrim(buf); if (msg) { - const char *newline = strchr(msg, '\n'); - - if (newline && newline[1] != '\0') { - giterr_set(GITERR_INVALID, "Reflog message cannot contain newline"); - return -1; - } - git_buf_putc(buf, '\t'); git_buf_puts(buf, msg); - - /* drop potential trailing LF */ - git_buf_rtrim(buf); } git_buf_putc(buf, '\n'); @@ -78,34 +68,28 @@ static int serialize_reflog_entry( return git_buf_oom(buf); } -static int reflog_write(const char *log_path, const git_oid *oid_old, - const git_oid *oid_new, const git_signature *committer, - const char *msg) +static int reflog_entry_new(git_reflog_entry **entry) { - int error = -1; - git_buf log = GIT_BUF_INIT; - git_filebuf fbuf = GIT_FILEBUF_INIT; + git_reflog_entry *e; - assert(log_path && oid_old && oid_new && committer); + assert(entry); - if (serialize_reflog_entry(&log, oid_old, oid_new, committer, msg) < 0) - goto cleanup; + e = git__malloc(sizeof(git_reflog_entry)); + GITERR_CHECK_ALLOC(e); - if ((error = git_filebuf_open(&fbuf, log_path, GIT_FILEBUF_APPEND)) < 0) - goto cleanup; + memset(e, 0, sizeof(git_reflog_entry)); - if ((error = git_filebuf_write(&fbuf, log.ptr, log.size)) < 0) - goto cleanup; + *entry = e; - error = git_filebuf_commit(&fbuf, GIT_REFLOG_FILE_MODE); - goto success; + return 0; +} -cleanup: - git_filebuf_cleanup(&fbuf); +static void reflog_entry_free(git_reflog_entry *entry) +{ + git_signature_free(entry->committer); -success: - git_buf_free(&log); - return error; + git__free(entry->msg); + git__free(entry); } static int reflog_parse(git_reflog *log, const char *buf, size_t buf_size) @@ -123,8 +107,8 @@ static int reflog_parse(git_reflog *log, const char *buf, size_t buf_size) } while (0) while (buf_size > GIT_REFLOG_SIZE_MIN) { - entry = git__malloc(sizeof(git_reflog_entry)); - GITERR_CHECK_ALLOC(entry); + if (reflog_entry_new(&entry) < 0) + return -1; entry->committer = git__malloc(sizeof(git_signature)); GITERR_CHECK_ALLOC(entry->committer); @@ -171,21 +155,12 @@ static int reflog_parse(git_reflog *log, const char *buf, size_t buf_size) #undef seek_forward fail: - if (entry) { - git__free(entry->committer); - git__free(entry); - } + if (entry) + reflog_entry_free(entry); + return -1; } -static void reflog_entry_free(git_reflog_entry *entry) -{ - git_signature_free(entry->committer); - - git__free(entry->msg); - git__free(entry); -} - void git_reflog_free(git_reflog *reflog) { unsigned int i; @@ -285,55 +260,58 @@ success: return error; } -int git_reflog_append(git_reference *ref, const git_oid *oid_old, +int git_reflog_append(git_reflog *reflog, const git_oid *new_oid, const git_signature *committer, const char *msg) { - int error; + int count; + git_reflog_entry *entry; + const char *newline; - git_buf log_path = GIT_BUF_INIT; - git_reference *r; - git_oid zero_oid; - const git_oid *oid; + assert(reflog && new_oid && committer); - if ((error = git_reference_resolve(&r, ref)) < 0) - return error; - - oid = git_reference_oid(r); - if (oid == NULL) { - giterr_set(GITERR_REFERENCE, - "Failed to write reflog. Cannot resolve reference `%s`", r->name); - git_reference_free(r); + if (reflog_entry_new(&entry) < 0) return -1; - } - error = retrieve_reflog_path(&log_path, ref); - if (error < 0) + if ((entry->committer = git_signature_dup(committer)) == NULL) goto cleanup; - if (git_path_exists(log_path.ptr) == false) { - error = git_futils_mkpath2file(log_path.ptr, GIT_REFLOG_DIR_MODE); - } else if (git_path_isfile(log_path.ptr) == false) { - giterr_set(GITERR_REFERENCE, - "Failed to write reflog. `%s` is directory", log_path.ptr); - error = -1; - } else if (oid_old == NULL) { - giterr_set(GITERR_REFERENCE, - "Failed to write reflog. Old OID cannot be NULL for existing reference"); - error = -1; + if (msg != NULL) { + if ((entry->msg = git__strdup(msg)) == NULL) + goto cleanup; + + newline = strchr(msg, '\n'); + + if (newline) { + if (newline[1] != '\0') { + giterr_set(GITERR_INVALID, "Reflog message cannot contain newline"); + goto cleanup; + } + + entry->msg[newline - msg] = '\0'; + } } - if (error < 0) + + count = git_reflog_entrycount(reflog); + + if (count == 0) + git_oid_fromstr(&entry->oid_old, GIT_OID_HEX_ZERO); + else { + const git_reflog_entry *previous; + + previous = git_reflog_entry_byindex(reflog, count -1); + git_oid_cpy(&entry->oid_old, &previous->oid_cur); + } + + git_oid_cpy(&entry->oid_cur, new_oid); + + if (git_vector_insert(&reflog->entries, entry) < 0) goto cleanup; - if (!oid_old) { - git_oid_fromstr(&zero_oid, GIT_OID_HEX_ZERO); - error = reflog_write(log_path.ptr, &zero_oid, oid, committer, msg); - } else - error = reflog_write(log_path.ptr, oid_old, oid, committer, msg); + return 0; cleanup: - git_reference_free(r); - git_buf_free(&log_path); - return error; + reflog_entry_free(entry); + return -1; } int git_reflog_rename(git_reference *ref, const char *new_name) diff --git a/tests-clar/refs/reflog/reflog.c b/tests-clar/refs/reflog/reflog.c index ac61f1343..ed3b31563 100644 --- a/tests-clar/refs/reflog/reflog.c +++ b/tests-clar/refs/reflog/reflog.c @@ -34,8 +34,6 @@ void test_refs_reflog_reflog__cleanup(void) cl_git_sandbox_cleanup(); } - - void test_refs_reflog_reflog__append_then_read(void) { // write a reflog for a given reference and ensure it can be read back @@ -44,21 +42,21 @@ void test_refs_reflog_reflog__append_then_read(void) git_oid oid; git_signature *committer; git_reflog *reflog; - git_reflog_entry *entry; - char oid_str[GIT_OID_HEXSZ+1]; + const git_reflog_entry *entry; /* Create a new branch pointing at the HEAD */ git_oid_fromstr(&oid, current_master_tip); cl_git_pass(git_reference_create_oid(&ref, g_repo, new_ref, &oid, 0)); - git_reference_free(ref); - cl_git_pass(git_reference_lookup(&ref, g_repo, new_ref)); cl_git_pass(git_signature_now(&committer, "foo", "foo@bar")); - cl_git_pass(git_reflog_append(ref, NULL, committer, NULL)); - cl_git_fail(git_reflog_append(ref, NULL, committer, "no ancestor NULL for an existing reflog")); - cl_git_fail(git_reflog_append(ref, NULL, committer, "no inner\nnewline")); - cl_git_pass(git_reflog_append(ref, &oid, committer, commit_msg "\n")); + cl_git_pass(git_reflog_read(&reflog, ref)); + + cl_git_fail(git_reflog_append(reflog, &oid, committer, "no inner\nnewline")); + cl_git_pass(git_reflog_append(reflog, &oid, committer, NULL)); + cl_git_pass(git_reflog_append(reflog, &oid, committer, commit_msg "\n")); + cl_git_pass(git_reflog_write(reflog)); + git_reflog_free(reflog); /* Reopen a new instance of the repository */ cl_git_pass(git_repository_open(&repo2, "testrepo.git")); @@ -68,22 +66,18 @@ void test_refs_reflog_reflog__append_then_read(void) /* Read and parse the reflog for this branch */ cl_git_pass(git_reflog_read(&reflog, lookedup_ref)); - cl_assert(reflog->entries.length == 2); + cl_assert_equal_i(2, git_reflog_entrycount(reflog)); - entry = (git_reflog_entry *)git_vector_get(&reflog->entries, 0); + entry = git_reflog_entry_byindex(reflog, 0); assert_signature(committer, entry->committer); - git_oid_tostr(oid_str, GIT_OID_HEXSZ+1, &entry->oid_old); - cl_assert_equal_s(GIT_OID_HEX_ZERO, oid_str); - git_oid_tostr(oid_str, GIT_OID_HEXSZ+1, &entry->oid_cur); - cl_assert_equal_s(current_master_tip, oid_str); + cl_assert(git_oid_streq(&entry->oid_old, GIT_OID_HEX_ZERO) == 0); + cl_assert(git_oid_cmp(&oid, &entry->oid_cur) == 0); cl_assert(entry->msg == NULL); - entry = (git_reflog_entry *)git_vector_get(&reflog->entries, 1); + entry = git_reflog_entry_byindex(reflog, 1); assert_signature(committer, entry->committer); - git_oid_tostr(oid_str, GIT_OID_HEXSZ+1, &entry->oid_old); - cl_assert_equal_s(current_master_tip, oid_str); - git_oid_tostr(oid_str, GIT_OID_HEXSZ+1, &entry->oid_cur); - cl_assert_equal_s(current_master_tip, oid_str); + cl_assert(git_oid_cmp(&oid, &entry->oid_old) == 0); + cl_assert(git_oid_cmp(&oid, &entry->oid_cur) == 0); cl_assert_equal_s(commit_msg, entry->msg); git_signature_free(committer); @@ -94,34 +88,6 @@ void test_refs_reflog_reflog__append_then_read(void) git_reference_free(lookedup_ref); } -void test_refs_reflog_reflog__dont_append_bad(void) -{ - // avoid writing an obviously wrong reflog - git_reference *ref; - git_oid oid; - git_signature *committer; - - /* Create a new branch pointing at the HEAD */ - git_oid_fromstr(&oid, current_master_tip); - cl_git_pass(git_reference_create_oid(&ref, g_repo, new_ref, &oid, 0)); - git_reference_free(ref); - cl_git_pass(git_reference_lookup(&ref, g_repo, new_ref)); - - cl_git_pass(git_signature_now(&committer, "foo", "foo@bar")); - - /* Write the reflog for the new branch */ - cl_git_pass(git_reflog_append(ref, NULL, committer, NULL)); - - /* Try to update the reflog with wrong information: - * It's no new reference, so the ancestor OID cannot - * be NULL. */ - cl_git_fail(git_reflog_append(ref, NULL, committer, NULL)); - - git_signature_free(committer); - - git_reference_free(ref); -} - void test_refs_reflog_reflog__renaming_the_reference_moves_the_reflog(void) { git_reference *master; @@ -163,3 +129,23 @@ void test_refs_reflog_reflog__reference_has_reflog(void) assert_has_reflog(true, "refs/heads/master"); assert_has_reflog(false, "refs/heads/subtrees"); } + +void test_refs_reflog_reflog__reading_the_reflog_from_a_reference_with_no_log_returns_an_empty_one(void) +{ + git_reference *subtrees; + git_reflog *reflog; + git_buf subtrees_log_path = GIT_BUF_INIT; + + cl_git_pass(git_reference_lookup(&subtrees, g_repo, "refs/heads/subtrees")); + + git_buf_join_n(&subtrees_log_path, '/', 3, git_repository_path(g_repo), GIT_REFLOG_DIR, git_reference_name(subtrees)); + cl_assert_equal_i(false, git_path_isfile(git_buf_cstr(&subtrees_log_path))); + + cl_git_pass(git_reflog_read(&reflog, subtrees)); + + cl_assert_equal_i(0, git_reflog_entrycount(reflog)); + + git_reflog_free(reflog); + git_reference_free(subtrees); + git_buf_free(&subtrees_log_path); +}