diff --git a/include/git2/branch.h b/include/git2/branch.h index 46aef3206..d2762019b 100644 --- a/include/git2/branch.h +++ b/include/git2/branch.h @@ -238,7 +238,7 @@ GIT_EXTERN(int) git_branch_upstream_name( * error code otherwise. */ GIT_EXTERN(int) git_branch_is_head( - git_reference *branch); + const git_reference *branch); /** * Return the name of remote that the remote tracking branch belongs to. diff --git a/include/git2/refs.h b/include/git2/refs.h index 65449e69e..1bbb4ca46 100644 --- a/include/git2/refs.h +++ b/include/git2/refs.h @@ -586,7 +586,7 @@ GIT_EXTERN(int) git_reference_ensure_log(git_repository *repo, const char *refna * @return 1 when the reference lives in the refs/heads * namespace; 0 otherwise. */ -GIT_EXTERN(int) git_reference_is_branch(git_reference *ref); +GIT_EXTERN(int) git_reference_is_branch(const git_reference *ref); /** * Check if a reference is a remote tracking branch diff --git a/src/branch.c b/src/branch.c index 7c888729d..df665a469 100644 --- a/src/branch.c +++ b/src/branch.c @@ -588,7 +588,7 @@ on_error: } int git_branch_is_head( - git_reference *branch) + const git_reference *branch) { git_reference *head; bool is_same = false; diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 8a26bec0b..f494afa71 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -920,7 +921,7 @@ fail: return -1; } -static int reflog_append(refdb_fs_backend *backend, const git_reference *ref, const git_signature *author, const char *message); +static int reflog_append(refdb_fs_backend *backend, const git_reference *ref, const git_oid *old, const git_oid *new, const git_signature *author, const char *message); static int has_reflog(git_repository *repo, const char *name); /* We only write if it's under heads/, remotes/ or notes/ or if it already has a log */ @@ -974,6 +975,40 @@ out: return error; } +/* + * The git.git comment regarding this, for your viewing pleasure: + * + * Special hack: If a branch is updated directly and HEAD + * points to it (may happen on the remote side of a push + * for example) then logically the HEAD reflog should be + * updated too. + * A generic solution implies reverse symref information, + * but finding all symrefs pointing to the given branch + * would be rather costly for this rare event (the direct + * update of a branch) to be worth it. So let's cheat and + * check with HEAD only which should cover 99% of all usage + * scenarios (even 100% of the default ones). + */ +static int maybe_append_head(refdb_fs_backend *backend, const git_reference *ref, const git_signature *who, const char *message) +{ + int error; + git_oid old_id; + git_reference *head; + + error = git_reference_name_to_id(&old_id, backend->repo, ref->name); + if (!git_branch_is_head(ref)) + return 0; + + if ((error = git_reference_lookup(&head, backend->repo, "HEAD")) < 0) + return error; + + error = reflog_append(backend, head, &old_id, git_reference_target(ref), who, message); + + git_reference_free(head); + return error; +} + + static int refdb_fs_backend__write( git_refdb_backend *_backend, const git_reference *ref, @@ -1006,10 +1041,11 @@ static int refdb_fs_backend__write( goto on_error; } - if (should_write_reflog(backend->repo, ref->name) && - (error = reflog_append(backend, ref, who, message)) < 0) { - git_filebuf_cleanup(&file); - return error; + if (should_write_reflog(backend->repo, ref->name)) { + if ((error = reflog_append(backend, ref, NULL, NULL, who, message)) < 0) + goto on_error; + if ((error = maybe_append_head(backend, ref, who, message)) < 0) + goto on_error; } return loose_commit(&file, ref); @@ -1128,7 +1164,7 @@ static int refdb_fs_backend__rename( /* 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)) { + ((error = reflog_append(backend, new, NULL, NULL, who, message)) < 0)) { git_reference_free(new); git_filebuf_cleanup(&file); return error; @@ -1517,34 +1553,61 @@ success: } /* 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) +static int reflog_append(refdb_fs_backend *backend, const git_reference *ref, const git_oid *old, const git_oid *new, const git_signature *who, const char *message) { - int error; - git_oid old_id, new_id = {{0}}; + int error, is_symbolic, was_symbolic = 0; + git_oid old_id = {{0}}, new_id = {{0}}; git_buf buf = GIT_BUF_INIT, path = GIT_BUF_INIT; git_repository *repo = backend->repo; + git_reference *current_ref = NULL; - /* Creation of a symbolic reference doesn't get a reflog entry, except for - * HEAD. git_repository_set_head and friends go through here. */ - if (ref->type == GIT_REF_SYMBOLIC && - 0 != strcmp(ref->name, GIT_HEAD_FILE)) + is_symbolic = ref->type == GIT_REF_SYMBOLIC; + + /* "normal" symbolic updates do not write */ + if (is_symbolic && + strcmp(ref->name, GIT_HEAD_FILE) && + !(old && new)) 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) + error = git_reference_lookup(¤t_ref, repo, ref->name); + if (error < 0 && error != GIT_ENOTFOUND) return error; - if (git_reference_symbolic_target(ref) != NULL) { + if (current_ref) + was_symbolic = current_ref->type == GIT_REF_SYMBOLIC; + + /* From here on is_symoblic also means that it's HEAD */ + + if (old) { + git_oid_cpy(&old_id, old); + } else if (!was_symbolic) { + 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; + } + + if (is_symbolic) { error = git_reference_name_to_id(&new_id, repo, git_reference_symbolic_target(ref)); if (error != 0 && error != GIT_ENOTFOUND) goto cleanup; + /* detaching HEAD does not create an entry */ + if (error == GIT_ENOTFOUND) { + error = 0; + goto cleanup; + } + giterr_clear(); } - else if (git_reference_target(ref) != NULL) + + + if (new) + git_oid_cpy(&new_id, new); + else if (!is_symbolic) git_oid_cpy(&new_id, git_reference_target(ref)); if ((error = serialize_reflog_entry(&buf, &old_id, &new_id, who, message)) < 0) diff --git a/src/refs.c b/src/refs.c index e63796c94..8b6e09a33 100644 --- a/src/refs.c +++ b/src/refs.c @@ -1137,7 +1137,7 @@ int git_reference__is_branch(const char *ref_name) return git__prefixcmp(ref_name, GIT_REFS_HEADS_DIR) == 0; } -int git_reference_is_branch(git_reference *ref) +int git_reference_is_branch(const git_reference *ref) { assert(ref); return git_reference__is_branch(ref->name); diff --git a/tests/refs/reflog/reflog.c b/tests/refs/reflog/reflog.c index 3f7d7d777..149e98273 100644 --- a/tests/refs/reflog/reflog.c +++ b/tests/refs/reflog/reflog.c @@ -214,3 +214,26 @@ void test_refs_reflog_reflog__write_when_explicitly_active(void) git_reference_free(ref); assert_has_reflog(true, "refs/tags/foo"); } + +void test_refs_reflog_reflog__append_to_HEAD_when_changing_current_branch(void) +{ + size_t nlogs, nlogs_after; + git_reference *ref; + git_reflog *log; + git_oid id; + + cl_git_pass(git_reflog_read(&log, g_repo, "HEAD")); + nlogs = git_reflog_entrycount(log); + git_reflog_free(log); + + /* Move it back */ + git_oid_fromstr(&id, "be3563ae3f795b2b4353bcce3a527ad0a4f7f644"); + cl_git_pass(git_reference_create(&ref, g_repo, "refs/heads/master", &id, 1, NULL, NULL)); + git_reference_free(ref); + + cl_git_pass(git_reflog_read(&log, g_repo, "HEAD")); + nlogs_after = git_reflog_entrycount(log); + git_reflog_free(log); + + cl_assert_equal_i(nlogs_after, nlogs + 1); +} diff --git a/tests/repo/head.c b/tests/repo/head.c index c5965fac6..d88fd90d1 100644 --- a/tests/repo/head.c +++ b/tests/repo/head.c @@ -262,32 +262,10 @@ void test_repo_head__setting_head_updates_reflog(void) cl_git_pass(git_repository_set_head_detached(repo, git_object_id(tag), sig, "message3")); cl_git_pass(git_repository_set_head(repo, "refs/heads/haacked", sig, "message4")); - test_reflog(repo, 3, NULL, "refs/heads/haacked", "foo@example.com", "message1"); - test_reflog(repo, 2, "refs/heads/haacked", NULL, "foo@example.com", "message2"); + test_reflog(repo, 2, NULL, "refs/heads/haacked", "foo@example.com", "message1"); test_reflog(repo, 1, NULL, "tags/test^{commit}", "foo@example.com", "message3"); test_reflog(repo, 0, "tags/test^{commit}", "refs/heads/haacked", "foo@example.com", "message4"); git_object_free(tag); git_signature_free(sig); } - -void test_repo_head__setting_creates_head_ref(void) -{ - git_reference *head; - git_reflog *log; - const git_reflog_entry *entry; - - cl_git_pass(git_reference_lookup(&head, repo, "HEAD")); - cl_git_pass(git_reference_delete(head)); - cl_git_pass(git_reflog_delete(repo, "HEAD")); - - cl_git_pass(git_repository_set_head(repo, "refs/heads/haacked", NULL, "create HEAD")); - - cl_git_pass(git_reflog_read(&log, repo, "HEAD")); - cl_assert_equal_i(1, git_reflog_entrycount(log)); - entry = git_reflog_entry_byindex(log, 0); - cl_assert_equal_s("create HEAD", git_reflog_entry_message(entry)); - - git_reflog_free(log); - git_reference_free(head); -} diff --git a/tests/reset/hard.c b/tests/reset/hard.c index 36120ee63..c6bf7a8ac 100644 --- a/tests/reset/hard.c +++ b/tests/reset/hard.c @@ -215,16 +215,18 @@ void test_reset_hard__reflog_is_correct(void) git_object_free(target); /* Moved branch, expect default message */ + exp_msg = "reset: moving"; cl_git_pass(git_revparse_single(&target, repo, "HEAD~^{commit}")); cl_git_pass(git_reset(repo, target, GIT_RESET_HARD, NULL, NULL)); - reflog_check(repo, "HEAD", 3, "emeric.fermas@gmail.com", exp_msg); - reflog_check(repo, "refs/heads/master", 4, NULL, "reset: moving"); + reflog_check(repo, "HEAD", 4, NULL, exp_msg); + reflog_check(repo, "refs/heads/master", 4, NULL, exp_msg); git_object_free(target); /* Moved branch, expect custom message */ + exp_msg = "message1"; cl_git_pass(git_revparse_single(&target, repo, "HEAD~^{commit}")); cl_git_pass(git_reset(repo, target, GIT_RESET_HARD, NULL, "message1")); - reflog_check(repo, "HEAD", 3, "emeric.fermas@gmail.com", exp_msg); - reflog_check(repo, "refs/heads/master", 5, NULL, "message1"); + reflog_check(repo, "HEAD", 5, NULL, exp_msg); + reflog_check(repo, "refs/heads/master", 5, NULL, exp_msg); } diff --git a/tests/reset/mixed.c b/tests/reset/mixed.c index 5d8ff63b4..55b8a2f88 100644 --- a/tests/reset/mixed.c +++ b/tests/reset/mixed.c @@ -65,17 +65,19 @@ void test_reset_mixed__reflog_is_correct(void) target = NULL; /* Moved branch, expect default message */ + exp_msg = "reset: moving"; cl_git_pass(git_revparse_single(&target, repo, "HEAD~^{commit}")); cl_git_pass(git_reset(repo, target, GIT_RESET_MIXED, NULL, NULL)); - reflog_check(repo, "HEAD", 9, "yoram.harmelin@gmail.com", exp_msg); - reflog_check(repo, "refs/heads/master", 10, NULL, "reset: moving"); + reflog_check(repo, "HEAD", 10, NULL, exp_msg); + reflog_check(repo, "refs/heads/master", 10, NULL, exp_msg); git_object_free(target); target = NULL; /* Moved branch, expect custom message */ + exp_msg = "message1"; cl_git_pass(git_revparse_single(&target, repo, "HEAD~^{commit}")); cl_git_pass(git_reset(repo, target, GIT_RESET_MIXED, NULL, "message1")); - reflog_check(repo, "HEAD", 9, "yoram.harmelin@gmail.com", exp_msg); - reflog_check(repo, "refs/heads/master", 11, NULL, "message1"); + reflog_check(repo, "HEAD", 11, NULL, exp_msg); + reflog_check(repo, "refs/heads/master", 11, NULL, exp_msg); }