All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Haggerty <mhagger@alum.mit.edu>
To: David Turner <dturner@twopensource.com>, git@vger.kernel.org
Subject: Re: [PATCH v7 7/8] update-ref and tag: add --create-reflog arg
Date: Wed, 08 Jul 2015 15:44:26 +0200	[thread overview]
Message-ID: <559D293A.9040307@alum.mit.edu> (raw)
In-Reply-To: <1436316963-25520-7-git-send-email-dturner@twopensource.com>

Please see my email about v6 [*] for an argument for why, at the API
level, the create-reflog functionality for new references needs to be
implemented within the ref_transaction API.

[*] http://article.gmane.org/gmane.comp.version-control.git/273682

On 07/08/2015 02:56 AM, David Turner wrote:
> update-ref and tag: add --create-reflog arg

`--create-reflog` is an "option", not an "arg".

> Allow the creation of a ref (e.g. stash) with a reflog already in
> place. For most refs (e.g. those under refs/heads), this happens
> automatically, but for others, we need this option.

As I mentioned, this fact is only true if core.logAllRefUpdates is
turned on (or by default in non-bare repos).

> Currently, git does this by pre-creating the reflog, but alternate ref
> backends might store reflogs somewhere other than .git/logs.  Code
> that now directly manipulates .git/logs should instead use git
> plumbing commands.
> 
> I also added --create-reflog to git tag, just for completeness.
> 
> In a moment, we will use this argument to make git stash work with
> alternate ref backends.

For a moment I thought that there should be a corresponding
`--no-create-reflog` option. But it would be pretty pointless, because
if this call to `update-ref` would want to create a reflog, then so
would *every* subsequent update to the reference. So the effect of
`--no-create-reflog` could at best be temporary.

> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
>  Documentation/git-tag.txt        |  5 ++++-
>  Documentation/git-update-ref.txt |  5 ++++-
>  builtin/tag.c                    |  5 +++++
>  builtin/update-ref.c             | 17 +++++++++++++++--
>  t/t1400-update-ref.sh            | 38 ++++++++++++++++++++++++++++++++++++++
>  t/t7004-tag.sh                   |  9 ++++++++-
>  6 files changed, 74 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
> index 034d10d..2312980 100644
> --- a/Documentation/git-tag.txt
> +++ b/Documentation/git-tag.txt
> @@ -13,7 +13,7 @@ SYNOPSIS
>  	<tagname> [<commit> | <object>]
>  'git tag' -d <tagname>...
>  'git tag' [-n[<num>]] -l [--contains <commit>] [--points-at <object>]
> -	[--column[=<options>] | --no-column] [<pattern>...]
> +	[--column[=<options>] | --no-column] [--create-reflog] [<pattern>...]
>  	[<pattern>...]
>  'git tag' -v <tagname>...
>  
> @@ -143,6 +143,9 @@ This option is only applicable when listing tags without annotation lines.
>  	all, 'whitespace' removes just leading/trailing whitespace lines and
>  	'strip' removes both whitespace and commentary.
>  
> +--create-reflog::
> +	Create a reflog for the tag.
> +
>  <tagname>::
>  	The name of the tag to create, delete, or describe.
>  	The new tag name must pass all checks defined by
> diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
> index c8f5ae5..969bfab 100644
> --- a/Documentation/git-update-ref.txt
> +++ b/Documentation/git-update-ref.txt
> @@ -8,7 +8,7 @@ git-update-ref - Update the object name stored in a ref safely
>  SYNOPSIS
>  --------
>  [verse]
> -'git update-ref' [-m <reason>] (-d <ref> [<oldvalue>] | [--no-deref] <ref> <newvalue> [<oldvalue>] | --stdin [-z])
> +'git update-ref' [-m <reason>] (-d <ref> [<oldvalue>] | [--no-deref] [--create-reflog] <ref> <newvalue> [<oldvalue>] | --stdin [-z])
>  
>  DESCRIPTION
>  -----------
> @@ -67,6 +67,9 @@ performs all modifications together.  Specify commands of the form:
>  	verify SP <ref> [SP <oldvalue>] LF
>  	option SP <opt> LF
>  
> +With `--create-reflog`, update-ref will create a reflog for each ref
> +even if one would not ordinarily be created.
> +
>  Quote fields containing whitespace as if they were strings in C source
>  code; i.e., surrounded by double-quotes and with backslash escapes.
>  Use 40 "0" characters or the empty string to specify a zero value.  To
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 5f6cdc5..896f434 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -579,6 +579,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>  	struct create_tag_options opt;
>  	char *cleanup_arg = NULL;
>  	int annotate = 0, force = 0, lines = -1;
> +	int create_reflog = 0;
>  	int cmdmode = 0;
>  	const char *msgfile = NULL, *keyid = NULL;
>  	struct msg_arg msg = { 0, STRBUF_INIT };
> @@ -605,6 +606,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>  		OPT_STRING('u', "local-user", &keyid, N_("key-id"),
>  					N_("use another key to sign the tag")),
>  		OPT__FORCE(&force, N_("replace the tag if exists")),
> +		OPT_BOOL(0, "create-reflog", &create_reflog, N_("create_reflog")),
>  
>  		OPT_GROUP(N_("Tag listing options")),
>  		OPT_COLUMN(0, "column", &colopts, N_("show tag list in columns")),
> @@ -730,6 +732,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
>  	if (annotate)
>  		create_tag(object, tag, &buf, &opt, prev, object);
>  
> +	if (create_reflog && safe_create_reflog(ref.buf, &err, 1))
> +		die("failed to create reflog for %s: %s", ref.buf, err.buf);
> +

This reflog should not be left around if the tag creation ultimately
fails for any reason.

