{"thread":{"id":"44896","subject":"HEAD's reflog entry for a renamed branch","startedAt":"2017-01-16T23:18:13Z","lastAt":"2017-02-25T03:06:26Z","messageCount":36,"participants":["Kyle Meyer","Jeff King","Junio C Hamano","Duy Nguyen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"309505","messageId":"87pojmwq5y.fsf@kyleam.com","threadId":"44896","inReplyTo":null,"subject":"HEAD's reflog entry for a renamed branch","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-01-16T23:17:29Z","receivedAt":"2017-01-16T23:18:13Z","isPatch":false,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Hello,\n\nI noticed that, after renaming the current branch, the corresponding\nmessage in .git/logs/HEAD is empty.\n\nFor example, running\n\n $ mkdir test-repo\n $ cd test-repo\n $ git init\n $ echo abc >file.txt\n $ git add file.txt\n $ git commit -m\"Add file.txt\"\n $ git branch -m master new-master\n\nresulted in the following reflogs:\n\n $ cat .git/logs/refs/heads/new-master\n 00000... 68730... Kyle Meyer 1484607020 -0500\tcommit (initial): Add file.txt\n 68730... 68730... Kyle Meyer 1484607020 -0500\tBranch: renamed refs/heads/master to refs/heads/new-master\n\n $ cat .git/logs/HEAD\n 00000... 68730... Kyle Meyer 1484607020 -0500\tcommit (initial): Add file.txt\n 68730... 00000... Kyle Meyer 1484607020 -0500\n\nI expected the second line of .git/logs/HEAD to mirror the second line\nof .git/logs/refs/heads/new-master. Are the empty message and null sha1\nin HEAD's entry intentional?\n\n--\nKyle\n"},{"id":"310359","messageId":"20170126211205.5gz3zsrptop7n34n@sigill.intra.peff.net","threadId":"44896","inReplyTo":"87pojmwq5y.fsf@kyleam.com","subject":"Re: HEAD's reflog entry for a renamed branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-26T21:12:05Z","receivedAt":"2017-01-26T21:12:12Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 16, 2017 at 06:17:29PM -0500, Kyle Meyer wrote:\n\n> I noticed that, after renaming the current branch, the corresponding\n> message in .git/logs/HEAD is empty.\n> \n> For example, running\n> \n> $ mkdir test-repo\n> $ cd test-repo\n> $ git init\n> $ echo abc >file.txt\n> $ git add file.txt\n> $ git commit -m\"Add file.txt\"\n> $ git branch -m master new-master\n\nThanks for providing a simple reproduction recipe.\n\n> resulted in the following reflogs:\n> \n> $ cat .git/logs/refs/heads/new-master\n> 00000... 68730... Kyle Meyer 1484607020 -0500\tcommit (initial): Add file.txt\n> 68730... 68730... Kyle Meyer 1484607020 -0500\tBranch: renamed refs/heads/master to refs/heads/new-master\n> \n> $ cat .git/logs/HEAD\n> 00000... 68730... Kyle Meyer 1484607020 -0500\tcommit (initial): Add file.txt\n> 68730... 00000... Kyle Meyer 1484607020 -0500\n> \n> I expected the second line of .git/logs/HEAD to mirror the second line\n> of .git/logs/refs/heads/new-master. Are the empty message and null sha1\n> in HEAD's entry intentional?\n\nThe null sha1 is correct, I think. The branch we were on went away, and\nwe use the null sha1 to indicate \"no value\" in both the creation and\ndeletion cases.\n\nIt's unfortunate that there's no message. This is because the rename\ncalls delete_ref() under the hood, but that function doesn't take a\nreflog message argument at all. It usually doesn't matter because\ndeleting the ref will also delete the reflog.\n\nBut as your example shows, deletions _can_ get logged in the HEAD\nreflog; the low-level ref code detects when HEAD points to the updated\nref and logs an entry there, too.\n\nYou can see the same thing with a simpler example:\n\n $ git init\n $ git commit -m foo --allow-empty\n $ git update-ref -d refs/heads/master\n $ cat .git/logs/HEAD\n 00000... 8ffd1... Jeff King 1485464779 -0500\tcommit (initial): foo\n 8ffd1... 00000... Jeff King 1485464787 -0500\n\nThis doesn't come up much because most porcelain commands will refuse to\ndelete the current branch (notice I had to use \"update-ref\" and not\n\"branch -d\").\n\nI'd say there are two potential improvements:\n\n - delete_ref() should take a reflog message argument, in case it\n updates the HEAD reflog (as a bonus, this will future-proof us for a\n day when we might keep reflogs for deleted refs).\n\n - \"git branch -m\" does seem to realize when we are renaming HEAD,\n because it updates HEAD to point to the new branch name. But it\n should probably insert another reflog entry mentioning the rename\n (we do for \"git checkout foo\", even when \"foo\" has the same sha1 as\n the current HEAD).\n\n-Peff\n"},{"id":"310362","messageId":"xmqqk29h33w1.fsf@gitster.mtv.corp.google.com","threadId":"44896","inReplyTo":"20170126211205.5gz3zsrptop7n34n@sigill.intra.peff.net","subject":"Re: HEAD's reflog entry for a renamed branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-01-26T21:30:54Z","receivedAt":"2017-01-26T21:36:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> It's unfortunate that there's no message. This is because the rename\n> calls delete_ref() under the hood, but that function doesn't take a\n> reflog message argument at all. It usually doesn't matter because\n> deleting the ref will also delete the reflog.\n>\n> But as your example shows, deletions _can_ get logged in the HEAD\n> reflog; the low-level ref code detects when HEAD points to the updated\n> ref and logs an entry there, too.\n> ...\n> I'd say there are two potential improvements:\n>\n> - delete_ref() should take a reflog message argument, in case it\n> updates the HEAD reflog (as a bonus, this will future-proof us for a\n> day when we might keep reflogs for deleted refs).\n\nThis sounds sensible.\n\n> - \"git branch -m\" does seem to realize when we are renaming HEAD,\n> because it updates HEAD to point to the new branch name. But it\n> should probably insert another reflog entry mentioning the rename\n> (we do for \"git checkout foo\", even when \"foo\" has the same sha1 as\n> the current HEAD).\n\nThis one I care less (not in the sense that I prefer it not done,\nbut in the sense that I do not mind it is left unfixed than the\nother one you pointed out).\n"},{"id":"310364","messageId":"20170126215309.n2lr6q5doidhzcf5@sigill.intra.peff.net","threadId":"44896","inReplyTo":"xmqqk29h33w1.fsf@gitster.mtv.corp.google.com","subject":"Re: HEAD's reflog entry for a renamed branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-01-26T21:53:09Z","receivedAt":"2017-01-26T21:53:16Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 26, 2017 at 01:30:54PM -0800, Junio C Hamano wrote:\n\n> > - \"git branch -m\" does seem to realize when we are renaming HEAD,\n> > because it updates HEAD to point to the new branch name. But it\n> > should probably insert another reflog entry mentioning the rename\n> > (we do for \"git checkout foo\", even when \"foo\" has the same sha1 as\n> > the current HEAD).\n> \n> This one I care less (not in the sense that I prefer it not done,\n> but in the sense that I do not mind it is left unfixed than the\n> other one you pointed out).\n\nI wondered if it might affect how \"git checkout -\" works. But that\nfeature looks for reflogs like \"checkout: moving from X to Y\" to know to\nmove back to X. So we are fine here. Even though the HEAD reflog does\nnot show us going _to_ new-master, we would see it in a later entry as\n\"from new-master to Y\". What we are missing is \"rename from master to\nnew-master\", but that entry does not matter. There is no \"master\" to\ngo back to anymore. :)\n\n-Peff\n"},{"id":"311835","messageId":"20170217035800.13214-1-kyle@kyleam.com","threadId":"44896","inReplyTo":"20170126211205.5gz3zsrptop7n34n@sigill.intra.peff.net","subject":"[PATCH 0/3] delete_ref(): support reflog messages","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-17T03:57:57Z","receivedAt":"2017-02-17T03:58:41Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"[Sorry for the slow response.]\n\nJeff King writes:\n\n> On Mon, Jan 16, 2017 at 06:17:29PM -0500, Kyle Meyer wrote:\n\n[...]\n\n>> $ cat .git/logs/refs/heads/new-master\n>> 00000... 68730... Kyle Meyer 1484607020 -0500\tcommit (initial): Add file.txt\n>> 68730... 68730... Kyle Meyer 1484607020 -0500\tBranch: renamed refs/heads/master to refs/heads/new-master\n>> \n>> $ cat .git/logs/HEAD\n>> 00000... 68730... Kyle Meyer 1484607020 -0500\tcommit (initial): Add file.txt\n>> 68730... 00000... Kyle Meyer 1484607020 -0500\n>> \n>> I expected the second line of .git/logs/HEAD to mirror the second line\n>> of .git/logs/refs/heads/new-master. Are the empty message and null sha1\n>> in HEAD's entry intentional?\n>\n> The null sha1 is correct, I think. The branch we were on went away, and\n> we use the null sha1 to indicate \"no value\" in both the creation and\n> deletion cases.\n\nI see, thanks.\n\n> I'd say there are two potential improvements:\n>\n> - delete_ref() should take a reflog message argument, in case it\n> updates the HEAD reflog (as a bonus, this will future-proof us for a\n> day when we might keep reflogs for deleted refs).\n\nI've tried to do this in the following patches.\n\n> - \"git branch -m\" does seem to realize when we are renaming HEAD,\n> because it updates HEAD to point to the new branch name. But it\n> should probably insert another reflog entry mentioning the rename\n> (we do for \"git checkout foo\", even when \"foo\" has the same sha1 as\n> the current HEAD).\n\nI haven't worked out how to do this part yet. I'm guessing the change\nwill involve modifying split_head_update().\n\nIf this is added, should it be instead of, rather than in addition to,\nthe deletion entry? If a \"Branch: renamed ...\" entry is present, it\ndoesn't seem like the deletion entry is providing any additional\ninformation.\n\n delete_refs(): accept a reflog message argument\n update-ref: pass reflog message argument to delete_refs\n rename_ref: replace empty deletion message in HEAD's log\n\n builtin/am.c | 4 ++--\n builtin/branch.c | 2 +-\n builtin/notes.c | 4 ++--\n builtin/remote.c | 4 ++--\n builtin/replace.c | 2 +-\n builtin/reset.c | 2 +-\n builtin/symbolic-ref.c | 2 +-\n builtin/tag.c | 2 +-\n builtin/update-ref.c | 2 +-\n fast-import.c | 2 +-\n refs.c | 4 ++--\n refs.h | 2 +-\n refs/files-backend.c | 12 +++++++++---\n t/t1400-update-ref.sh | 9 +++++++++\n t/t3200-branch.sh | 4 ++++\n transport.c | 2 +-\n 16 files changed, 39 insertions(+), 20 deletions(-)\n\n-- \nKyle\n"},{"id":"311836","messageId":"20170217035800.13214-2-kyle@kyleam.com","threadId":"44896","inReplyTo":"20170217035800.13214-1-kyle@kyleam.com","subject":"[PATCH 1/3] delete_refs(): accept a reflog message argument","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-17T03:57:58Z","receivedAt":"2017-02-17T03:58:59Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"When the current branch is renamed with 'git branch -m/-M' or deleted\nwith 'git update-ref -m -d', the event is recorded in HEAD's log\nwith an empty message.\n\nIn preparation for adding a more meaningful message to HEAD's log in\nthese cases, update delete_ref() to take a message argument and pass\nit along to ref_transaction_delete(). Modify all callers to pass NULL\nfor the new message argument; no change in behavior is intended.\n\nSigned-off-by: Kyle Meyer \n---\n builtin/am.c | 4 ++--\n builtin/branch.c | 2 +-\n builtin/notes.c | 4 ++--\n builtin/remote.c | 4 ++--\n builtin/replace.c | 2 +-\n builtin/reset.c | 2 +-\n builtin/symbolic-ref.c | 2 +-\n builtin/tag.c | 2 +-\n builtin/update-ref.c | 2 +-\n fast-import.c | 2 +-\n refs.c | 4 ++--\n refs.h | 2 +-\n refs/files-backend.c | 6 +++---\n transport.c | 2 +-\n 14 files changed, 20 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 31fb60578..e08c739d4 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1049,7 +1049,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,\n \t} else {\n \t\twrite_state_text(state, \"abort-safety\", \"\");\n \t\tif (!state->rebasing)\n-\t\t\tdelete_ref(\"ORIG_HEAD\", NULL, 0);\n+\t\t\tdelete_ref(\"ORIG_HEAD\", NULL, 0, NULL);\n \t}\n \n \t/*\n@@ -2172,7 +2172,7 @@ static void am_abort(struct am_state *state)\n \t\t\t\thas_curr_head ? &curr_head : NULL, 0,\n \t\t\t\tUPDATE_REFS_DIE_ON_ERR);\n \telse if (curr_branch)\n-\t\tdelete_ref(curr_branch, NULL, REF_NODEREF);\n+\t\tdelete_ref(curr_branch, NULL, REF_NODEREF, NULL);\n \n \tfree(curr_branch);\n \tam_destroy(state);\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 9d30f55b0..44f23208f 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -252,7 +252,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\t}\n \n \t\tif (delete_ref(name, is_null_sha1(sha1) ? NULL : sha1,\n-\t\t\t REF_NODEREF)) {\n+\t\t\t REF_NODEREF, NULL)) {\n \t\t\terror(remote_branch\n \t\t\t ? _(\"Error deleting remote-tracking branch '%s'\")\n \t\t\t : _(\"Error deleting branch '%s'\"),\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 5248a9bad..991448d4e 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -681,9 +681,9 @@ static int merge_abort(struct notes_merge_options *o)\n \t * notes_merge_abort() to remove .git/NOTES_MERGE_WORKTREE.\n \t */\n \n-\tif (delete_ref(\"NOTES_MERGE_PARTIAL\", NULL, 0))\n+\tif (delete_ref(\"NOTES_MERGE_PARTIAL\", NULL, 0, NULL))\n \t\tret += error(_(\"failed to delete ref NOTES_MERGE_PARTIAL\"));\n-\tif (delete_ref(\"NOTES_MERGE_REF\", NULL, REF_NODEREF))\n+\tif (delete_ref(\"NOTES_MERGE_REF\", NULL, REF_NODEREF, NULL))\n \t\tret += error(_(\"failed to delete ref NOTES_MERGE_REF\"));\n \tif (notes_merge_abort(o))\n \t\tret += error(_(\"failed to remove 'git notes merge' worktree\"));\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 5339ed6ad..bfa8a5189 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -691,7 +691,7 @@ static int mv(int argc, const char **argv)\n \t\tread_ref_full(item->string, RESOLVE_REF_READING, oid.hash, &flag);\n \t\tif (!(flag & REF_ISSYMREF))\n \t\t\tcontinue;\n-\t\tif (delete_ref(item->string, NULL, REF_NODEREF))\n+\t\tif (delete_ref(item->string, NULL, REF_NODEREF, NULL))\n \t\t\tdie(_(\"deleting '%s' failed\"), item->string);\n \t}\n \tfor (i = 0; i < remote_branches.nr; i++) {\n@@ -1248,7 +1248,7 @@ static int set_head(int argc, const char **argv)\n \t\t\thead_name = xstrdup(states.heads.items[0].string);\n \t\tfree_remote_ref_states(&states);\n \t} else if (opt_d && !opt_a && argc == 1) {\n-\t\tif (delete_ref(buf.buf, NULL, REF_NODEREF))\n+\t\tif (delete_ref(buf.buf, NULL, REF_NODEREF, NULL))\n \t\t\tresult |= error(_(\"Could not delete %s\"), buf.buf);\n \t} else\n \t\tusage_with_options(builtin_remote_sethead_usage, options);\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex b58c714cb..d32d0a3ae 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -121,7 +121,7 @@ static int for_each_replace_name(const char **argv, each_replace_name_fn fn)\n static int delete_replace_ref(const char *name, const char *ref,\n \t\t\t const unsigned char *sha1)\n {\n-\tif (delete_ref(ref, sha1, 0))\n+\tif (delete_ref(ref, sha1, 0, NULL))\n \t\treturn 1;\n \tprintf(\"Deleted replace ref '%s'\\n\", name);\n \treturn 0;\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 8ab915bfc..cccd3f099 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -256,7 +256,7 @@ static int reset_refs(const char *rev, const struct object_id *oid)\n \t\tupdate_ref_oid(msg.buf, \"ORIG_HEAD\", orig, old_orig, 0,\n \t\t\t UPDATE_REFS_MSG_ON_ERR);\n \t} else if (old_orig)\n-\t\tdelete_ref(\"ORIG_HEAD\", old_orig->hash, 0);\n+\t\tdelete_ref(\"ORIG_HEAD\", old_orig->hash, 0, NULL);\n \tset_reflog_message(&msg, \"updating HEAD\", rev);\n \tupdate_ref_status = update_ref_oid(msg.buf, \"HEAD\", oid, orig, 0,\n \t\t\t\t UPDATE_REFS_MSG_ON_ERR);\ndiff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c\nindex 96eed9446..c9d5bd3e8 100644\n--- a/builtin/symbolic-ref.c\n+++ b/builtin/symbolic-ref.c\n@@ -58,7 +58,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)\n \t\t\tdie(\"Cannot delete %s, not a symbolic ref\", argv[0]);\n \t\tif (!strcmp(argv[0], \"HEAD\"))\n \t\t\tdie(\"deleting '%s' is not allowed\", argv[0]);\n-\t\treturn delete_ref(argv[0], NULL, REF_NODEREF);\n+\t\treturn delete_ref(argv[0], NULL, REF_NODEREF, NULL);\n \t}\n \n \tswitch (argc) {\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex e40c4a967..850a0674c 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -97,7 +97,7 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn,\n static int delete_tag(const char *name, const char *ref,\n \t\t const unsigned char *sha1, const void *cb_data)\n {\n-\tif (delete_ref(ref, sha1, 0))\n+\tif (delete_ref(ref, sha1, 0, NULL))\n \t\treturn 1;\n \tprintf(_(\"Deleted tag '%s' (was %s)\\n\"), name, find_unique_abbrev(sha1, DEFAULT_ABBREV));\n \treturn 0;\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 7f30d3a76..a41f9adf1 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -435,7 +435,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\t */\n \t\treturn delete_ref(refname,\n \t\t\t\t (oldval && !is_null_sha1(oldsha1)) ? oldsha1 : NULL,\n-\t\t\t\t flags);\n+\t\t\t\t flags, NULL);\n \telse\n \t\treturn update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,\n \t\t\t\t flags | create_reflog_flag,\ndiff --git a/fast-import.c b/fast-import.c\nindex 64fe602f0..07412a85a 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1752,7 +1752,7 @@ static int update_branch(struct branch *b)\n \n \tif (is_null_sha1(b->sha1)) {\n \t\tif (b->delete)\n-\t\t\tdelete_ref(b->name, NULL, 0);\n+\t\t\tdelete_ref(b->name, NULL, 0, NULL);\n \t\treturn 0;\n \t}\n \tif (read_ref(b->name, old_sha1))\ndiff --git a/refs.c b/refs.c\nindex cd36b64ed..4c80ebf2e 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -592,7 +592,7 @@ static int delete_pseudoref(const char *pseudoref, const unsigned char *old_sha1\n }\n \n int delete_ref(const char *refname, const unsigned char *old_sha1,\n-\t unsigned int flags)\n+\t unsigned int flags, const char *msg)\n {\n \tstruct ref_transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n@@ -603,7 +603,7 @@ int delete_ref(const char *refname, const unsigned char *old_sha1,\n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, refname, old_sha1,\n-\t\t\t\t flags, NULL, &err) ||\n+\t\t\t\t flags, msg, &err) ||\n \t ref_transaction_commit(transaction, &err)) {\n \t\terror(\"%s\", err.buf);\n \t\tref_transaction_free(transaction);\ndiff --git a/refs.h b/refs.h\nindex 9fbff90e7..81627a63d 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -277,7 +277,7 @@ int reflog_exists(const char *refname);\n * be NULL_SHA1. flags is passed through to ref_transaction_delete().\n */\n int delete_ref(const char *refname, const unsigned char *old_sha1,\n-\t unsigned int flags);\n+\t unsigned int flags, const char *msg);\n \n /*\n * Delete the specified references. If there are any problems, emit\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex c041d4ba2..ffa75d816 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2489,7 +2489,7 @@ static int files_delete_refs(struct ref_store *ref_store,\n \tfor (i = 0; i < refnames->nr; i++) {\n \t\tconst char *refname = refnames->items[i].string;\n \n-\t\tif (delete_ref(refname, NULL, flags))\n+\t\tif (delete_ref(refname, NULL, flags, NULL))\n \t\t\tresult |= error(_(\"could not remove reference %s\"), refname);\n \t}\n \n@@ -2616,7 +2616,7 @@ static int files_rename_ref(struct ref_store *ref_store,\n \t\treturn error(\"unable to move logfile logs/%s to \"TMP_RENAMED_LOG\": %s\",\n \t\t\toldrefname, strerror(errno));\n \n-\tif (delete_ref(oldrefname, orig_sha1, REF_NODEREF)) {\n+\tif (delete_ref(oldrefname, orig_sha1, REF_NODEREF, NULL)) {\n \t\terror(\"unable to delete old %s\", oldrefname);\n \t\tgoto rollback;\n \t}\n@@ -2630,7 +2630,7 @@ static int files_rename_ref(struct ref_store *ref_store,\n \t */\n \tif (!read_ref_full(newrefname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n \t\t\t sha1, NULL) &&\n-\t delete_ref(newrefname, NULL, REF_NODEREF)) {\n+\t delete_ref(newrefname, NULL, REF_NODEREF, NULL)) {\n \t\tif (errno==EISDIR) {\n \t\t\tstruct strbuf path = STRBUF_INIT;\n \t\t\tint result;\ndiff --git a/transport.c b/transport.c\nindex d72e08948..e025d9b29 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -299,7 +299,7 @@ void transport_update_tracking_ref(struct remote *remote, struct ref *ref, int v\n \t\tif (verbose)\n \t\t\tfprintf(stderr, \"updating local tracking ref '%s'\\n\", rs.dst);\n \t\tif (ref->deletion) {\n-\t\t\tdelete_ref(rs.dst, NULL, 0);\n+\t\t\tdelete_ref(rs.dst, NULL, 0, NULL);\n \t\t} else\n \t\t\tupdate_ref(\"update by push\", rs.dst,\n \t\t\t\t\tref->new_oid.hash, NULL, 0, 0);\n-- \n2.11.1\n\n"},{"id":"311837","messageId":"20170217035800.13214-3-kyle@kyleam.com","threadId":"44896","inReplyTo":"20170217035800.13214-1-kyle@kyleam.com","subject":"[PATCH 2/3] update-ref: pass reflog message argument to delete_refs","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-17T03:57:59Z","receivedAt":"2017-02-17T03:59:02Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Now that delete_refs() accepts a reflog message, pass the\nuser-provided message to delete_refs() rather than silently dropping\nit. The doesn't matter for the deleted ref's log because the log is\ndeleted along with the ref, but this entry will show up in HEAD's\nreflog when deleting a checked out branch.\n\nSigned-off-by: Kyle Meyer \n---\nI'm not sure if the test here (or in the next patch) is worth including.\n\n builtin/update-ref.c | 2 +-\n t/t1400-update-ref.sh | 9 +++++++++\n 2 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex a41f9adf1..f642acc22 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -435,7 +435,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\t */\n \t\treturn delete_ref(refname,\n \t\t\t\t (oldval && !is_null_sha1(oldsha1)) ? oldsha1 : NULL,\n-\t\t\t\t flags, NULL);\n+\t\t\t\t flags, msg);\n \telse\n \t\treturn update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,\n \t\t\t\t flags | create_reflog_flag,\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex b0ffc0b57..65918d984 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -85,6 +85,15 @@ test_expect_success \"delete $m (by HEAD)\" '\n '\n rm -f .git/$m\n \n+test_expect_success \"deleting current branch adds message to HEAD's log\" '\n+\tgit update-ref $m $A &&\n+\tgit symbolic-ref HEAD $m &&\n+\tgit update-ref -mdelmsg -d $m &&\n+\t! test -f .git/$m &&\n+\tgrep \"delmsg$\" .git/logs/HEAD >/dev/null\n+'\n+rm -f .git/$m\n+\n test_expect_success 'update-ref does not create reflogs by default' '\n \ttest_when_finished \"git update-ref -d $outside\" &&\n \tgit update-ref $outside $A &&\n-- \n2.11.1\n\n"},{"id":"311838","messageId":"20170217035800.13214-4-kyle@kyleam.com","threadId":"44896","inReplyTo":"20170217035800.13214-1-kyle@kyleam.com","subject":"[PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-17T03:58:00Z","receivedAt":"2017-02-17T03:59:05Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"When the current branch is renamed, the deletion of the old ref is\nrecorded in HEAD's log with an empty message. Now that delete_refs()\naccepts a reflog message, provide a more descriptive message. This\nmessage will not be included in the reflog of the renamed branch, but\nits log already covers the renaming event with a message of \"Branch:\nrenamed ...\".\n\nSigned-off-by: Kyle Meyer \n---\n refs/files-backend.c | 8 +++++++-\n t/t3200-branch.sh | 4 ++++\n 2 files changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex ffa75d816..bb5df7ee6 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2598,6 +2598,7 @@ static int files_rename_ref(struct ref_store *ref_store,\n \tstruct stat loginfo;\n \tint log = !lstat(git_path(\"logs/%s\", oldrefname), &loginfo);\n \tstruct strbuf err = STRBUF_INIT;\n+\tstruct strbuf logmsg_del = STRBUF_INIT;\n \n \tif (log && S_ISLNK(loginfo.st_mode))\n \t\treturn error(\"reflog for %s is a symlink\", oldrefname);\n@@ -2616,10 +2617,15 @@ static int files_rename_ref(struct ref_store *ref_store,\n \t\treturn error(\"unable to move logfile logs/%s to \"TMP_RENAMED_LOG\": %s\",\n \t\t\toldrefname, strerror(errno));\n \n-\tif (delete_ref(oldrefname, orig_sha1, REF_NODEREF, NULL)) {\n+\tstrbuf_addf(&logmsg_del, \"Deleted %s\", oldrefname);\n+\n+\tif (delete_ref(oldrefname, orig_sha1, REF_NODEREF, logmsg_del.buf)) {\n \t\terror(\"unable to delete old %s\", oldrefname);\n+\t\tstrbuf_release(&logmsg_del);\n \t\tgoto rollback;\n \t}\n+\tstrbuf_release(&logmsg_del);\n+\n \n \t/*\n \t * Since we are doing a shallow lookup, sha1 is not the\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 8a833f354..4af7cd2b7 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -139,6 +139,10 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou\n \ttest $(git rev-parse --abbrev-ref HEAD) = bam\n '\n \n+test_expect_success 'git branch -M baz bam should add entry to .git/logs/HEAD' '\n+\tgrep \"Deleted refs/heads/baz$\" .git/logs/HEAD >/dev/null\n+'\n+\n test_expect_success 'git branch -M baz bam should succeed when baz is checked out as linked working tree' '\n \tgit checkout master &&\n \tgit worktree add -b baz bazdir &&\n-- \n2.11.1\n\n"},{"id":"311845","messageId":"20170217081205.zn7j6d5cffgdvfbn@sigill.intra.peff.net","threadId":"44896","inReplyTo":"20170217035800.13214-2-kyle@kyleam.com","subject":"Re: [PATCH 1/3] delete_refs(): accept a reflog message argument","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-17T08:12:06Z","receivedAt":"2017-02-17T08:12:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 16, 2017 at 10:57:58PM -0500, Kyle Meyer wrote:\n\n> When the current branch is renamed with 'git branch -m/-M' or deleted\n> with 'git update-ref -m -d', the event is recorded in HEAD's log\n> with an empty message.\n> \n> In preparation for adding a more meaningful message to HEAD's log in\n> these cases, update delete_ref() to take a message argument and pass\n> it along to ref_transaction_delete(). Modify all callers to pass NULL\n> for the new message argument; no change in behavior is intended.\n\nSeems like a good first step.\n\n> diff --git a/refs.h b/refs.h\n> index 9fbff90e7..81627a63d 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -277,7 +277,7 @@ int reflog_exists(const char *refname);\n> * be NULL_SHA1. flags is passed through to ref_transaction_delete().\n> */\n> int delete_ref(const char *refname, const unsigned char *old_sha1,\n> -\t unsigned int flags);\n> +\t unsigned int flags, const char *msg);\n\nShould the \"msg\" argument go at the beginning, to match update_ref()?\n\n-Peff\n"},{"id":"311846","messageId":"20170217081752.oqozaaoscdwwzqce@sigill.intra.peff.net","threadId":"44896","inReplyTo":"20170217035800.13214-1-kyle@kyleam.com","subject":"Re: [PATCH 0/3] delete_ref(): support reflog messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-17T08:17:52Z","receivedAt":"2017-02-17T08:18:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 16, 2017 at 10:57:57PM -0500, Kyle Meyer wrote:\n\n> [Sorry for the slow response.]\n\nNo problem. The pace of open source varies wildly. :)\n\n> > - \"git branch -m\" does seem to realize when we are renaming HEAD,\n> > because it updates HEAD to point to the new branch name. But it\n> > should probably insert another reflog entry mentioning the rename\n> > (we do for \"git checkout foo\", even when \"foo\" has the same sha1 as\n> > the current HEAD).\n> \n> I haven't worked out how to do this part yet. I'm guessing the change\n> will involve modifying split_head_update().\n> \n> If this is added, should it be instead of, rather than in addition to,\n> the deletion entry? If a \"Branch: renamed ...\" entry is present, it\n> doesn't seem like the deletion entry is providing any additional\n> information.\n\nI think you could do an \"instead of\" that goes from sha1 X to X, and\njust mentions the rename. Or you could add a second entry after the\ndelete that takes it from 0{40} back to X.\n\nI suspect the latter is easier to do, and I doubt anybody would care\nthat much of the exact form. These entries aren't really doing anything\nfor reachability. They're just giving an audit log of what happened. So\nI don't think anybody would really care unless they were debugging a\nconfusing situation by hand. And as long as there's enough information\nto figure out what happened, they'll be happy.\n\n-Peff\n"},{"id":"311847","messageId":"20170217082253.kxjezkxfqkfxjhzr@sigill.intra.peff.net","threadId":"44896","inReplyTo":"20170217035800.13214-3-kyle@kyleam.com","subject":"Re: [PATCH 2/3] update-ref: pass reflog message argument to delete_refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-17T08:22:54Z","receivedAt":"2017-02-17T08:23:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 16, 2017 at 10:57:59PM -0500, Kyle Meyer wrote:\n\n> Now that delete_refs() accepts a reflog message, pass the\n> user-provided message to delete_refs() rather than silently dropping\n> it. The doesn't matter for the deleted ref's log because the log is\n> deleted along with the ref, but this entry will show up in HEAD's\n> reflog when deleting a checked out branch.\n\nSounds good.\n\n> diff --git a/builtin/update-ref.c b/builtin/update-ref.c\n> index a41f9adf1..f642acc22 100644\n> --- a/builtin/update-ref.c\n> +++ b/builtin/update-ref.c\n> @@ -435,7 +435,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n> \t\t */\n> \t\treturn delete_ref(refname,\n> \t\t\t\t (oldval && !is_null_sha1(oldsha1)) ? oldsha1 : NULL,\n> -\t\t\t\t flags, NULL);\n> +\t\t\t\t flags, msg);\n\nThis looks obviously correct.\n\n> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\n> index b0ffc0b57..65918d984 100755\n> --- a/t/t1400-update-ref.sh\n> +++ b/t/t1400-update-ref.sh\n> @@ -85,6 +85,15 @@ test_expect_success \"delete $m (by HEAD)\" '\n> '\n> rm -f .git/$m\n> \n> +test_expect_success \"deleting current branch adds message to HEAD's log\" '\n> +\tgit update-ref $m $A &&\n> +\tgit symbolic-ref HEAD $m &&\n> +\tgit update-ref -mdelmsg -d $m &&\n> +\t! test -f .git/$m &&\n> +\tgrep \"delmsg$\" .git/logs/HEAD >/dev/null\n> +'\n> +rm -f .git/$m\n\nI think covering this with a test is good.\n\nI don't know if it's also worth testing that deleting via HEAD also\nwrites the reflog. I.e.,:\n\n git update-ref -m delete-by-head -d HEAD\n\nSome of the style here is a bit out-dated, but I think you are just\nmatching the surrounding tests. So that's OK by me (though a patch to\nmodernize the whole thing would be welcome, too).\n\nFor reference, the two things I notice are:\n\n - we prefer test_path_is_missing to \"! test -f\" these days.\n\n - we don't redirect the output of grep (it's handled already in\n non-verbose mode, and in verbose mode we try to be...verbose).\n\n-Peff\n"},{"id":"311849","messageId":"20170217083112.vn7m4udsopmlvnn5@sigill.intra.peff.net","threadId":"44896","inReplyTo":"20170217035800.13214-4-kyle@kyleam.com","subject":"Re: [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-17T08:31:12Z","receivedAt":"2017-02-17T08:31:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 16, 2017 at 10:58:00PM -0500, Kyle Meyer wrote:\n\n> When the current branch is renamed, the deletion of the old ref is\n> recorded in HEAD's log with an empty message. Now that delete_refs()\n> accepts a reflog message, provide a more descriptive message. This\n> message will not be included in the reflog of the renamed branch, but\n> its log already covers the renaming event with a message of \"Branch:\n> renamed ...\".\n\nRight, makes sense. The code overall looks fine, though I have one\nnit:\n\n> @@ -2616,10 +2617,15 @@ static int files_rename_ref(struct ref_store *ref_store,\n> \t\treturn error(\"unable to move logfile logs/%s to \"TMP_RENAMED_LOG\": %s\",\n> \t\t\toldrefname, strerror(errno));\n> \n> -\tif (delete_ref(oldrefname, orig_sha1, REF_NODEREF, NULL)) {\n> +\tstrbuf_addf(&logmsg_del, \"Deleted %s\", oldrefname);\n\nWe've been anything but consistent with our reflog messages in the past,\nbut I think the general guideline for new ones is to use \"command:\naction\". Of course we don't _know_ the action here, because we're deep\nin rename_ref().\n\nShould we have the caller pass it in for us, as we do with delete_ref()\nand update_ref()?\n\nI see we actually already have a \"logmsg\" parameter. It already says\n\"Branch: renamed %s to %s\". Could we just reuse that? I know that this\nstep is technically just the deletion, but I think it more accurately\ndescribes the whole operation that the deletion is part of.\n\n-Peff\n"},{"id":"311908","messageId":"xmqqshncu4r4.fsf@gitster.mtv.corp.google.com","threadId":"44896","inReplyTo":"20170217081205.zn7j6d5cffgdvfbn@sigill.intra.peff.net","subject":"Re: [PATCH 1/3] delete_refs(): accept a reflog message argument","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-17T17:05:51Z","receivedAt":"2017-02-17T17:05:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> diff --git a/refs.h b/refs.h\n>> index 9fbff90e7..81627a63d 100644\n>> --- a/refs.h\n>> +++ b/refs.h\n>> @@ -277,7 +277,7 @@ int reflog_exists(const char *refname);\n>> * be NULL_SHA1. flags is passed through to ref_transaction_delete().\n>> */\n>> int delete_ref(const char *refname, const unsigned char *old_sha1,\n>> -\t unsigned int flags);\n>> +\t unsigned int flags, const char *msg);\n>\n> Should the \"msg\" argument go at the beginning, to match update_ref()?\n\nProbably. rename/create have the message at the end but their\nparameters are very different from update/delete. The parameters\nupdate and delete take are not identical, but we can view them as a\nlot more similar than the other two. So I think it makes sense for\ndelete to try matching update, even though trying to make all four\nthe same may proabably be pointless.\n"},{"id":"311911","messageId":"xmqqo9y0u4hs.fsf@gitster.mtv.corp.google.com","threadId":"44896","inReplyTo":"20170217082253.kxjezkxfqkfxjhzr@sigill.intra.peff.net","subject":"Re: [PATCH 2/3] update-ref: pass reflog message argument to delete_refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-17T17:11:27Z","receivedAt":"2017-02-17T17:11:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> ...\n\nAll good review comments.\n\nI briefly wondered if recording the deletion of the current branch\nin HEAD's reflog has much practical values (Porcelain \"git branch\"\nwould not even allow deletion in the first place), but because it is\na rare and unusual event, it probably makes more sense to have a\nrecord of it.\n"},{"id":"311914","messageId":"xmqqk28ou2o1.fsf@gitster.mtv.corp.google.com","threadId":"44896","inReplyTo":"20170217083112.vn7m4udsopmlvnn5@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-17T17:50:54Z","receivedAt":"2017-02-17T18:16:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Feb 16, 2017 at 10:58:00PM -0500, Kyle Meyer wrote:\n>\n>> When the current branch is renamed, the deletion of the old ref is\n>> recorded in HEAD's log with an empty message. Now that delete_refs()\n>> accepts a reflog message, provide a more descriptive message. This\n>> message will not be included in the reflog of the renamed branch, but\n>> its log already covers the renaming event with a message of \"Branch:\n>> renamed ...\".\n>\n> Right, makes sense. The code overall looks fine, though I have one\n> nit:\n>\n>> @@ -2616,10 +2617,15 @@ static int files_rename_ref(struct ref_store *ref_store,\n>> \t\treturn error(\"unable to move logfile logs/%s to \"TMP_RENAMED_LOG\": %s\",\n>> \t\t\toldrefname, strerror(errno));\n>> \n>> -\tif (delete_ref(oldrefname, orig_sha1, REF_NODEREF, NULL)) {\n>> +\tstrbuf_addf(&logmsg_del, \"Deleted %s\", oldrefname);\n>\n> We've been anything but consistent with our reflog messages in the past,\n> but I think the general guideline for new ones is to use \"command:\n> action\". Of course we don't _know_ the action here, because we're deep\n> in rename_ref().\n>\n> Should we have the caller pass it in for us, as we do with delete_ref()\n> and update_ref()?\n>\n> I see we actually already have a \"logmsg\" parameter. It already says\n> \"Branch: renamed %s to %s\". Could we just reuse that? I know that this\n> step is technically just the deletion, but I think it more accurately\n> describes the whole operation that the deletion is part of.\n\nTrue, but stepping back a bit,...\n\nDo we even want these \"internal\" delete_ref() invocations to be\nlogged in HEAD's reflog? I understand that this is inside the\nimplementation of renaming an old ref to a new ref, and reflog\nmessage given to delete_ref() would matter only if the HEAD happens\nto be pointing at old ref---but then HEAD will be repointed to the\nnew ref by somebody else [*1*] that called this function to rename\nold to new and it _will_ log it. So I am not sure if it is a good\nthing to describe the deletion more readably with a message (which\nis what this patch does) in the first place. If we can just say\n\"don't log this deletion event in HEAD's reflog\", wouldn't that be\nmore desirable?\n\n\n[Footnote]\n\n*1* Is the reason why the code in files_rename_ref() we are looking\n at does not adjust HEAD to point at the new ref is because it is\n just handing one ref-store and obviouvious to symrefs in other\n backends?\n"},{"id":"311928","messageId":"20170217194350.prhp5joh33cbvwsd@sigill.intra.peff.net","threadId":"44896","inReplyTo":"xmqqk28ou2o1.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-17T19:43:50Z","receivedAt":"2017-02-17T19:43:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 17, 2017 at 09:50:54AM -0800, Junio C Hamano wrote:\n\n> > I see we actually already have a \"logmsg\" parameter. It already says\n> > \"Branch: renamed %s to %s\". Could we just reuse that? I know that this\n> > step is technically just the deletion, but I think it more accurately\n> > describes the whole operation that the deletion is part of.\n> \n> True, but stepping back a bit,...\n> \n> Do we even want these \"internal\" delete_ref() invocations to be\n> logged in HEAD's reflog? I understand that this is inside the\n> implementation of renaming an old ref to a new ref, and reflog\n> message given to delete_ref() would matter only if the HEAD happens\n> to be pointing at old ref---but then HEAD will be repointed to the\n> new ref by somebody else [*1*] that called this function to rename\n> old to new and it _will_ log it. So I am not sure if it is a good\n> thing to describe the deletion more readably with a message (which\n> is what this patch does) in the first place. If we can just say\n> \"don't log this deletion event in HEAD's reflog\", wouldn't that be\n> more desirable?\n\nYes. I think the options are basically (in order of decreasing\npreference in my opinion):\n\n 1. Log a rename entry (same sha1, but note the rename in the free-form\n text).\n\n 2. Log a delete (sha1 goes to null) followed by a creation (from null\n back to the original sha1).\n\n 3. Log nothing at all for HEAD.\n\nThis does half of (2). If we do the second half, then I'd prefer it to\n(3). But if we can do (1), that is better still (IMHO).\n\n> *1* Is the reason why the code in files_rename_ref() we are looking\n> at does not adjust HEAD to point at the new ref is because it is\n> just handing one ref-store and obviouvious to symrefs in other\n> backends?\n\nI'm actually confused about which bit of code is updating HEAD. I do not\nsee it either in files_rename_ref() or in the caller. Yet it clearly\nhappens. But that is the code that would know enough to do (1) or the\nsecond half of (2) above.\n\n-Peff\n"},{"id":"311930","messageId":"20170217195549.z6uyy7hbbhj5avh7@sigill.intra.peff.net","threadId":"44896","inReplyTo":"20170217194350.prhp5joh33cbvwsd@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-17T19:55:49Z","receivedAt":"2017-02-17T20:08:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 17, 2017 at 02:43:50PM -0500, Jeff King wrote:\n\n> Yes. I think the options are basically (in order of decreasing\n> preference in my opinion):\n> \n> 1. Log a rename entry (same sha1, but note the rename in the free-form\n> text).\n> \n> 2. Log a delete (sha1 goes to null) followed by a creation (from null\n> back to the original sha1).\n> \n> 3. Log nothing at all for HEAD.\n> \n> This does half of (2). If we do the second half, then I'd prefer it to\n> (3). But if we can do (1), that is better still (IMHO).\n> \n> > *1* Is the reason why the code in files_rename_ref() we are looking\n> > at does not adjust HEAD to point at the new ref is because it is\n> > just handing one ref-store and obviouvious to symrefs in other\n> > backends?\n> \n> I'm actually confused about which bit of code is updating HEAD. I do not\n> see it either in files_rename_ref() or in the caller. Yet it clearly\n> happens. But that is the code that would know enough to do (1) or the\n> second half of (2) above.\n\nAh, I found it. It's in replace_each_worktree_head_symref() these days,\nwhich does not bother to pass a log message.\n\nSo I think the second half of (2) is probably something like the patch\nbelow.\n\nThinking on it more, we probably _do_ want two entries. Because the\noperations are not atomic, it's possible that we may end up in a\nhalf-way state after the first entry is written. And when debugging such\na case, I'd much rather see the first half of the operation logged than\nnothing at all.\n\n-Peff\n\n---\ndiff --git a/branch.c b/branch.c\nindex b955d4f31..5c12036b0 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -345,7 +345,8 @@ void die_if_checked_out(const char *branch, int ignore_current_worktree)\n \t branch, wt->path);\n }\n \n-int replace_each_worktree_head_symref(const char *oldref, const char *newref)\n+int replace_each_worktree_head_symref(const char *oldref, const char *newref,\n+\t\t\t\t const char *logmsg)\n {\n \tint ret = 0;\n \tstruct worktree **worktrees = get_worktrees(0);\n@@ -358,7 +359,7 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref)\n \t\t\tcontinue;\n \n \t\tif (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),\n-\t\t\t\t\t newref)) {\n+\t\t\t\t\t newref, logmsg)) {\n \t\t\tret = -1;\n \t\t\terror(_(\"HEAD of working tree %s is not updated\"),\n \t\t\t worktrees[i]->path);\ndiff --git a/branch.h b/branch.h\nindex 3103eb9ad..b07788558 100644\n--- a/branch.h\n+++ b/branch.h\n@@ -71,6 +71,7 @@ extern void die_if_checked_out(const char *branch, int ignore_current_worktree);\n * This will be used when renaming a branch. Returns 0 if successful, non-zero\n * otherwise.\n */\n-extern int replace_each_worktree_head_symref(const char *oldref, const char *newref);\n+extern int replace_each_worktree_head_symref(const char *oldref, const char *newref,\n+\t\t\t\t\t const char *logmsg);\n \n #endif\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex cbaa6d03c..383005912 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -471,14 +471,15 @@ static void rename_branch(const char *oldname, const char *newname, int force)\n \n \tif (rename_ref(oldref.buf, newref.buf, logmsg.buf))\n \t\tdie(_(\"Branch rename failed\"));\n-\tstrbuf_release(&logmsg);\n \n \tif (recovery)\n \t\twarning(_(\"Renamed a misnamed branch '%s' away\"), oldref.buf + 11);\n \n-\tif (replace_each_worktree_head_symref(oldref.buf, newref.buf))\n+\tif (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf))\n \t\tdie(_(\"Branch renamed to %s, but HEAD is not updated!\"), newname);\n \n+\tstrbuf_release(&logmsg);\n+\n \tstrbuf_addf(&oldsection, \"branch.%s\", oldref.buf + 11);\n \tstrbuf_release(&oldref);\n \tstrbuf_addf(&newsection, \"branch.%s\", newref.buf + 11);\ndiff --git a/refs.h b/refs.h\nindex 9fbff90e7..b33035c4a 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -334,7 +334,8 @@ int create_symref(const char *refname, const char *target, const char *logmsg);\n * $GIT_DIR points to.\n * Return 0 if successful, non-zero otherwise.\n * */\n-int set_worktree_head_symref(const char *gitdir, const char *target);\n+int set_worktree_head_symref(const char *gitdir, const char *target,\n+\t\t\t const char *logmsg);\n \n enum action_on_err {\n \tUPDATE_REFS_MSG_ON_ERR,\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 4f1a88f6d..fa8d08e3c 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3024,7 +3024,7 @@ static int files_create_symref(struct ref_store *ref_store,\n \treturn ret;\n }\n \n-int set_worktree_head_symref(const char *gitdir, const char *target)\n+int set_worktree_head_symref(const char *gitdir, const char *target, const char *logmsg)\n {\n \tstatic struct lock_file head_lock;\n \tstruct ref_lock *lock;\n@@ -3052,7 +3052,7 @@ int set_worktree_head_symref(const char *gitdir, const char *target)\n \tlock->lk = &head_lock;\n \tlock->ref_name = xstrdup(head_rel);\n \n-\tret = create_symref_locked(lock, head_rel, target, NULL);\n+\tret = create_symref_locked(lock, head_rel, target, logmsg);\n \n \tunlock_ref(lock); /* will free lock */\n \tstrbuf_release(&head_path);\n"},{"id":"311948","messageId":"xmqqino8qx5t.fsf@gitster.mtv.corp.google.com","threadId":"44896","inReplyTo":"20170217195549.z6uyy7hbbhj5avh7@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-17T22:18:06Z","receivedAt":"2017-02-17T22:18:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Thinking on it more, we probably _do_ want two entries. Because the\n> operations are not atomic, it's possible that we may end up in a\n> half-way state after the first entry is written. And when debugging such\n> a case, I'd much rather see the first half of the operation logged than\n> nothing at all.\n\nYes, that sounds right.\n"},{"id":"311982","messageId":"874lzsqtk0.fsf@kyleam.com","threadId":"44896","inReplyTo":"xmqqshncu4r4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/3] delete_refs(): accept a reflog message argument","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-17T23:35:59Z","receivedAt":"2017-02-17T23:37:31Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>>> diff --git a/refs.h b/refs.h\n>>> index 9fbff90e7..81627a63d 100644\n>>> --- a/refs.h\n>>> +++ b/refs.h\n>>> @@ -277,7 +277,7 @@ int reflog_exists(const char *refname);\n>>> * be NULL_SHA1. flags is passed through to ref_transaction_delete().\n>>> */\n>>> int delete_ref(const char *refname, const unsigned char *old_sha1,\n>>> -\t unsigned int flags);\n>>> +\t unsigned int flags, const char *msg);\n>>\n>> Should the \"msg\" argument go at the beginning, to match update_ref()?\n>\n> Probably. rename/create have the message at the end but their\n> parameters are very different from update/delete. The parameters\n> update and delete take are not identical, but we can view them as a\n> lot more similar than the other two. So I think it makes sense for\n> delete to try matching update, even though trying to make all four\n> the same may proabably be pointless.\n\nI put \"msg\" after \"flags\" because that's where it occurs in\nref_transaction_delete(), but matching update_ref() makes sense.\n\n-- \nKyle\n"},{"id":"311985","messageId":"8737fcqtcj.fsf@kyleam.com","threadId":"44896","inReplyTo":"20170217082253.kxjezkxfqkfxjhzr@sigill.intra.peff.net","subject":"Re: [PATCH 2/3] update-ref: pass reflog message argument to delete_refs","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-17T23:40:28Z","receivedAt":"2017-02-17T23:40:35Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Jeff King writes:\n\n[...]\n\n>> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\n>> index b0ffc0b57..65918d984 100755\n>> --- a/t/t1400-update-ref.sh\n>> +++ b/t/t1400-update-ref.sh\n>> @@ -85,6 +85,15 @@ test_expect_success \"delete $m (by HEAD)\" '\n>> '\n>> rm -f .git/$m\n>> \n>> +test_expect_success \"deleting current branch adds message to HEAD's log\" '\n>> +\tgit update-ref $m $A &&\n>> +\tgit symbolic-ref HEAD $m &&\n>> +\tgit update-ref -mdelmsg -d $m &&\n>> +\t! test -f .git/$m &&\n>> +\tgrep \"delmsg$\" .git/logs/HEAD >/dev/null\n>> +'\n>> +rm -f .git/$m\n>\n> I think covering this with a test is good.\n>\n> I don't know if it's also worth testing that deleting via HEAD also\n> writes the reflog. I.e.,:\n>\n> git update-ref -m delete-by-head -d HEAD\n\nSeems reasonable to cover this case as well.\n\n> Some of the style here is a bit out-dated, but I think you are just\n> matching the surrounding tests. So that's OK by me (though a patch to\n> modernize the whole thing would be welcome, too).\n\nRight. I'd be happy to follow up with a patch updating the style in\nt1400-update-ref.sh.\n\n> For reference, the two things I notice are:\n>\n> - we prefer test_path_is_missing to \"! test -f\" these days.\n>\n> - we don't redirect the output of grep (it's handled already in\n> non-verbose mode, and in verbose mode we try to be...verbose).\n\nWould moving cleanup like \"rm -f .git/$m\" within the test's body using\ntest_when_finished also be preferred?\n\n-- \nKyle\n"},{"id":"311986","messageId":"20170217234128.d7uhxu4fjzsc4yjh@sigill.intra.peff.net","threadId":"44896","inReplyTo":"8737fcqtcj.fsf@kyleam.com","subject":"Re: [PATCH 2/3] update-ref: pass reflog message argument to delete_refs","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-17T23:41:28Z","receivedAt":"2017-02-17T23:41:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 17, 2017 at 06:40:28PM -0500, Kyle Meyer wrote:\n\n> > For reference, the two things I notice are:\n> >\n> > - we prefer test_path_is_missing to \"! test -f\" these days.\n> >\n> > - we don't redirect the output of grep (it's handled already in\n> > non-verbose mode, and in verbose mode we try to be...verbose).\n> \n> Would moving cleanup like \"rm -f .git/$m\" within the test's body using\n> test_when_finished also be preferred?\n\nYeah, that too. I forgot to mention it.\n\n-Peff\n"},{"id":"311987","messageId":"871suwqtar.fsf@kyleam.com","threadId":"44896","inReplyTo":"xmqqk28ou2o1.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-17T23:41:32Z","receivedAt":"2017-02-17T23:41:40Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Junio C Hamano writes:\n\n[...]\n\n> Do we even want these \"internal\" delete_ref() invocations to be\n> logged in HEAD's reflog? I understand that this is inside the\n> implementation of renaming an old ref to a new ref, and reflog\n> message given to delete_ref() would matter only if the HEAD happens\n> to be pointing at old ref---but then HEAD will be repointed to the\n> new ref by somebody else [*1*] that called this function to rename\n> old to new and it _will_ log it.\n\nI know the discussion has developed further, but just a note that I\nthink the last statement is inaccurate: currently, a rename will not be\nrecorded in HEAD's log. \"git branch -m\" will show a renaming event in\nthe new branch's log, but the only trace of the event in HEAD's log is\nthe deletion entry with an empty message.\n\n-- \nKyle\n"},{"id":"311988","messageId":"87zihkpeoz.fsf@kyleam.com","threadId":"44896","inReplyTo":"20170217195549.z6uyy7hbbhj5avh7@sigill.intra.peff.net","subject":"Re: [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-17T23:42:20Z","receivedAt":"2017-02-17T23:42:26Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Jeff King writes:\n\n> On Fri, Feb 17, 2017 at 02:43:50PM -0500, Jeff King wrote:\n>\n>> Yes. I think the options are basically (in order of decreasing\n>> preference in my opinion):\n>> \n>> 1. Log a rename entry (same sha1, but note the rename in the free-form\n>> text).\n>> \n>> 2. Log a delete (sha1 goes to null) followed by a creation (from null\n>> back to the original sha1).\n>> \n>> 3. Log nothing at all for HEAD.\n>> \n>> This does half of (2). If we do the second half, then I'd prefer it to\n>> (3). But if we can do (1), that is better still (IMHO).\n\n[...]\n\n>> I'm actually confused about which bit of code is updating HEAD. I do not\n>> see it either in files_rename_ref() or in the caller. Yet it clearly\n>> happens. But that is the code that would know enough to do (1) or the\n>> second half of (2) above.\n>\n> Ah, I found it. It's in replace_each_worktree_head_symref() these days,\n> which does not bother to pass a log message.\n> \n> So I think the second half of (2) is probably something like the patch\n> below.\n>\n> Thinking on it more, we probably _do_ want two entries. Because the\n> operations are not atomic, it's possible that we may end up in a\n> half-way state after the first entry is written. And when debugging such\n> a case, I'd much rather see the first half of the operation logged than\n> nothing at all.\n\nOK, I'll have a go at replacing patch 3 with this approach.\n\n-- \nKyle\n"},{"id":"311992","messageId":"20170217235324.n3jabck23bvd4cs2@sigill.intra.peff.net","threadId":"44896","inReplyTo":"871suwqtar.fsf@kyleam.com","subject":"Re: [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-17T23:53:25Z","receivedAt":"2017-02-17T23:54:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 17, 2017 at 06:41:32PM -0500, Kyle Meyer wrote:\n\n> Junio C Hamano writes:\n> \n> [...]\n> \n> > Do we even want these \"internal\" delete_ref() invocations to be\n> > logged in HEAD's reflog? I understand that this is inside the\n> > implementation of renaming an old ref to a new ref, and reflog\n> > message given to delete_ref() would matter only if the HEAD happens\n> > to be pointing at old ref---but then HEAD will be repointed to the\n> > new ref by somebody else [*1*] that called this function to rename\n> > old to new and it _will_ log it.\n> \n> I know the discussion has developed further, but just a note that I\n> think the last statement is inaccurate: currently, a rename will not be\n> recorded in HEAD's log. \"git branch -m\" will show a renaming event in\n> the new branch's log, but the only trace of the event in HEAD's log is\n> the deletion entry with an empty message.\n\nRight. I assumed Junio was talking about the hypothetical behavior we'd\nwant.\n\nYour response did make me think of one other option: if we updated HEAD\n_before_ writing the new ref, then it would automatically get the\n\"create\" half of the rename. IOW, if the rename process were:\n\n 1. delete old; this writes a reflog to HEAD, per the magic\n HEAD-detection in commit_ref_update().\n\n 2. update HEAD to point to new\n\n 3. re-create new; this uses the same magic to write a HEAD reflog.\n\nThat's probably a crazy path to go, though. Right now steps (1) and (3)\nhappen in a low-level function, and step (2) happens outside of it\ncompletely. Arguably it would be good to change that, but I think we\nwant to think of (1) and (3) as an atomic operation. Putting more things\nwhich might fail between them is a bad idea.\n\nSo I think your existing patches (modulo the review comments), plus the\n\"something like this\" that I sent on top are probably a better end\nstate.\n\n-Peff\n"},{"id":"311993","messageId":"xmqqmvdknzm7.fsf@gitster.mtv.corp.google.com","threadId":"44896","inReplyTo":"871suwqtar.fsf@kyleam.com","subject":"Re: [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-17T23:53:20Z","receivedAt":"2017-02-17T23:54:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kyle Meyer writes:\n\n> Junio C Hamano writes:\n>\n> [...]\n>\n>> Do we even want these \"internal\" delete_ref() invocations to be\n>> logged in HEAD's reflog? I understand that this is inside the\n>> implementation of renaming an old ref to a new ref, and reflog\n>> message given to delete_ref() would matter only if the HEAD happens\n>> to be pointing at old ref---but then HEAD will be repointed to the\n>> new ref by somebody else [*1*] that called this function to rename\n>> old to new and it _will_ log it.\n>\n> I know the discussion has developed further, but just a note that I\n> think the last statement is inaccurate: currently, a rename will not be\n> recorded in HEAD's log. \"git branch -m\" will show a renaming event in\n> the new branch's log, but the only trace of the event in HEAD's log is\n> the deletion entry with an empty message.\n\nYes, I think Peff diagnosed it and suggested a fix.\n\n"},{"id":"312176","messageId":"20170221011035.847-1-kyle@kyleam.com","threadId":"44896","inReplyTo":"20170217035800.13214-1-kyle@kyleam.com","subject":"[PATCH v2 0/4] delete_ref: support reflog messages","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-21T01:10:31Z","receivedAt":"2017-02-21T01:10:53Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Thanks to Junio and Peff for their feedback on v1.\n\n https://public-inbox.org/git/20170217035800.13214-1-kyle@kyleam.com/T/#u\n\nChanges from v1:\n\n * \"msg\" is now positioned as the first argument to delete_ref() to\n match update_ref().\n\n 20170217081205.zn7j6d5cffgdvfbn@sigill.intra.peff.net\n\n * A \"delete by head\" test case has been added for the update-ref\n change.\n\n 20170217082253.kxjezkxfqkfxjhzr@sigill.intra.peff.net\n\n * The added tests no longer send grep's output to /dev/null.\n\n 20170217082253.kxjezkxfqkfxjhzr@sigill.intra.peff.net\n\n * Renaming the current branch is represented by two entries in HEAD's\n log, both which reuse the log message passed to rename_ref().\n \n 20170217083112.vn7m4udsopmlvnn5@sigill.intra.peff.net,\n 20170217195549.z6uyy7hbbhj5avh7@sigill.intra.peff.net\n\n * Corrected a few places in the commit messages where \"delete_refs\"\n was written instead of \"delete_ref\".\n\n\nKyle Meyer (4):\n delete_ref: accept a reflog message argument\n update-ref: pass reflog message to delete_ref()\n rename_ref: replace empty message in HEAD's log\n branch: record creation of renamed branch in HEAD's log\n\n branch.c | 5 +++--\n branch.h | 3 ++-\n builtin/am.c | 4 ++--\n builtin/branch.c | 7 ++++---\n builtin/notes.c | 4 ++--\n builtin/remote.c | 4 ++--\n builtin/replace.c | 2 +-\n builtin/reset.c | 2 +-\n builtin/symbolic-ref.c | 2 +-\n builtin/tag.c | 2 +-\n builtin/update-ref.c | 2 +-\n fast-import.c | 2 +-\n refs.c | 6 +++---\n refs.h | 7 ++++---\n refs/files-backend.c | 10 +++++-----\n t/t1400-update-ref.sh | 18 ++++++++++++++++++\n t/t3200-branch.sh | 6 ++++++\n transport.c | 2 +-\n 18 files changed, 58 insertions(+), 30 deletions(-)\n\n-- \n2.11.1\n\n"},{"id":"312177","messageId":"20170221011035.847-2-kyle@kyleam.com","threadId":"44896","inReplyTo":"20170221011035.847-1-kyle@kyleam.com","subject":"[PATCH v2 1/4] delete_ref: accept a reflog message argument","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-21T01:10:32Z","receivedAt":"2017-02-21T01:10:56Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"When the current branch is renamed with 'git branch -m/-M' or deleted\nwith 'git update-ref -m -d', the event is recorded in HEAD's log\nwith an empty message. In preparation for adding a more meaningful\nmessage to HEAD's log in these cases, update delete_ref() to take a\nmessage argument and pass it along to ref_transaction_delete().\nModify all callers to pass NULL for the new message argument; no\nchange in behavior is intended.\n\nNote that this is relevant for HEAD's log but not for the deleted\nref's log, which is currently deleted along with the ref. Even if it\nwere not, an entry for the deletion wouldn't be present in the deleted\nref's log. files_transaction_commit() writes to the log if\nREF_NEEDS_COMMIT or REF_LOG_ONLY are set, but lock_ref_for_update()\ndoesn't set REF_NEEDS_COMMIT for the deleted ref because REF_DELETING\nis set. In contrast, the update for HEAD has REF_LOG_ONLY set by\nsplit_head_update(), resulting in the deletion being logged.\n\nSigned-off-by: Kyle Meyer \n---\n builtin/am.c | 4 ++--\n builtin/branch.c | 2 +-\n builtin/notes.c | 4 ++--\n builtin/remote.c | 4 ++--\n builtin/replace.c | 2 +-\n builtin/reset.c | 2 +-\n builtin/symbolic-ref.c | 2 +-\n builtin/tag.c | 2 +-\n builtin/update-ref.c | 2 +-\n fast-import.c | 2 +-\n refs.c | 6 +++---\n refs.h | 4 ++--\n refs/files-backend.c | 6 +++---\n transport.c | 2 +-\n 14 files changed, 22 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 31fb60578..f7a7a971f 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1049,7 +1049,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format,\n \t} else {\n \t\twrite_state_text(state, \"abort-safety\", \"\");\n \t\tif (!state->rebasing)\n-\t\t\tdelete_ref(\"ORIG_HEAD\", NULL, 0);\n+\t\t\tdelete_ref(NULL, \"ORIG_HEAD\", NULL, 0);\n \t}\n \n \t/*\n@@ -2172,7 +2172,7 @@ static void am_abort(struct am_state *state)\n \t\t\t\thas_curr_head ? &curr_head : NULL, 0,\n \t\t\t\tUPDATE_REFS_DIE_ON_ERR);\n \telse if (curr_branch)\n-\t\tdelete_ref(curr_branch, NULL, REF_NODEREF);\n+\t\tdelete_ref(NULL, curr_branch, NULL, REF_NODEREF);\n \n \tfree(curr_branch);\n \tam_destroy(state);\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 9d30f55b0..8f8242e07 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -251,7 +251,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\t\tgoto next;\n \t\t}\n \n-\t\tif (delete_ref(name, is_null_sha1(sha1) ? NULL : sha1,\n+\t\tif (delete_ref(NULL, name, is_null_sha1(sha1) ? NULL : sha1,\n \t\t\t REF_NODEREF)) {\n \t\t\terror(remote_branch\n \t\t\t ? _(\"Error deleting remote-tracking branch '%s'\")\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 5248a9bad..4b492abd4 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -681,9 +681,9 @@ static int merge_abort(struct notes_merge_options *o)\n \t * notes_merge_abort() to remove .git/NOTES_MERGE_WORKTREE.\n \t */\n \n-\tif (delete_ref(\"NOTES_MERGE_PARTIAL\", NULL, 0))\n+\tif (delete_ref(NULL, \"NOTES_MERGE_PARTIAL\", NULL, 0))\n \t\tret += error(_(\"failed to delete ref NOTES_MERGE_PARTIAL\"));\n-\tif (delete_ref(\"NOTES_MERGE_REF\", NULL, REF_NODEREF))\n+\tif (delete_ref(NULL, \"NOTES_MERGE_REF\", NULL, REF_NODEREF))\n \t\tret += error(_(\"failed to delete ref NOTES_MERGE_REF\"));\n \tif (notes_merge_abort(o))\n \t\tret += error(_(\"failed to remove 'git notes merge' worktree\"));\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 5339ed6ad..2b415911b 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -691,7 +691,7 @@ static int mv(int argc, const char **argv)\n \t\tread_ref_full(item->string, RESOLVE_REF_READING, oid.hash, &flag);\n \t\tif (!(flag & REF_ISSYMREF))\n \t\t\tcontinue;\n-\t\tif (delete_ref(item->string, NULL, REF_NODEREF))\n+\t\tif (delete_ref(NULL, item->string, NULL, REF_NODEREF))\n \t\t\tdie(_(\"deleting '%s' failed\"), item->string);\n \t}\n \tfor (i = 0; i < remote_branches.nr; i++) {\n@@ -1248,7 +1248,7 @@ static int set_head(int argc, const char **argv)\n \t\t\thead_name = xstrdup(states.heads.items[0].string);\n \t\tfree_remote_ref_states(&states);\n \t} else if (opt_d && !opt_a && argc == 1) {\n-\t\tif (delete_ref(buf.buf, NULL, REF_NODEREF))\n+\t\tif (delete_ref(NULL, buf.buf, NULL, REF_NODEREF))\n \t\t\tresult |= error(_(\"Could not delete %s\"), buf.buf);\n \t} else\n \t\tusage_with_options(builtin_remote_sethead_usage, options);\ndiff --git a/builtin/replace.c b/builtin/replace.c\nindex b58c714cb..226d0f952 100644\n--- a/builtin/replace.c\n+++ b/builtin/replace.c\n@@ -121,7 +121,7 @@ static int for_each_replace_name(const char **argv, each_replace_name_fn fn)\n static int delete_replace_ref(const char *name, const char *ref,\n \t\t\t const unsigned char *sha1)\n {\n-\tif (delete_ref(ref, sha1, 0))\n+\tif (delete_ref(NULL, ref, sha1, 0))\n \t\treturn 1;\n \tprintf(\"Deleted replace ref '%s'\\n\", name);\n \treturn 0;\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 8ab915bfc..fc3b906c4 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -256,7 +256,7 @@ static int reset_refs(const char *rev, const struct object_id *oid)\n \t\tupdate_ref_oid(msg.buf, \"ORIG_HEAD\", orig, old_orig, 0,\n \t\t\t UPDATE_REFS_MSG_ON_ERR);\n \t} else if (old_orig)\n-\t\tdelete_ref(\"ORIG_HEAD\", old_orig->hash, 0);\n+\t\tdelete_ref(NULL, \"ORIG_HEAD\", old_orig->hash, 0);\n \tset_reflog_message(&msg, \"updating HEAD\", rev);\n \tupdate_ref_status = update_ref_oid(msg.buf, \"HEAD\", oid, orig, 0,\n \t\t\t\t UPDATE_REFS_MSG_ON_ERR);\ndiff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c\nindex 96eed9446..70addef15 100644\n--- a/builtin/symbolic-ref.c\n+++ b/builtin/symbolic-ref.c\n@@ -58,7 +58,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)\n \t\t\tdie(\"Cannot delete %s, not a symbolic ref\", argv[0]);\n \t\tif (!strcmp(argv[0], \"HEAD\"))\n \t\t\tdie(\"deleting '%s' is not allowed\", argv[0]);\n-\t\treturn delete_ref(argv[0], NULL, REF_NODEREF);\n+\t\treturn delete_ref(NULL, argv[0], NULL, REF_NODEREF);\n \t}\n \n \tswitch (argc) {\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex e40c4a967..c23d6d4bc 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -97,7 +97,7 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn,\n static int delete_tag(const char *name, const char *ref,\n \t\t const unsigned char *sha1, const void *cb_data)\n {\n-\tif (delete_ref(ref, sha1, 0))\n+\tif (delete_ref(NULL, ref, sha1, 0))\n \t\treturn 1;\n \tprintf(_(\"Deleted tag '%s' (was %s)\\n\"), name, find_unique_abbrev(sha1, DEFAULT_ABBREV));\n \treturn 0;\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 7f30d3a76..86d006d36 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -433,7 +433,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\t * For purposes of backwards compatibility, we treat\n \t\t * NULL_SHA1 as \"don't care\" here:\n \t\t */\n-\t\treturn delete_ref(refname,\n+\t\treturn delete_ref(NULL, refname,\n \t\t\t\t (oldval && !is_null_sha1(oldsha1)) ? oldsha1 : NULL,\n \t\t\t\t flags);\n \telse\ndiff --git a/fast-import.c b/fast-import.c\nindex 64fe602f0..6c13472c4 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1752,7 +1752,7 @@ static int update_branch(struct branch *b)\n \n \tif (is_null_sha1(b->sha1)) {\n \t\tif (b->delete)\n-\t\t\tdelete_ref(b->name, NULL, 0);\n+\t\t\tdelete_ref(NULL, b->name, NULL, 0);\n \t\treturn 0;\n \t}\n \tif (read_ref(b->name, old_sha1))\ndiff --git a/refs.c b/refs.c\nindex cd36b64ed..053d23a90 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -591,8 +591,8 @@ static int delete_pseudoref(const char *pseudoref, const unsigned char *old_sha1\n \treturn 0;\n }\n \n-int delete_ref(const char *refname, const unsigned char *old_sha1,\n-\t unsigned int flags)\n+int delete_ref(const char *msg, const char *refname,\n+\t const unsigned char *old_sha1, unsigned int flags)\n {\n \tstruct ref_transaction *transaction;\n \tstruct strbuf err = STRBUF_INIT;\n@@ -603,7 +603,7 @@ int delete_ref(const char *refname, const unsigned char *old_sha1,\n \ttransaction = ref_transaction_begin(&err);\n \tif (!transaction ||\n \t ref_transaction_delete(transaction, refname, old_sha1,\n-\t\t\t\t flags, NULL, &err) ||\n+\t\t\t\t flags, msg, &err) ||\n \t ref_transaction_commit(transaction, &err)) {\n \t\terror(\"%s\", err.buf);\n \t\tref_transaction_free(transaction);\ndiff --git a/refs.h b/refs.h\nindex 9fbff90e7..5880886a7 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -276,8 +276,8 @@ int reflog_exists(const char *refname);\n * exists, regardless of its old value. It is an error for old_sha1 to\n * be NULL_SHA1. flags is passed through to ref_transaction_delete().\n */\n-int delete_ref(const char *refname, const unsigned char *old_sha1,\n-\t unsigned int flags);\n+int delete_ref(const char *msg, const char *refname,\n+\t const unsigned char *old_sha1, unsigned int flags);\n \n /*\n * Delete the specified references. If there are any problems, emit\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex c041d4ba2..299eb4d8a 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2489,7 +2489,7 @@ static int files_delete_refs(struct ref_store *ref_store,\n \tfor (i = 0; i < refnames->nr; i++) {\n \t\tconst char *refname = refnames->items[i].string;\n \n-\t\tif (delete_ref(refname, NULL, flags))\n+\t\tif (delete_ref(NULL, refname, NULL, flags))\n \t\t\tresult |= error(_(\"could not remove reference %s\"), refname);\n \t}\n \n@@ -2616,7 +2616,7 @@ static int files_rename_ref(struct ref_store *ref_store,\n \t\treturn error(\"unable to move logfile logs/%s to \"TMP_RENAMED_LOG\": %s\",\n \t\t\toldrefname, strerror(errno));\n \n-\tif (delete_ref(oldrefname, orig_sha1, REF_NODEREF)) {\n+\tif (delete_ref(NULL, oldrefname, orig_sha1, REF_NODEREF)) {\n \t\terror(\"unable to delete old %s\", oldrefname);\n \t\tgoto rollback;\n \t}\n@@ -2630,7 +2630,7 @@ static int files_rename_ref(struct ref_store *ref_store,\n \t */\n \tif (!read_ref_full(newrefname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,\n \t\t\t sha1, NULL) &&\n-\t delete_ref(newrefname, NULL, REF_NODEREF)) {\n+\t delete_ref(NULL, newrefname, NULL, REF_NODEREF)) {\n \t\tif (errno==EISDIR) {\n \t\t\tstruct strbuf path = STRBUF_INIT;\n \t\t\tint result;\ndiff --git a/transport.c b/transport.c\nindex d72e08948..269352b22 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -299,7 +299,7 @@ void transport_update_tracking_ref(struct remote *remote, struct ref *ref, int v\n \t\tif (verbose)\n \t\t\tfprintf(stderr, \"updating local tracking ref '%s'\\n\", rs.dst);\n \t\tif (ref->deletion) {\n-\t\t\tdelete_ref(rs.dst, NULL, 0);\n+\t\t\tdelete_ref(NULL, rs.dst, NULL, 0);\n \t\t} else\n \t\t\tupdate_ref(\"update by push\", rs.dst,\n \t\t\t\t\tref->new_oid.hash, NULL, 0, 0);\n-- \n2.11.1\n\n"},{"id":"312178","messageId":"20170221011035.847-3-kyle@kyleam.com","threadId":"44896","inReplyTo":"20170221011035.847-1-kyle@kyleam.com","subject":"[PATCH v2 2/4] update-ref: pass reflog message to delete_ref()","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-21T01:10:33Z","receivedAt":"2017-02-21T01:10:58Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Now that delete_ref() accepts a reflog message, pass the user-provided\nmessage to delete_ref() rather than silently dropping it.\n\nSigned-off-by: Kyle Meyer \n---\n builtin/update-ref.c | 2 +-\n t/t1400-update-ref.sh | 18 ++++++++++++++++++\n 2 files changed, 19 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 86d006d36..0b2ecf41a 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -433,7 +433,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)\n \t\t * For purposes of backwards compatibility, we treat\n \t\t * NULL_SHA1 as \"don't care\" here:\n \t\t */\n-\t\treturn delete_ref(NULL, refname,\n+\t\treturn delete_ref(msg, refname,\n \t\t\t\t (oldval && !is_null_sha1(oldsha1)) ? oldsha1 : NULL,\n \t\t\t\t flags);\n \telse\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex b0ffc0b57..6e112fb5f 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -85,6 +85,24 @@ test_expect_success \"delete $m (by HEAD)\" '\n '\n rm -f .git/$m\n \n+test_expect_success \"deleting current branch adds message to HEAD's log\" '\n+\tgit update-ref $m $A &&\n+\tgit symbolic-ref HEAD $m &&\n+\tgit update-ref -m delete-$m -d $m &&\n+\t! test -f .git/$m &&\n+\tgrep \"delete-$m$\" .git/logs/HEAD\n+'\n+rm -f .git/$m\n+\n+test_expect_success \"deleting by HEAD adds message to HEAD's log\" '\n+\tgit update-ref $m $A &&\n+\tgit symbolic-ref HEAD $m &&\n+\tgit update-ref -m delete-by-head -d HEAD &&\n+\t! test -f .git/$m &&\n+\tgrep \"delete-by-head$\" .git/logs/HEAD\n+'\n+rm -f .git/$m\n+\n test_expect_success 'update-ref does not create reflogs by default' '\n \ttest_when_finished \"git update-ref -d $outside\" &&\n \tgit update-ref $outside $A &&\n-- \n2.11.1\n\n"},{"id":"312179","messageId":"20170221011035.847-5-kyle@kyleam.com","threadId":"44896","inReplyTo":"20170221011035.847-1-kyle@kyleam.com","subject":"[PATCH v2 4/4] branch: record creation of renamed branch in HEAD's log","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-21T01:10:35Z","receivedAt":"2017-02-21T01:11:01Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Renaming the current branch adds an event to the current branch's log\nand to HEAD's log. However, the logged entries differ. The entry in\nthe branch's log represents the entire renaming operation (the old and\nnew hash are identical), whereas the entry in HEAD's log represents\nthe deletion only (the new sha1 is null).\n\nExtend replace_each_worktree_head_symref(), whose only caller is\nbranch_rename(), to take a reflog message argument. This allows the\ncreation of the new ref to be recorded in HEAD's log. As a result,\nthe renaming event is represented by two entries (a deletion and a\ncreation entry) in HEAD's log.\n\nIt's a bit unfortunate that the branch's log and HEAD's log now\nrepresent the renaming event in different ways. Given that the\nrenaming operation is not atomic, the two-entry form is a more\naccurate representation of the operation and is more useful for\ndebugging purposes if a failure occurs between the deletion and\ncreation events. It would make sense to move the branch's log to the\ntwo-entry form, but this would involve changes to how the rename is\ncarried out and to how the update flags and reflogs are processed for\ndeletions, so it may not be worth the effort.\n\nBased-on-patch-by: Jeff King \nSigned-off-by: Kyle Meyer \n---\n branch.c | 5 +++--\n branch.h | 3 ++-\n builtin/branch.c | 5 +++--\n refs.h | 3 ++-\n refs/files-backend.c | 4 ++--\n t/t3200-branch.sh | 5 +++--\n 6 files changed, 15 insertions(+), 10 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex b955d4f31..5c12036b0 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -345,7 +345,8 @@ void die_if_checked_out(const char *branch, int ignore_current_worktree)\n \t branch, wt->path);\n }\n \n-int replace_each_worktree_head_symref(const char *oldref, const char *newref)\n+int replace_each_worktree_head_symref(const char *oldref, const char *newref,\n+\t\t\t\t const char *logmsg)\n {\n \tint ret = 0;\n \tstruct worktree **worktrees = get_worktrees(0);\n@@ -358,7 +359,7 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref)\n \t\t\tcontinue;\n \n \t\tif (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]),\n-\t\t\t\t\t newref)) {\n+\t\t\t\t\t newref, logmsg)) {\n \t\t\tret = -1;\n \t\t\terror(_(\"HEAD of working tree %s is not updated\"),\n \t\t\t worktrees[i]->path);\ndiff --git a/branch.h b/branch.h\nindex 3103eb9ad..b07788558 100644\n--- a/branch.h\n+++ b/branch.h\n@@ -71,6 +71,7 @@ extern void die_if_checked_out(const char *branch, int ignore_current_worktree);\n * This will be used when renaming a branch. Returns 0 if successful, non-zero\n * otherwise.\n */\n-extern int replace_each_worktree_head_symref(const char *oldref, const char *newref);\n+extern int replace_each_worktree_head_symref(const char *oldref, const char *newref,\n+\t\t\t\t\t const char *logmsg);\n \n #endif\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 8f8242e07..e1f97dcfc 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -579,14 +579,15 @@ static void rename_branch(const char *oldname, const char *newname, int force)\n \n \tif (rename_ref(oldref.buf, newref.buf, logmsg.buf))\n \t\tdie(_(\"Branch rename failed\"));\n-\tstrbuf_release(&logmsg);\n \n \tif (recovery)\n \t\twarning(_(\"Renamed a misnamed branch '%s' away\"), oldref.buf + 11);\n \n-\tif (replace_each_worktree_head_symref(oldref.buf, newref.buf))\n+\tif (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf))\n \t\tdie(_(\"Branch renamed to %s, but HEAD is not updated!\"), newname);\n \n+\tstrbuf_release(&logmsg);\n+\n \tstrbuf_addf(&oldsection, \"branch.%s\", oldref.buf + 11);\n \tstrbuf_release(&oldref);\n \tstrbuf_addf(&newsection, \"branch.%s\", newref.buf + 11);\ndiff --git a/refs.h b/refs.h\nindex 5880886a7..e529f4c3a 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -334,7 +334,8 @@ int create_symref(const char *refname, const char *target, const char *logmsg);\n * $GIT_DIR points to.\n * Return 0 if successful, non-zero otherwise.\n * */\n-int set_worktree_head_symref(const char *gitdir, const char *target);\n+int set_worktree_head_symref(const char *gitdir, const char *target,\n+\t\t\t const char *logmsg);\n \n enum action_on_err {\n \tUPDATE_REFS_MSG_ON_ERR,\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex f6e7c192c..42b137bb1 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3055,7 +3055,7 @@ static int files_create_symref(struct ref_store *ref_store,\n \treturn ret;\n }\n \n-int set_worktree_head_symref(const char *gitdir, const char *target)\n+int set_worktree_head_symref(const char *gitdir, const char *target, const char *logmsg)\n {\n \tstatic struct lock_file head_lock;\n \tstruct ref_lock *lock;\n@@ -3083,7 +3083,7 @@ int set_worktree_head_symref(const char *gitdir, const char *target)\n \tlock->lk = &head_lock;\n \tlock->ref_name = xstrdup(head_rel);\n \n-\tret = create_symref_locked(lock, head_rel, target, NULL);\n+\tret = create_symref_locked(lock, head_rel, target, logmsg);\n \n \tunlock_ref(lock); /* will free lock */\n \tstrbuf_release(&head_path);\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 0dbc54003..d84837d08 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -139,9 +139,10 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou\n \ttest $(git rev-parse --abbrev-ref HEAD) = bam\n '\n \n-test_expect_success 'git branch -M baz bam should add entry to .git/logs/HEAD' '\n+test_expect_success 'git branch -M baz bam should add entries to .git/logs/HEAD' '\n msg=\"Branch: renamed refs/heads/baz to refs/heads/bam\" &&\n-\tgrep \" 0\\{40\\}.*$msg$\" .git/logs/HEAD\n+\tgrep \" 0\\{40\\}.*$msg$\" .git/logs/HEAD &&\n+\tgrep \"^0\\{40\\}.*$msg$\" .git/logs/HEAD\n '\n \n test_expect_success 'git branch -M baz bam should succeed when baz is checked out as linked working tree' '\n-- \n2.11.1\n\n"},{"id":"312180","messageId":"20170221011035.847-4-kyle@kyleam.com","threadId":"44896","inReplyTo":"20170221011035.847-1-kyle@kyleam.com","subject":"[PATCH v2 3/4] rename_ref: replace empty message in HEAD's log","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-21T01:10:34Z","receivedAt":"2017-02-21T01:11:03Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"When the current branch is renamed, the deletion of the old ref is\nrecorded in HEAD's log with an empty message. Now that delete_ref()\naccepts a reflog message, provide a more descriptive message by\npassing along the log message that is given to rename_ref().\n\nThe next step will be to extend HEAD's log to also include the second\npart of the rename, the creation of the new branch.\n\nHelped-by: Jeff King \nSigned-off-by: Kyle Meyer \n---\n refs/files-backend.c | 2 +-\n t/t3200-branch.sh | 5 +++++\n 2 files changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 299eb4d8a..f6e7c192c 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -2616,7 +2616,7 @@ static int files_rename_ref(struct ref_store *ref_store,\n \t\treturn error(\"unable to move logfile logs/%s to \"TMP_RENAMED_LOG\": %s\",\n \t\t\toldrefname, strerror(errno));\n \n-\tif (delete_ref(NULL, oldrefname, orig_sha1, REF_NODEREF)) {\n+\tif (delete_ref(logmsg, oldrefname, orig_sha1, REF_NODEREF)) {\n \t\terror(\"unable to delete old %s\", oldrefname);\n \t\tgoto rollback;\n \t}\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 8a833f354..0dbc54003 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -139,6 +139,11 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou\n \ttest $(git rev-parse --abbrev-ref HEAD) = bam\n '\n \n+test_expect_success 'git branch -M baz bam should add entry to .git/logs/HEAD' '\n+ msg=\"Branch: renamed refs/heads/baz to refs/heads/bam\" &&\n+\tgrep \" 0\\{40\\}.*$msg$\" .git/logs/HEAD\n+'\n+\n test_expect_success 'git branch -M baz bam should succeed when baz is checked out as linked working tree' '\n \tgit checkout master &&\n \tgit worktree add -b baz bazdir &&\n-- \n2.11.1\n\n"},{"id":"312192","messageId":"20170221071209.2n4jer4wuyn23ci3@sigill.intra.peff.net","threadId":"44896","inReplyTo":"20170221011035.847-1-kyle@kyleam.com","subject":"Re: [PATCH v2 0/4] delete_ref: support reflog messages","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-02-21T07:12:09Z","receivedAt":"2017-02-21T07:12:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 20, 2017 at 08:10:31PM -0500, Kyle Meyer wrote:\n\n> Kyle Meyer (4):\n> delete_ref: accept a reflog message argument\n> update-ref: pass reflog message to delete_ref()\n> rename_ref: replace empty message in HEAD's log\n> branch: record creation of renamed branch in HEAD's log\n\nThese look good to me. I think you did a nice job of summarizing the\ndiscussion in the commit messages.\n\n-Peff\n"},{"id":"312193","messageId":"xmqqr32sggfo.fsf@gitster.mtv.corp.google.com","threadId":"44896","inReplyTo":"20170221011035.847-1-kyle@kyleam.com","subject":"Re: [PATCH v2 0/4] delete_ref: support reflog messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-21T07:18:35Z","receivedAt":"2017-02-21T07:18:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kyle Meyer writes:\n\n> Kyle Meyer (4):\n> delete_ref: accept a reflog message argument\n> update-ref: pass reflog message to delete_ref()\n> rename_ref: replace empty message in HEAD's log\n> branch: record creation of renamed branch in HEAD's log\n\nThese looked reasonable. I had to resolve conflicts with another\ntopic in flight that removed set_worktree_head_symref() function\nwhile queuing, and I think I resolved it correctly, but please\ndouble check what is pushed out on 'pu'.\n\nThanks.\n"},{"id":"312204","messageId":"87d1eb4hnc.fsf@kyleam.com","threadId":"44896","inReplyTo":"xmqqr32sggfo.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/4] delete_ref: support reflog messages","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-21T16:45:27Z","receivedAt":"2017-02-21T16:45:40Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Junio C Hamano writes:\n\n> These looked reasonable. I had to resolve conflicts with another\n> topic in flight that removed set_worktree_head_symref() function\n> while queuing, and I think I resolved it correctly, but please\n> double check what is pushed out on 'pu'.\n\nThe merge looks right to me, thanks. Thanks also for touching up the\ntab/space issue in t3200-branch.sh.\n\n-- \nKyle\n"},{"id":"312387","messageId":"CACsJy8BJgL1i85nZ9CpAXCKaG+PQQkjRKLFPWzbsMF8WN8TEjQ@mail.gmail.com","threadId":"44896","inReplyTo":"20170221011035.847-2-kyle@kyleam.com","subject":"Re: [PATCH v2 1/4] delete_ref: accept a reflog message argument","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2017-02-23T09:33:25Z","receivedAt":"2017-02-23T09:34:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Feb 21, 2017 at 8:10 AM, Kyle Meyer wrote:\n> diff --git a/refs.h b/refs.h\n> index 9fbff90e7..5880886a7 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -276,8 +276,8 @@ int reflog_exists(const char *refname);\n> * exists, regardless of its old value. It is an error for old_sha1 to\n> * be NULL_SHA1. flags is passed through to ref_transaction_delete().\n> */\n> -int delete_ref(const char *refname, const unsigned char *old_sha1,\n> - unsigned int flags);\n> +int delete_ref(const char *msg, const char *refname,\n> + const unsigned char *old_sha1, unsigned int flags);\n\nIs it just me who thinks it's weird that msg comes in front here? The\nkey identity, refname, should be the first argument imo. You'll\nprobably want to update the comment block above if msg can be NULL. We\nhave _very_ good documentation in this file, let's keep it uptodate.\n-- \nDuy\n"},{"id":"312445","messageId":"xmqq37f47d7k.fsf@gitster.mtv.corp.google.com","threadId":"44896","inReplyTo":"CACsJy8BJgL1i85nZ9CpAXCKaG+PQQkjRKLFPWzbsMF8WN8TEjQ@mail.gmail.com","subject":"Re: [PATCH v2 1/4] delete_ref: accept a reflog message argument","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-23T22:29:51Z","receivedAt":"2017-02-23T22:30:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Tue, Feb 21, 2017 at 8:10 AM, Kyle Meyer wrote:\n>> diff --git a/refs.h b/refs.h\n>> index 9fbff90e7..5880886a7 100644\n>> --- a/refs.h\n>> +++ b/refs.h\n>> @@ -276,8 +276,8 @@ int reflog_exists(const char *refname);\n>> * exists, regardless of its old value. It is an error for old_sha1 to\n>> * be NULL_SHA1. flags is passed through to ref_transaction_delete().\n>> */\n>> -int delete_ref(const char *refname, const unsigned char *old_sha1,\n>> - unsigned int flags);\n>> +int delete_ref(const char *msg, const char *refname,\n>> + const unsigned char *old_sha1, unsigned int flags);\n>\n> Is it just me who thinks it's weird that msg comes in front here?\n\nYou and anybody who didn't read what was discussed earlier, methinks\n;-) cf. <20170217081205.zn7j6d5cffgdvfbn@sigill.intra.peff.net>\n\n> ... You'll\n> probably want to update the comment block above if msg can be\n> NULL.\n\nGood suggestion.\n\nThanks for taking a look at this topic. IIRC a recent update to one\nof your topics introduced a new conflicts with this one.\n\n"},{"id":"312610","messageId":"87poi7c6n2.fsf@kyleam.com","threadId":"44896","inReplyTo":"CACsJy8BJgL1i85nZ9CpAXCKaG+PQQkjRKLFPWzbsMF8WN8TEjQ@mail.gmail.com","subject":"Re: [PATCH v2 1/4] delete_ref: accept a reflog message argument","fromName":"Kyle Meyer","fromEmail":"kyle@kyleam.com","sentAt":"2017-02-25T03:05:05Z","receivedAt":"2017-02-25T03:06:26Z","isPatch":true,"sender":{"key":"kyle@kyleam.com","avatar":"https://avatars.githubusercontent.com/u/1297788?v=4"},"body":"Duy Nguyen writes:\n\n> You'll probably want to update the comment block above if msg can be\n> NULL. We have _very_ good documentation in this file, let's keep it\n> uptodate.\n\nLooking at how other functions in refs.h document their \"msg\" or\n\"logmsg\" parameter, none seem to mention it explicitly. update_ref\nrefers to ref_transaction_update, and\nref_transaction_{update,create,delete,verify} refer to ref.h's\n\"Reference transaction updates\" comment.\n\ndelete_ref's docstring already mentions that \"flag\" is passed to\nref_transaction_delete, so perhaps it should mention \"msg\" here as\nwell.\n\n-- >8 --\nSubject: [PATCH] delete_ref: mention \"msg\" parameter in docstring\n\ndelete_ref() was recently extended to take a \"msg\" argument.\n---\n refs.h | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.h b/refs.h\nindex e529f4c3a..988750218 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -274,7 +274,8 @@ int reflog_exists(const char *refname);\n * verify that the current value of the reference is old_sha1 before\n * deleting it. If old_sha1 is NULL, delete the reference if it\n * exists, regardless of its old value. It is an error for old_sha1 to\n- * be NULL_SHA1. flags is passed through to ref_transaction_delete().\n+ * be NULL_SHA1. msg and flags are passed through to\n+ * ref_transaction_delete().\n */\n int delete_ref(const char *msg, const char *refname,\n \t const unsigned char *old_sha1, unsigned int flags);\n-- \n2.11.1\n\n"}]}