From 755b49ae965c9d17d91fbe7902050428f366bf69 Mon Sep 17 00:00:00 2001 From: Kyle Meyer Date: Mon, 20 Feb 2017 20:10:32 -0500 Subject: delete_ref: accept a reflog message argument When the current branch is renamed with 'git branch -m/-M' or deleted with 'git update-ref -m -d', the event is recorded in HEAD's log with an empty message. In preparation for adding a more meaningful message to HEAD's log in these cases, update delete_ref() to take a message argument and pass it along to ref_transaction_delete(). Modify all callers to pass NULL for the new message argument; no change in behavior is intended. Note that this is relevant for HEAD's log but not for the deleted ref's log, which is currently deleted along with the ref. Even if it were not, an entry for the deletion wouldn't be present in the deleted ref's log. files_transaction_commit() writes to the log if REF_NEEDS_COMMIT or REF_LOG_ONLY are set, but lock_ref_for_update() doesn't set REF_NEEDS_COMMIT for the deleted ref because REF_DELETING is set. In contrast, the update for HEAD has REF_LOG_ONLY set by split_head_update(), resulting in the deletion being logged. Signed-off-by: Kyle Meyer Signed-off-by: Junio C Hamano diff --git a/builtin/am.c b/builtin/am.c index 31fb605..f7a7a97 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -1049,7 +1049,7 @@ static void am_setup(struct am_state *state, enum patch_format patch_format, } else { write_state_text(state, "abort-safety", ""); if (!state->rebasing) - delete_ref("ORIG_HEAD", NULL, 0); + delete_ref(NULL, "ORIG_HEAD", NULL, 0); } /* @@ -2172,7 +2172,7 @@ static void am_abort(struct am_state *state) has_curr_head ? &curr_head : NULL, 0, UPDATE_REFS_DIE_ON_ERR); else if (curr_branch) - delete_ref(curr_branch, NULL, REF_NODEREF); + delete_ref(NULL, curr_branch, NULL, REF_NODEREF); free(curr_branch); am_destroy(state); diff --git a/builtin/branch.c b/builtin/branch.c index 9d30f55..8f8242e 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -251,7 +251,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds, goto next; } - if (delete_ref(name, is_null_sha1(sha1) ? NULL : sha1, + if (delete_ref(NULL, name, is_null_sha1(sha1) ? NULL : sha1, REF_NODEREF)) { error(remote_branch ? _("Error deleting remote-tracking branch '%s'") diff --git a/builtin/notes.c b/builtin/notes.c index 5248a9b..4b492ab 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -681,9 +681,9 @@ static int merge_abort(struct notes_merge_options *o) * notes_merge_abort() to remove .git/NOTES_MERGE_WORKTREE. */ - if (delete_ref("NOTES_MERGE_PARTIAL", NULL, 0)) + if (delete_ref(NULL, "NOTES_MERGE_PARTIAL", NULL, 0)) ret += error(_("failed to delete ref NOTES_MERGE_PARTIAL")); - if (delete_ref("NOTES_MERGE_REF", NULL, REF_NODEREF)) + if (delete_ref(NULL, "NOTES_MERGE_REF", NULL, REF_NODEREF)) ret += error(_("failed to delete ref NOTES_MERGE_REF")); if (notes_merge_abort(o)) ret += error(_("failed to remove 'git notes merge' worktree")); diff --git a/builtin/remote.c b/builtin/remote.c index 5339ed6..2b41591 100644 --- a/builtin/remote.c +++ b/builtin/remote.c @@ -691,7 +691,7 @@ static int mv(int argc, const char **argv) read_ref_full(item->string, RESOLVE_REF_READING, oid.hash, &flag); if (!(flag & REF_ISSYMREF)) continue; - if (delete_ref(item->string, NULL, REF_NODEREF)) + if (delete_ref(NULL, item->string, NULL, REF_NODEREF)) die(_("deleting '%s' failed"), item->string); } for (i = 0; i < remote_branches.nr; i++) { @@ -1248,7 +1248,7 @@ static int set_head(int argc, const char **argv) head_name = xstrdup(states.heads.items[0].string); free_remote_ref_states(&states); } else if (opt_d && !opt_a && argc == 1) { - if (delete_ref(buf.buf, NULL, REF_NODEREF)) + if (delete_ref(NULL, buf.buf, NULL, REF_NODEREF)) result |= error(_("Could not delete %s"), buf.buf); } else usage_with_options(builtin_remote_sethead_usage, options); diff --git a/builtin/replace.c b/builtin/replace.c index b58c714..226d0f9 100644 --- a/builtin/replace.c +++ b/builtin/replace.c @@ -121,7 +121,7 @@ static int for_each_replace_name(const char **argv, each_replace_name_fn fn) static int delete_replace_ref(const char *name, const char *ref, const unsigned char *sha1) { - if (delete_ref(ref, sha1, 0)) + if (delete_ref(NULL, ref, sha1, 0)) return 1; printf("Deleted replace ref '%s'\n", name); return 0; diff --git a/builtin/reset.c b/builtin/reset.c index 8ab915b..fc3b906 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -256,7 +256,7 @@ static int reset_refs(const char *rev, const struct object_id *oid) update_ref_oid(msg.buf, "ORIG_HEAD", orig, old_orig, 0, UPDATE_REFS_MSG_ON_ERR); } else if (old_orig) - delete_ref("ORIG_HEAD", old_orig->hash, 0); + delete_ref(NULL, "ORIG_HEAD", old_orig->hash, 0); set_reflog_message(&msg, "updating HEAD", rev); update_ref_status = update_ref_oid(msg.buf, "HEAD", oid, orig, 0, UPDATE_REFS_MSG_ON_ERR); diff --git a/builtin/symbolic-ref.c b/builtin/symbolic-ref.c index 96eed94..70addef 100644 --- a/builtin/symbolic-ref.c +++ b/builtin/symbolic-ref.c @@ -58,7 +58,7 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix) die("Cannot delete %s, not a symbolic ref", argv[0]); if (!strcmp(argv[0], "HEAD")) die("deleting '%s' is not allowed", argv[0]); - return delete_ref(argv[0], NULL, REF_NODEREF); + return delete_ref(NULL, argv[0], NULL, REF_NODEREF); } switch (argc) { diff --git a/builtin/tag.c b/builtin/tag.c index e40c4a9..c23d6d4 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -97,7 +97,7 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn, static int delete_tag(const char *name, const char *ref, const unsigned char *sha1, const void *cb_data) { - if (delete_ref(ref, sha1, 0)) + if (delete_ref(NULL, ref, sha1, 0)) return 1; printf(_("Deleted tag '%s' (was %s)\n"), name, find_unique_abbrev(sha1, DEFAULT_ABBREV)); return 0; diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 7f30d3a..86d006d 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -433,7 +433,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix) * For purposes of backwards compatibility, we treat * NULL_SHA1 as "don't care" here: */ - return delete_ref(refname, + return delete_ref(NULL, refname, (oldval && !is_null_sha1(oldsha1)) ? oldsha1 : NULL, flags); else diff --git a/fast-import.c b/fast-import.c index 64fe602..6c13472 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1752,7 +1752,7 @@ static int update_branch(struct branch *b) if (is_null_sha1(b->sha1)) { if (b->delete) - delete_ref(b->name, NULL, 0); + delete_ref(NULL, b->name, NULL, 0); return 0; } if (read_ref(b->name, old_sha1)) diff --git a/refs.c b/refs.c index cd36b64..053d23a 100644 --- a/refs.c +++ b/refs.c @@ -591,8 +591,8 @@ static int delete_pseudoref(const char *pseudoref, const unsigned char *old_sha1 return 0; } -int delete_ref(const char *refname, const unsigned char *old_sha1, - unsigned int flags) +int delete_ref(const char *msg, const char *refname, + const unsigned char *old_sha1, unsigned int flags) { struct ref_transaction *transaction; struct strbuf err = STRBUF_INIT; @@ -603,7 +603,7 @@ int delete_ref(const char *refname, const unsigned char *old_sha1, transaction = ref_transaction_begin(&err); if (!transaction || ref_transaction_delete(transaction, refname, old_sha1, - flags, NULL, &err) || + flags, msg, &err) || ref_transaction_commit(transaction, &err)) { error("%s", err.buf); ref_transaction_free(transaction); diff --git a/refs.h b/refs.h index 9fbff90..5880886 100644 --- a/refs.h +++ b/refs.h @@ -276,8 +276,8 @@ int reflog_exists(const char *refname); * exists, regardless of its old value. It is an error for old_sha1 to * be NULL_SHA1. flags is passed through to ref_transaction_delete(). */ -int delete_ref(const char *refname, const unsigned char *old_sha1, - unsigned int flags); +int delete_ref(const char *msg, const char *refname, + const unsigned char *old_sha1, unsigned int flags); /* * Delete the specified references. If there are any problems, emit diff --git a/refs/files-backend.c b/refs/files-backend.c index c041d4b..299eb4d 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2489,7 +2489,7 @@ static int files_delete_refs(struct ref_store *ref_store, for (i = 0; i < refnames->nr; i++) { const char *refname = refnames->items[i].string; - if (delete_ref(refname, NULL, flags)) + if (delete_ref(NULL, refname, NULL, flags)) result |= error(_("could not remove reference %s"), refname); } @@ -2616,7 +2616,7 @@ static int files_rename_ref(struct ref_store *ref_store, return error("unable to move logfile logs/%s to "TMP_RENAMED_LOG": %s", oldrefname, strerror(errno)); - if (delete_ref(oldrefname, orig_sha1, REF_NODEREF)) { + if (delete_ref(NULL, oldrefname, orig_sha1, REF_NODEREF)) { error("unable to delete old %s", oldrefname); goto rollback; } @@ -2630,7 +2630,7 @@ static int files_rename_ref(struct ref_store *ref_store, */ if (!read_ref_full(newrefname, RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE, sha1, NULL) && - delete_ref(newrefname, NULL, REF_NODEREF)) { + delete_ref(NULL, newrefname, NULL, REF_NODEREF)) { if (errno==EISDIR) { struct strbuf path = STRBUF_INIT; int result; diff --git a/transport.c b/transport.c index d72e089..269352b 100644 --- a/transport.c +++ b/transport.c @@ -299,7 +299,7 @@ void transport_update_tracking_ref(struct remote *remote, struct ref *ref, int v if (verbose) fprintf(stderr, "updating local tracking ref '%s'\n", rs.dst); if (ref->deletion) { - delete_ref(rs.dst, NULL, 0); + delete_ref(NULL, rs.dst, NULL, 0); } else update_ref("update by push", rs.dst, ref->new_oid.hash, NULL, 0, 0); -- cgit v0.10.2-6-g49f6 From de922669ab0508109cdb5672e5515f7524e3ccf8 Mon Sep 17 00:00:00 2001 From: Kyle Meyer Date: Mon, 20 Feb 2017 20:10:33 -0500 Subject: update-ref: pass reflog message to delete_ref() Now that delete_ref() accepts a reflog message, pass the user-provided message to delete_ref() rather than silently dropping it. Signed-off-by: Kyle Meyer Signed-off-by: Junio C Hamano diff --git a/builtin/update-ref.c b/builtin/update-ref.c index 86d006d..0b2ecf4 100644 --- a/builtin/update-ref.c +++ b/builtin/update-ref.c @@ -433,7 +433,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix) * For purposes of backwards compatibility, we treat * NULL_SHA1 as "don't care" here: */ - return delete_ref(NULL, refname, + return delete_ref(msg, refname, (oldval && !is_null_sha1(oldsha1)) ? oldsha1 : NULL, flags); else diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index b0ffc0b..6e112fb 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -85,6 +85,24 @@ test_expect_success "delete $m (by HEAD)" ' ' rm -f .git/$m +test_expect_success "deleting current branch adds message to HEAD's log" ' + git update-ref $m $A && + git symbolic-ref HEAD $m && + git update-ref -m delete-$m -d $m && + ! test -f .git/$m && + grep "delete-$m$" .git/logs/HEAD +' +rm -f .git/$m + +test_expect_success "deleting by HEAD adds message to HEAD's log" ' + git update-ref $m $A && + git symbolic-ref HEAD $m && + git update-ref -m delete-by-head -d HEAD && + ! test -f .git/$m && + grep "delete-by-head$" .git/logs/HEAD +' +rm -f .git/$m + test_expect_success 'update-ref does not create reflogs by default' ' test_when_finished "git update-ref -d $outside" && git update-ref $outside $A && -- cgit v0.10.2-6-g49f6 From 893dbf5ba16c47c7284209bc6c527195f368ee35 Mon Sep 17 00:00:00 2001 From: Kyle Meyer Date: Mon, 20 Feb 2017 20:10:34 -0500 Subject: rename_ref: replace empty message in HEAD's log When the current branch is renamed, the deletion of the old ref is recorded in HEAD's log with an empty message. Now that delete_ref() accepts a reflog message, provide a more descriptive message by passing along the log message that is given to rename_ref(). The next step will be to extend HEAD's log to also include the second part of the rename, the creation of the new branch. Helped-by: Jeff King Signed-off-by: Kyle Meyer Signed-off-by: Junio C Hamano diff --git a/refs/files-backend.c b/refs/files-backend.c index 299eb4d..f6e7c19 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2616,7 +2616,7 @@ static int files_rename_ref(struct ref_store *ref_store, return error("unable to move logfile logs/%s to "TMP_RENAMED_LOG": %s", oldrefname, strerror(errno)); - if (delete_ref(NULL, oldrefname, orig_sha1, REF_NODEREF)) { + if (delete_ref(logmsg, oldrefname, orig_sha1, REF_NODEREF)) { error("unable to delete old %s", oldrefname); goto rollback; } diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index 8a833f3..47bc151 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -139,6 +139,11 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou test $(git rev-parse --abbrev-ref HEAD) = bam ' +test_expect_success 'git branch -M baz bam should add entry to .git/logs/HEAD' ' + msg="Branch: renamed refs/heads/baz to refs/heads/bam" && + grep " 0\{40\}.*$msg$" .git/logs/HEAD +' + test_expect_success 'git branch -M baz bam should succeed when baz is checked out as linked working tree' ' git checkout master && git worktree add -b baz bazdir && -- cgit v0.10.2-6-g49f6 From 39ee4c6c2fc80960094ae1454922c2d10c72f210 Mon Sep 17 00:00:00 2001 From: Kyle Meyer Date: Mon, 20 Feb 2017 20:10:35 -0500 Subject: branch: record creation of renamed branch in HEAD's log Renaming the current branch adds an event to the current branch's log and to HEAD's log. However, the logged entries differ. The entry in the branch's log represents the entire renaming operation (the old and new hash are identical), whereas the entry in HEAD's log represents the deletion only (the new sha1 is null). Extend replace_each_worktree_head_symref(), whose only caller is branch_rename(), to take a reflog message argument. This allows the creation of the new ref to be recorded in HEAD's log. As a result, the renaming event is represented by two entries (a deletion and a creation entry) in HEAD's log. It's a bit unfortunate that the branch's log and HEAD's log now represent the renaming event in different ways. Given that the renaming operation is not atomic, the two-entry form is a more accurate representation of the operation and is more useful for debugging purposes if a failure occurs between the deletion and creation events. It would make sense to move the branch's log to the two-entry form, but this would involve changes to how the rename is carried out and to how the update flags and reflogs are processed for deletions, so it may not be worth the effort. Based-on-patch-by: Jeff King Signed-off-by: Kyle Meyer Signed-off-by: Junio C Hamano diff --git a/branch.c b/branch.c index b955d4f..5c12036 100644 --- a/branch.c +++ b/branch.c @@ -345,7 +345,8 @@ void die_if_checked_out(const char *branch, int ignore_current_worktree) branch, wt->path); } -int replace_each_worktree_head_symref(const char *oldref, const char *newref) +int replace_each_worktree_head_symref(const char *oldref, const char *newref, + const char *logmsg) { int ret = 0; struct worktree **worktrees = get_worktrees(0); @@ -358,7 +359,7 @@ int replace_each_worktree_head_symref(const char *oldref, const char *newref) continue; if (set_worktree_head_symref(get_worktree_git_dir(worktrees[i]), - newref)) { + newref, logmsg)) { ret = -1; error(_("HEAD of working tree %s is not updated"), worktrees[i]->path); diff --git a/branch.h b/branch.h index 3103eb9..b077885 100644 --- a/branch.h +++ b/branch.h @@ -71,6 +71,7 @@ extern void die_if_checked_out(const char *branch, int ignore_current_worktree); * This will be used when renaming a branch. Returns 0 if successful, non-zero * otherwise. */ -extern int replace_each_worktree_head_symref(const char *oldref, const char *newref); +extern int replace_each_worktree_head_symref(const char *oldref, const char *newref, + const char *logmsg); #endif diff --git a/builtin/branch.c b/builtin/branch.c index 8f8242e..e1f97dc 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -579,14 +579,15 @@ static void rename_branch(const char *oldname, const char *newname, int force) if (rename_ref(oldref.buf, newref.buf, logmsg.buf)) die(_("Branch rename failed")); - strbuf_release(&logmsg); if (recovery) warning(_("Renamed a misnamed branch '%s' away"), oldref.buf + 11); - if (replace_each_worktree_head_symref(oldref.buf, newref.buf)) + if (replace_each_worktree_head_symref(oldref.buf, newref.buf, logmsg.buf)) die(_("Branch renamed to %s, but HEAD is not updated!"), newname); + strbuf_release(&logmsg); + strbuf_addf(&oldsection, "branch.%s", oldref.buf + 11); strbuf_release(&oldref); strbuf_addf(&newsection, "branch.%s", newref.buf + 11); diff --git a/refs.h b/refs.h index 5880886..e529f4c 100644 --- a/refs.h +++ b/refs.h @@ -334,7 +334,8 @@ int create_symref(const char *refname, const char *target, const char *logmsg); * $GIT_DIR points to. * Return 0 if successful, non-zero otherwise. * */ -int set_worktree_head_symref(const char *gitdir, const char *target); +int set_worktree_head_symref(const char *gitdir, const char *target, + const char *logmsg); enum action_on_err { UPDATE_REFS_MSG_ON_ERR, diff --git a/refs/files-backend.c b/refs/files-backend.c index f6e7c19..42b137b 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -3055,7 +3055,7 @@ static int files_create_symref(struct ref_store *ref_store, return ret; } -int set_worktree_head_symref(const char *gitdir, const char *target) +int set_worktree_head_symref(const char *gitdir, const char *target, const char *logmsg) { static struct lock_file head_lock; struct ref_lock *lock; @@ -3083,7 +3083,7 @@ int set_worktree_head_symref(const char *gitdir, const char *target) lock->lk = &head_lock; lock->ref_name = xstrdup(head_rel); - ret = create_symref_locked(lock, head_rel, target, NULL); + ret = create_symref_locked(lock, head_rel, target, logmsg); unlock_ref(lock); /* will free lock */ strbuf_release(&head_path); diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh index 47bc151..e36ed3b 100755 --- a/t/t3200-branch.sh +++ b/t/t3200-branch.sh @@ -139,9 +139,10 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou test $(git rev-parse --abbrev-ref HEAD) = bam ' -test_expect_success 'git branch -M baz bam should add entry to .git/logs/HEAD' ' +test_expect_success 'git branch -M baz bam should add entries to .git/logs/HEAD' ' msg="Branch: renamed refs/heads/baz to refs/heads/bam" && - grep " 0\{40\}.*$msg$" .git/logs/HEAD + grep " 0\{40\}.*$msg$" .git/logs/HEAD && + grep "^0\{40\}.*$msg$" .git/logs/HEAD ' test_expect_success 'git branch -M baz bam should succeed when baz is checked out as linked working tree' ' -- cgit v0.10.2-6-g49f6