>  	transaction = ref_transaction_begin(&err);
>  	if (!transaction ||
>  	    ref_transaction_update(transaction, ref.buf, object, prev,
> diff --git a/builtin/update-ref.c b/builtin/update-ref.c
> index 6763cf1..d570e5e 100644
> --- a/builtin/update-ref.c
> +++ b/builtin/update-ref.c
> @@ -14,6 +14,7 @@ static const char * const git_update_ref_usage[] = {
>  
>  static char line_termination = '\n';
>  static int update_flags;
> +int create_reflog;
>  static const char *msg;
>  
>  /*
> @@ -198,6 +199,9 @@ static const char *parse_cmd_update(struct ref_transaction *transaction,
>  	if (*next != line_termination)
>  		die("update %s: extra input: %s", refname, next);
>  
> +	if (create_reflog && safe_create_reflog(refname, &err, 1))
> +		die("failed to create reflog for %s: %s", refname, err.buf);
> +

It is valid to pass the empty string or 0{40} to the `update` command as
the "new" value, in which case the reference will be deleted. In that
case, no reflog should be created.

>  	if (ref_transaction_update(transaction, refname,
>  				   new_sha1, have_old ? old_sha1 : NULL,
>  				   update_flags, msg, &err))
> @@ -230,6 +234,9 @@ static const char *parse_cmd_create(struct ref_transaction *transaction,
>  	if (*next != line_termination)
>  		die("create %s: extra input: %s", refname, next);
>  
> +	if (create_reflog && safe_create_reflog(refname, &err, 1))
> +		die("failed to create reflog for %s: %s", refname, err.buf);
> +
>  	if (ref_transaction_create(transaction, refname, new_sha1,
>  				   update_flags, msg, &err))
>  		die("%s", err.buf);

Should the "verify" command also create a reflog, at least if the
reference is not being verified to be missing?

> @@ -361,6 +368,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
>  					N_("update <refname> not the one it points to")),
>  		OPT_BOOL('z', NULL, &end_null, N_("stdin has NUL-terminated arguments")),
>  		OPT_BOOL( 0 , "stdin", &read_stdin, N_("read updates from stdin")),
> +		OPT_BOOL( 0 , "create-reflog", &create_reflog, N_("create_reflog")),
>  		OPT_END(),
>  	};
>  
> @@ -421,7 +429,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
>  
>  	if (no_deref)
>  		flags = REF_NODEREF;
> -	if (delete)
> +	if (delete) {
>  		/*
>  		 * For purposes of backwards compatibility, we treat
>  		 * NULL_SHA1 as "don't care" here:
> @@ -429,7 +437,12 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
>  		return delete_ref(refname,
>  				  (oldval && !is_null_sha1(oldsha1)) ? oldsha1 : NULL,
>  				  flags);
> -	else
> +	} else {
> +		struct strbuf err = STRBUF_INIT;
> +		if (create_reflog && safe_create_reflog(refname, &err, 1))
> +			die("failed to create reflog for %s: %s", refname, err.buf);

This reflog also should not be left around if the reference does not
exist at the end of this command.

> +
>  		return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,
>  				  flags, UPDATE_REFS_DIE_ON_ERR);
> +	}
>  }
> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
> index d787bf5..9d21c19 100755
> --- a/t/t1400-update-ref.sh
> +++ b/t/t1400-update-ref.sh
> [...]

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu

  reply	other threads:[~2015-07-08 13:45 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-08  0:55 [PATCH v7 1/8] refs.c: add err arguments to reflog functions David Turner
2015-07-08  0:55 ` [PATCH v7 2/8] cherry-pick: treat CHERRY_PICK_HEAD and REVERT_HEAD as refs David Turner
2015-07-08 17:46   ` Johannes Sixt
2015-07-08 19:16     ` David Turner
2015-07-08 21:14       ` Johannes Sixt
2015-07-08 23:23         ` Junio C Hamano
2015-07-08 23:44           ` David Turner
2015-07-09  5:55             ` Junio C Hamano
2015-07-09 21:53               ` David Turner
2015-07-09 22:06                 ` Junio C Hamano
2015-07-10  4:30                   ` Michael Haggerty
2015-07-10 17:59                     ` David Turner
2015-07-14  4:33                     ` David Turner
2015-07-15 16:24                       ` Junio C Hamano
2015-07-15 18:04                         ` David Turner
2015-07-08 23:41         ` David Turner
2015-07-08  0:55 ` [PATCH v7 3/8] bisect: treat BISECT_HEAD as a ref David Turner
2015-07-08  0:55 ` [PATCH v7 4/8] refs: Break out check for reflog autocreation David Turner
2015-07-08  0:56 ` [PATCH v7 5/8] refs: new public ref function: safe_create_reflog David Turner
2015-07-08 11:49   ` Michael Haggerty
2015-07-08  0:56 ` [PATCH v7 6/8] git-reflog: add exists command David Turner
2015-07-08  0:56 ` [PATCH v7 7/8] update-ref and tag: add --create-reflog arg David Turner
2015-07-08 13:44   ` Michael Haggerty [this message]
2015-07-08 20:21     ` David Turner
2015-07-08  0:56 ` [PATCH v7 8/8] git-stash: use update-ref --create-reflog instead of creating files David Turner
2015-07-08  7:28   ` Junio C Hamano
2015-07-08  7:33   ` Junio C Hamano
2015-07-08 13:50   ` Michael Haggerty
2015-07-08 11:36 ` [PATCH v7 1/8] refs.c: add err arguments to reflog functions Michael Haggerty
2015-07-08 20:01   ` 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

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

  git send-email \
    --in-reply-to=559D293A.9040307@alum.mit.edu \
    --to=mhagger@alum.mit.edu \
    --cc=dturner@twopensource.com \
    --cc=git@vger.kernel.org \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.