diff --git a/include/git2/sys/refdb_backend.h b/include/git2/sys/refdb_backend.h index d5611d01e..131e1b5d0 100644 --- a/include/git2/sys/refdb_backend.h +++ b/include/git2/sys/refdb_backend.h @@ -94,12 +94,12 @@ struct git_refdb_backend { */ int (*write)(git_refdb_backend *backend, const git_reference *ref, int force, - const char *message); + const git_signature *who, const char *message); int (*rename)( git_reference **out, git_refdb_backend *backend, const char *old_name, const char *new_name, int force, - const char *message); + const git_signature *who, const char *message); /** * Deletes the given reference from the refdb. A refdb implementation diff --git a/src/refdb.c b/src/refdb.c index 8f002ebcc..bc8c2b366 100644 --- a/src/refdb.c +++ b/src/refdb.c @@ -167,14 +167,14 @@ void git_refdb_iterator_free(git_reference_iterator *iter) iter->free(iter); } -int git_refdb_write(git_refdb *db, git_reference *ref, int force, const char *message) +int git_refdb_write(git_refdb *db, git_reference *ref, int force, const git_signature *who, const char *message) { assert(db && db->backend); GIT_REFCOUNT_INC(db); ref->db = db; - return db->backend->write(db->backend, ref, force, message); + return db->backend->write(db->backend, ref, force, who, message); } int git_refdb_rename( @@ -183,12 +183,13 @@ int git_refdb_rename( const char *old_name, const char *new_name, int force, + const git_signature *who, const char *message) { int error; assert(db && db->backend); - error = db->backend->rename(out, db->backend, old_name, new_name, force, message); + error = db->backend->rename(out, db->backend, old_name, new_name, force, who, message); if (error < 0) return error; diff --git a/src/refdb.h b/src/refdb.h index 4dea20bd9..215ae17c5 100644 --- a/src/refdb.h +++ b/src/refdb.h @@ -34,6 +34,7 @@ int git_refdb_rename( const char *old_name, const char *new_name, int force, + const git_signature *who, const char *message); int git_refdb_iterator(git_reference_iterator **out, git_refdb *db, const char *glob); @@ -41,7 +42,7 @@ int git_refdb_iterator_next(git_reference **out, git_reference_iterator *iter); int git_refdb_iterator_next_name(const char **out, git_reference_iterator *iter); void git_refdb_iterator_free(git_reference_iterator *iter); -int git_refdb_write(git_refdb *refdb, git_reference *ref, int force, const char *message); +int git_refdb_write(git_refdb *refdb, git_reference *ref, int force, const git_signature *who, const char *message); int git_refdb_delete(git_refdb *refdb, const char *ref_name); int git_refdb_reflog_read(git_reflog **out, git_refdb *db, const char *name); diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 6eb6ec418..2f7b43401 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -688,9 +688,8 @@ static int reference_path_available( return 0; } -static int loose_write(refdb_fs_backend *backend, const git_reference *ref) +static int loose_lock(git_filebuf *file, refdb_fs_backend *backend, const git_reference *ref) { - git_filebuf file = GIT_FILEBUF_INIT; git_buf ref_path = GIT_BUF_INIT; /* Remove a possibly existing empty directory hierarchy @@ -702,25 +701,29 @@ static int loose_write(refdb_fs_backend *backend, const git_reference *ref) if (git_buf_joinpath(&ref_path, backend->path, ref->name) < 0) return -1; - if (git_filebuf_open(&file, ref_path.ptr, GIT_FILEBUF_FORCE, GIT_REFS_FILE_MODE) < 0) { + if (git_filebuf_open(file, ref_path.ptr, GIT_FILEBUF_FORCE, GIT_REFS_FILE_MODE) < 0) { git_buf_free(&ref_path); return -1; } git_buf_free(&ref_path); + return 0; +} +static int loose_commit(git_filebuf *file, const git_reference *ref) +{ if (ref->type == GIT_REF_OID) { char oid[GIT_OID_HEXSZ + 1]; git_oid_nfmt(oid, sizeof(oid), &ref->target.oid); - git_filebuf_printf(&file, "%s\n", oid); + git_filebuf_printf(file, "%s\n", oid); } else if (ref->type == GIT_REF_SYMBOLIC) { - git_filebuf_printf(&file, GIT_SYMREF "%s\n", ref->target.symbolic); + git_filebuf_printf(file, GIT_SYMREF "%s\n", ref->target.symbolic); } else { assert(0); /* don't let this happen */ } - return git_filebuf_commit(&file); + return git_filebuf_commit(file); } /* @@ -907,13 +910,17 @@ fail: return -1; } +static int reflog_append(refdb_fs_backend *backend, const git_reference *ref, const git_signature *author, const char *message); + static int refdb_fs_backend__write( git_refdb_backend *_backend, const git_reference *ref, int force, + const git_signature *who, const char *message) { refdb_fs_backend *backend = (refdb_fs_backend *)_backend; + git_filebuf file = GIT_FILEBUF_INIT; int error; assert(backend); @@ -922,7 +929,16 @@ static int refdb_fs_backend__write( if (error < 0) return error; - return loose_write(backend, ref); + /* We need to perform the reflog append under the ref's lock */ + if ((error = loose_lock(&file, backend, ref)) < 0) + return error; + + if ((error = reflog_append(backend, ref, who, message)) < 0) { + git_filebuf_cleanup(&file); + return error; + } + + return loose_commit(&file, ref); } static int refdb_fs_backend__delete( @@ -970,16 +986,20 @@ static int refdb_fs_backend__delete( return packed_write(backend); } +static int refdb_reflog_fs__rename(git_refdb_backend *_backend, const char *old_name, const char *new_name); + static int refdb_fs_backend__rename( git_reference **out, git_refdb_backend *_backend, const char *old_name, const char *new_name, int force, + const git_signature *who, const char *message) { refdb_fs_backend *backend = (refdb_fs_backend *)_backend; git_reference *old, *new; + git_filebuf file = GIT_FILEBUF_INIT; int error; assert(backend); @@ -1000,7 +1020,28 @@ static int refdb_fs_backend__rename( return -1; } - if ((error = loose_write(backend, new)) < 0 || out == NULL) { + if ((error = loose_lock(&file, backend, new)) < 0) { + git_reference_free(new); + return error; + } + + /* Try to rename the refog; it's ok if the old doesn't exist */ + error = refdb_reflog_fs__rename(_backend, old_name, new_name); + if (((error == 0) || (error == GIT_ENOTFOUND)) && + ((error = reflog_append(backend, new, who, message)) < 0)) { + git_reference_free(new); + git_filebuf_cleanup(&file); + return error; + } + + if (error < 0) { + git_reference_free(new); + git_filebuf_cleanup(&file); + return error; + } + + + if ((error = loose_commit(&file, new)) < 0 || out == NULL) { git_reference_free(new); return error; } @@ -1330,6 +1371,48 @@ success: return error; } +/* Append to the reflog, must be called under reference lock */ +static int reflog_append(refdb_fs_backend *backend, const git_reference *ref, const git_signature *who, const char *message) +{ + int error; + git_oid old_id, new_id; + git_buf buf = GIT_BUF_INIT, path = GIT_BUF_INIT; + git_repository *repo = backend->repo; + + /* Creation of symbolic references doesn't get a reflog entry */ + if (ref->type == GIT_REF_SYMBOLIC) + return 0; + + error = git_reference_name_to_id(&old_id, repo, ref->name); + if (error == GIT_ENOTFOUND) { + memset(&old_id, 0, sizeof(git_oid)); + error = 0; + } + if (error < 0) + return error; + + git_oid_cpy(&new_id, git_reference_target(ref)); + + if ((error = serialize_reflog_entry(&buf, &old_id, &new_id, who, message)) < 0) + goto cleanup; + + if ((error = retrieve_reflog_path(&path, repo, ref->name)) < 0) + goto cleanup; + + if (((error = git_futils_mkpath2file(git_buf_cstr(&path), 0777)) < 0) && + (error != GIT_EEXISTS)) { + goto cleanup; + } + + error = git_futils_writebuffer(&buf, git_buf_cstr(&path), O_WRONLY|O_CREAT|O_APPEND, GIT_REFLOG_FILE_MODE); + +cleanup: + git_buf_free(&buf); + git_buf_free(&path); + + return error; +} + static int refdb_reflog_fs__rename(git_refdb_backend *_backend, const char *old_name, const char *new_name) { int error = 0, fd; @@ -1358,6 +1441,11 @@ static int refdb_reflog_fs__rename(git_refdb_backend *_backend, const char *old_ if (git_buf_joinpath(&new_path, git_buf_cstr(&temp_path), git_buf_cstr(&normalized)) < 0) return -1; + if (!git_path_exists(git_buf_cstr(&old_path))) { + error = GIT_ENOTFOUND; + goto cleanup; + } + /* * Move the reflog to a temporary place. This two-phase renaming is required * in order to cope with funny renaming use cases when one tries to move a reference diff --git a/src/refs.c b/src/refs.c index dbbdda045..598d6873c 100644 --- a/src/refs.c +++ b/src/refs.c @@ -21,6 +21,7 @@ #include #include #include +#include GIT__USE_STRMAP; @@ -322,23 +323,6 @@ const char *git_reference_symbolic_target(const git_reference *ref) return ref->target.symbolic; } -static int feed_reflog( - const git_reference *ref, - const git_signature *signature, - const char *log_message) -{ - - git_oid peeled_ref_oid; - int error; - - if ((error = git_reference_name_to_id(&peeled_ref_oid, - git_reference_owner(ref), git_reference_name(ref))) < 0) - return error; - - return git_reflog_append_to(git_reference_owner(ref), git_reference_name(ref), - &peeled_ref_oid, signature, log_message); -} - static int reference__create( git_reference **ref_out, git_repository *repo, @@ -355,7 +339,7 @@ static int reference__create( int error = 0; assert(repo && name); - assert(!((signature == NULL) ^ (log_message == NULL))); + assert(symbolic || signature); if (ref_out) *ref_out = NULL; @@ -396,12 +380,7 @@ static int reference__create( GITERR_CHECK_ALLOC(ref); - if ((error = git_refdb_write(refdb, ref, force, log_message)) < 0) { - git_reference_free(ref); - return error; - } - - if (log_message && (error = feed_reflog(ref, signature, log_message)) < 0) { + if ((error = git_refdb_write(refdb, ref, force, signature, log_message)) < 0) { git_reference_free(ref); return error; } @@ -421,9 +400,22 @@ int git_reference_create( const git_oid *oid, int force) { + git_signature *who; + int error; + assert(oid); - return reference__create(ref_out, repo, name, oid, NULL, force, NULL, NULL); + /* Should we return an error if there is no default? */ + if (((error = git_signature_default(&who, repo)) < 0) && + ((error = git_signature_now(&who, "unknown", "unknown")) < 0)) { + return error; + } + + error = reference__create(ref_out, repo, name, oid, NULL, force, who, NULL); + + git_signature_free(who); + + return error; } int git_reference_create_with_log( @@ -449,25 +441,9 @@ int git_reference_symbolic_create( int force) { assert(target); - return reference__create(ref_out, repo, name, NULL, target, force, NULL, NULL); } -int git_reference_symbolic_create_with_log( - git_reference **ref_out, - git_repository *repo, - const char *name, - const char *target, - int force, - const git_signature *signature, - const char* log_message) -{ - assert(target && signature && log_message); - - return reference__create( - ref_out, repo, name, NULL, target, force, signature, log_message); -} - static int ensure_is_an_updatable_direct_reference(git_reference *ref) { if (ref->type == GIT_REF_OID) @@ -535,36 +511,15 @@ int git_reference_symbolic_set_target( return git_reference_symbolic_create(out, ref->db->repo, ref->name, target, 1); } -int git_reference_symbolic_set_target_with_log( - git_reference **out, - git_reference *ref, - const char *target, - const git_signature *signature, - const char *log_message) -{ - int error; - - assert(out && ref && target); - assert(signature && log_message); - - if ((error = ensure_is_an_updatable_symbolic_reference(ref)) < 0) - return error; - - return git_reference_symbolic_create_with_log( - out, ref->db->repo, ref->name, target, 1, signature, log_message); -} - -int git_reference_rename( - git_reference **out, - git_reference *ref, - const char *new_name, - int force) +static int reference__rename(git_reference **out, git_reference *ref, const char *new_name, int force, + const git_signature *signature, const char *message) { unsigned int normalization_flags; char normalized[GIT_REFNAME_MAX]; bool should_head_be_updated = false; int error = 0; - int reference_has_log; + + assert(ref && new_name && signature); normalization_flags = ref->type == GIT_REF_SYMBOLIC ? GIT_REF_FORMAT_ALLOW_ONELEVEL : GIT_REF_FORMAT_NORMAL; @@ -573,13 +528,14 @@ int git_reference_rename( normalized, sizeof(normalized), new_name, normalization_flags)) < 0) return error; + /* Check if we have to update HEAD. */ if ((error = git_branch_is_head(ref)) < 0) return error; should_head_be_updated = (error > 0); - if ((error = git_refdb_rename(out, ref->db, ref->name, new_name, force, NULL)) < 0) + if ((error = git_refdb_rename(out, ref->db, ref->name, new_name, force, signature, message)) < 0) return error; /* Update HEAD it was poiting to the reference being renamed. */ @@ -589,17 +545,43 @@ int git_reference_rename( return error; } - /* Rename the reflog file, if it exists. */ - reference_has_log = git_reference_has_log(ref); - if (reference_has_log < 0) - return reference_has_log; - - if (reference_has_log && (error = git_reflog_rename(git_reference_owner(ref), git_reference_name(ref), new_name)) < 0) - return error; - return 0; } + +int git_reference_rename( + git_reference **out, + git_reference *ref, + const char *new_name, + int force) +{ + git_signature *who; + int error; + + /* Should we return an error if there is no default? */ + if (((error = git_signature_default(&who, ref->db->repo)) < 0) && + ((error = git_signature_now(&who, "unknown", "unknown")) < 0)) { + return error; + } + + error = reference__rename(out, ref, new_name, force, who, NULL); + + git_signature_free(who); + + return error; +} + +int git_reference_rename_with_log( + git_reference **out, + git_reference *ref, + const char *new_name, + int force, + const git_signature *who, + const char * message) +{ + return reference__rename(out, ref, new_name, force, who, message); +} + int git_reference_resolve(git_reference **ref_out, const git_reference *ref) { switch (git_reference_type(ref)) { diff --git a/src/stash.c b/src/stash.c index 083c2a4cd..66b1cd7c5 100644 --- a/src/stash.c +++ b/src/stash.c @@ -412,25 +412,12 @@ static int update_reflog( const char *message) { git_reference *stash; - git_reflog *reflog = NULL; int error; - if ((error = git_reference_create(&stash, repo, GIT_REFS_STASH_FILE, w_commit_oid, 1)) < 0) - goto cleanup; + error = git_reference_create_with_log(&stash, repo, GIT_REFS_STASH_FILE, w_commit_oid, 1, stasher, message); git_reference_free(stash); - if ((error = git_reflog_read(&reflog, repo, GIT_REFS_STASH_FILE) < 0)) - goto cleanup; - - if ((error = git_reflog_append(reflog, w_commit_oid, stasher, message)) < 0) - goto cleanup; - - if ((error = git_reflog_write(reflog)) < 0) - goto cleanup; - -cleanup: - git_reflog_free(reflog); return error; } @@ -636,7 +623,11 @@ int git_stash_drop( entry = git_reflog_entry_byindex(reflog, 0); git_reference_free(stash); - error = git_reference_create(&stash, repo, GIT_REFS_STASH_FILE, &entry->oid_cur, 1); + if ((error = git_reference_create(&stash, repo, GIT_REFS_STASH_FILE, &entry->oid_cur, 1) < 0)) + goto cleanup; + + /* We need to undo the writing that we just did */ + error = git_reflog_write(reflog); } cleanup: diff --git a/tests/refs/createwithlog.c b/tests/refs/createwithlog.c index ff36ffdcd..0fe81df91 100644 --- a/tests/refs/createwithlog.c +++ b/tests/refs/createwithlog.c @@ -6,7 +6,6 @@ #include "ref_helpers.h" static const char *current_master_tip = "a65fedf39aefe402d3bb6e24df4d4f5fe4547750"; -static const char *current_head_target = "refs/heads/master"; static git_repository *g_repo; @@ -50,34 +49,3 @@ void test_refs_createwithlog__creating_a_direct_reference_adds_a_reflog_entry(vo git_reference_free(reference); git_signature_free(signature); } - -void test_refs_createwithlog__creating_a_symbolic_reference_adds_a_reflog_entry(void) -{ - git_reference *reference; - git_oid id; - git_signature *signature; - git_reflog *reflog; - const git_reflog_entry *entry; - - const char *name = "ANOTHER_HEAD_TRACKER"; - const char *message = "You've been logged, mate!"; - - git_oid_fromstr(&id, current_master_tip); - - cl_git_pass(git_signature_now(&signature, "foo", "foo@bar")); - - cl_git_pass(git_reference_symbolic_create_with_log(&reference, g_repo, - name, current_head_target, 0, signature, message)); - - cl_git_pass(git_reflog_read(&reflog, g_repo, name)); - cl_assert_equal_sz(1, git_reflog_entrycount(reflog)); - - entry = git_reflog_entry_byindex(reflog, 0); - cl_assert(git_oid_streq(&entry->oid_old, GIT_OID_HEX_ZERO) == 0); - cl_assert(git_oid_cmp(&id, &entry->oid_cur) == 0); - cl_assert_equal_s(message, entry->msg); - - git_reflog_free(reflog); - git_reference_free(reference); - git_signature_free(signature); -} diff --git a/tests/refs/reflog/reflog.c b/tests/refs/reflog/reflog.c index bcd224270..60085791c 100644 --- a/tests/refs/reflog/reflog.c +++ b/tests/refs/reflog/reflog.c @@ -49,17 +49,20 @@ static void assert_appends(const git_signature *committer, const git_oid *oid) /* Read and parse the reflog for this branch */ cl_git_pass(git_reflog_read(&reflog, repo2, new_ref)); - cl_assert_equal_i(2, (int)git_reflog_entrycount(reflog)); + cl_assert_equal_i(3, (int)git_reflog_entrycount(reflog)); + + /* The first one was the creation of the branch */ + entry = git_reflog_entry_byindex(reflog, 2); + cl_assert(git_oid_streq(&entry->oid_old, GIT_OID_HEX_ZERO) == 0); entry = git_reflog_entry_byindex(reflog, 1); assert_signature(committer, entry->committer); - cl_assert(git_oid_streq(&entry->oid_old, GIT_OID_HEX_ZERO) == 0); + cl_assert(git_oid_cmp(oid, &entry->oid_old) == 0); cl_assert(git_oid_cmp(oid, &entry->oid_cur) == 0); cl_assert(entry->msg == NULL); entry = git_reflog_entry_byindex(reflog, 0); assert_signature(committer, entry->committer); - 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); diff --git a/tests/refs/settargetwithlog.c b/tests/refs/settargetwithlog.c index 99377dad7..cfa1c99d5 100644 --- a/tests/refs/settargetwithlog.c +++ b/tests/refs/settargetwithlog.c @@ -53,42 +53,3 @@ void test_refs_settargetwithlog__updating_a_direct_reference_adds_a_reflog_entry git_reference_free(reference); git_signature_free(signature); } - -void test_refs_settargetwithlog__updating_a_symbolic_reference_adds_a_reflog_entry(void) -{ - git_reference *reference, *reference_out; - git_oid id; - git_signature *signature; - git_reflog *reflog; - const git_reflog_entry *entry; - - const char *name = "HEAD"; - const char *message = "You've been logged, mate!"; - - git_oid_fromstr(&id, br2_tip); - - cl_git_pass(git_signature_now(&signature, "foo", "foo@bar")); - - cl_git_pass(git_reference_lookup(&reference, g_repo, name)); - - cl_assert_equal_s( - "refs/heads/master", git_reference_symbolic_target(reference)); - - cl_git_pass(git_reference_symbolic_set_target_with_log(&reference_out, - reference, br2_name, signature, message)); - - cl_assert_equal_s( - br2_name, git_reference_symbolic_target(reference_out)); - - cl_git_pass(git_reflog_read(&reflog, g_repo, name)); - - entry = git_reflog_entry_byindex(reflog, 0); - cl_assert(git_oid_streq(&entry->oid_old, master_tip) == 0); - cl_assert(git_oid_streq(&entry->oid_cur, br2_tip) == 0); - cl_assert_equal_s(message, entry->msg); - - git_reflog_free(reflog); - git_reference_free(reference_out); - git_reference_free(reference); - git_signature_free(signature); -}