git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: David Turner <dturner@twopensource.com>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v6 6/7] git-reflog: add create and exists functions
Date: Tue, 07 Jul 2015 20:49:37 -0400	[thread overview]
Message-ID: <1436316577.5521.25.camel@twopensource.com> (raw)
In-Reply-To: <559AB200.5020108@alum.mit.edu>

On Mon, 2015-07-06 at 18:51 +0200, Michael Haggerty wrote:

> > +{
> > +	int i, status = 0, start = 0;
> 
> It looks like start is initialized unconditionally after the first loop,
> so the initialization here is a red herring.

Will fix.

> So, I have a philosophical question here with a practical side...
> 
> It appears that this command allows you to create a reflog for a
> reference that doesn't yet exist. At first blush, this seems to make
> sense. Suppose you want the creation of a reference to be logged. Then
> you can first turn on its reflog, then create it.
> 
> But I think this is going to create problems. Reflogs are rather
> intimately connected to their references. For example, writes to a
> reflog are guarded by locking the corresponding reference. And currently
> a reflog file is deleted when the corresponding reference is deleted.
> Also, there are constraints about which references can coexist; for
> example, references "refs/heads/foo" and "refs/heads/foo/bar" cannot
> exist at the same time, because (at least when using the filesystem
> backend), "refs/heads/foo" would have to be both a file and a directory
> at the same time. So if one of these references already exists, it would
> be an error to create a reflog for the other one.
> 
> Similarly, if there is a reflog file for one of these references that
> was created by this command, but there is not yet a corresponding
> reference, then any command that later tries to create the other
> reference will not be able to create the reflog for *that* reference
> because it will conflict with the existing reflog. I doubt that the
> reference creation is smart enough to deal with this situation.
> 
> So all in all, I think it is unwise to allow a reflog to be created
> without its corresponding reference.
> 
> This, in turn, suggests one or both of the following alternatives:
> 
> 1. Allow "git reflog create", but only for references that already exist.

This turns out not to work for git stash, which wants to create a reflog
for stash creation.

> 2. If we want to allow a reflog to be created at the same time as the
> corresponding reference, the reference-creation commands ("git branch",
> "git tag", "git update-ref", and maybe some others) probably need a new
> option like "--create-reflog" (and, for symmetry, probably
> "--no-create-reflog").

git branch should already autocreate reflogs, since the refs it creates
are under refs/heads.

> At the API level, it might make sense for the ref-transaction functions
> to get a new "REF_FORCE_CREATE_REFLOG" flag or something.

Junio was opposed to the converse flag, so I'm going to just add
manually add code to create reflogs.

  reply	other threads:[~2015-07-08  0:49 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-29 20:17 [PATCH v6 0/7] refs backend preamble David Turner
2015-06-29 20:17 ` [PATCH v6 1/7] refs.c: add err arguments to reflog functions David Turner
2015-07-06 15:53   ` Michael Haggerty
2015-07-07 22:41     ` David Turner
2015-07-08 10:59       ` Michael Haggerty
2015-07-08 17:11         ` Junio C Hamano
2015-07-09  6:47           ` Michael Haggerty
2015-06-29 20:17 ` [PATCH v6 2/7] cherry-pick: treat CHERRY_PICK_HEAD and REVERT_HEAD as refs David Turner
2015-07-06 16:00   ` Michael Haggerty
2015-06-29 20:17 ` [PATCH v6 3/7] bisect: treat BISECT_HEAD as a ref David Turner
2015-06-29 20:17 ` [PATCH v6 4/7] refs: Break out check for reflog autocreation David Turner
2015-06-29 20:17 ` [PATCH v6 5/7] refs: new public ref function: safe_create_reflog David Turner
2015-07-06 16:21   ` Michael Haggerty
2015-07-07 23:18     ` David Turner
2015-07-08 11:04       ` Michael Haggerty
2015-06-29 20:17 ` [PATCH v6 6/7] git-reflog: add create and exists functions David Turner
2015-06-30  7:34   ` Eric Sunshine
2015-06-30 15:57     ` David Turner
2015-06-30 16:07     ` Junio C Hamano
2015-06-30 18:20       ` Eric Sunshine
2015-06-30 19:48         ` Junio C Hamano
2015-06-30 21:19           ` David Turner
2015-06-30 21:28             ` Junio C Hamano
2015-07-06 16:51   ` Michael Haggerty
2015-07-08  0:49     ` David Turner [this message]
2015-07-08 13:16       ` Michael Haggerty
2015-07-08 20:12         ` David Turner
2015-06-29 20:17 ` [PATCH v6 7/7] git-stash: use git-reflog instead of creating files David Turner
2015-06-29 21:03   ` Junio C Hamano
2015-06-29 20:31 ` [PATCH v6 0/7] refs backend preamble Junio C Hamano
2015-06-29 20:48   ` David Turner

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=1436316577.5521.25.camel@twopensource.com \
    --to=dturner@twopensource.com \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    /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).