git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Kyle Meyer <kyle@kyleam.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log
Date: Fri, 17 Feb 2017 18:53:25 -0500	[thread overview]
Message-ID: <20170217235324.n3jabck23bvd4cs2@sigill.intra.peff.net> (raw)
In-Reply-To: <871suwqtar.fsf@kyleam.com>

On Fri, Feb 17, 2017 at 06:41:32PM -0500, Kyle Meyer wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> [...]
> 
> > Do we even want these "internal" delete_ref() invocations to be
> > logged in HEAD's reflog?  I understand that this is inside the
> > implementation of renaming an old ref to a new ref, and reflog
> > message given to delete_ref() would matter only if the HEAD happens
> > to be pointing at old ref---but then HEAD will be repointed to the
> > new ref by somebody else [*1*] that called this function to rename
> > old to new and it _will_ log it.
> 
> I know the discussion has developed further, but just a note that I
> think the last statement is inaccurate: currently, a rename will not be
> recorded in HEAD's log.  "git branch -m" will show a renaming event in
> the new branch's log, but the only trace of the event in HEAD's log is
> the deletion entry with an empty message.

Right. I assumed Junio was talking about the hypothetical behavior we'd
want.

Your response did make me think of one other option: if we updated HEAD
_before_ writing the new ref, then it would automatically get the
"create" half of the rename. IOW, if the rename process were:

  1. delete old; this writes a reflog to HEAD, per the magic
     HEAD-detection in commit_ref_update().

  2. update HEAD to point to new

  3. re-create new; this uses the same magic to write a HEAD reflog.

That's probably a crazy path to go, though. Right now steps (1) and (3)
happen in a low-level function, and step (2) happens outside of it
completely. Arguably it would be good to change that, but I think we
want to think of (1) and (3) as an atomic operation. Putting more things
which might fail between them is a bad idea.

So I think your existing patches (modulo the review comments), plus the
"something like this" that I sent on top are probably a better end
state.

-Peff

  parent reply	other threads:[~2017-02-17 23:54 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-16 23:17 HEAD's reflog entry for a renamed branch Kyle Meyer
2017-01-26 21:12 ` Jeff King
2017-01-26 21:30   ` Junio C Hamano
2017-01-26 21:53     ` Jeff King
2017-02-17  3:57   ` [PATCH 0/3] delete_ref(): support reflog messages Kyle Meyer
2017-02-17  3:57     ` [PATCH 1/3] delete_refs(): accept a reflog message argument Kyle Meyer
2017-02-17  8:12       ` Jeff King
2017-02-17 17:05         ` Junio C Hamano
2017-02-17 23:35           ` Kyle Meyer
2017-02-17  3:57     ` [PATCH 2/3] update-ref: pass reflog message argument to delete_refs Kyle Meyer
2017-02-17  8:22       ` Jeff King
2017-02-17 17:11         ` Junio C Hamano
2017-02-17 23:40         ` Kyle Meyer
2017-02-17 23:41           ` Jeff King
2017-02-17  3:58     ` [PATCH 3/3] rename_ref: replace empty deletion message in HEAD's log Kyle Meyer
2017-02-17  8:31       ` Jeff King
2017-02-17 17:50         ` Junio C Hamano
2017-02-17 19:43           ` Jeff King
2017-02-17 19:55             ` Jeff King
2017-02-17 22:18               ` Junio C Hamano
2017-02-17 23:42               ` Kyle Meyer
2017-02-17 23:41           ` Kyle Meyer
2017-02-17 23:53             ` Junio C Hamano
2017-02-17 23:53             ` Jeff King [this message]
2017-02-17  8:17     ` [PATCH 0/3] delete_ref(): support reflog messages Jeff King
2017-02-21  1:10     ` [PATCH v2 0/4] delete_ref: " Kyle Meyer
2017-02-21  1:10       ` [PATCH v2 1/4] delete_ref: accept a reflog message argument Kyle Meyer
2017-02-23  9:33         ` Duy Nguyen
2017-02-23 22:29           ` Junio C Hamano
2017-02-25  3:05           ` Kyle Meyer
2017-02-21  1:10       ` [PATCH v2 2/4] update-ref: pass reflog message to delete_ref() Kyle Meyer
2017-02-21  1:10       ` [PATCH v2 3/4] rename_ref: replace empty message in HEAD's log Kyle Meyer
2017-02-21  1:10       ` [PATCH v2 4/4] branch: record creation of renamed branch " Kyle Meyer
2017-02-21  7:12       ` [PATCH v2 0/4] delete_ref: support reflog messages Jeff King
2017-02-21  7:18       ` Junio C Hamano
2017-02-21 16:45         ` Kyle Meyer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170217235324.n3jabck23bvd4cs2@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=kyle@kyleam.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).