On 06/29/2015 10:17 PM, David Turner wrote:
> These are necessary because alternate ref backends might store reflogs
> somewhere other than .git/logs. Code that now directly manipulates
> .git/logs should instead go through git-reflog.
>
> In a moment, we will use these functions to make git stash work with
> alternate ref backends.
>
> Signed-off-by: David Turner <dturner@xxxxxxxxxxxxxxxx>
> ---
> Documentation/git-reflog.txt | 10 ++++++
> builtin/reflog.c | 75 +++++++++++++++++++++++++++++++++++++++++++-
> t/t1411-reflog-show.sh | 12 +++++++
> 3 files changed, 96 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt
> index 5e7908e..2bf8aa6 100644
> --- a/Documentation/git-reflog.txt
> +++ b/Documentation/git-reflog.txt
> @@ -23,6 +23,8 @@ depending on the subcommand:
> [--dry-run] [--verbose] [--all | <refs>...]
> 'git reflog delete' [--rewrite] [--updateref]
> [--dry-run] [--verbose] ref@\{specifier\}...
> +'git reflog create' <refs>...
> +'git reflog exists' <ref>
>
> Reference logs, or "reflogs", record when the tips of branches and
> other references were updated in the local repository. Reflogs are
> @@ -52,6 +54,14 @@ argument must be an _exact_ entry (e.g. "`git reflog delete
> master@{2}`"). This subcommand is also typically not used directly by
> end users.
>
> +The "create" subcommand creates a reflog for one or more refs. Most
> +refs (those under refs/heads, refs/remotes, and refs/tags) will
> +automatically have reflogs created. Other refs will not. This command
> +allows manual ref creation.
> +
> +The "exists" subcommand checks whether a ref has a reflog. It exists
> +with zero status if the reflog exists, and non-zero status if it does
> +not.
>
> OPTIONS
> -------
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> index c2eb8ff..3080865 100644
> --- a/builtin/reflog.c
> +++ b/builtin/reflog.c
> @@ -13,6 +13,10 @@ static const char reflog_expire_usage[] =
> "git reflog expire [--expire=<time>] [--expire-unreachable=<time>] [--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] [--verbose] [--all] <refs>...";
> static const char reflog_delete_usage[] =
> "git reflog delete [--rewrite] [--updateref] [--dry-run | -n] [--verbose] <refs>...";
> +static const char reflog_create_usage[] =
> +"git reflog create <refs>...";
> +static const char reflog_exists_usage[] =
> +"git reflog exists <ref>";
>
> static unsigned long default_reflog_expire;
> static unsigned long default_reflog_expire_unreachable;
> @@ -699,12 +703,75 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)
> return status;
> }
>
> +static int cmd_reflog_create(int argc, const char **argv, const char *prefix)
> +{
> + 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.
> + struct strbuf err = STRBUF_INIT;
> +
> + for (i = 1; i < argc; i++) {
> + const char *arg = argv[i];
> + if (!strcmp(arg, "--")) {
> + i++;
> + break;
> + }
> + else if (arg[0] == '-')
> + usage(reflog_create_usage);
> + else
> + break;
> + }
> +
> + start = i;
> +
> + if (argc - start < 1)
if (start == argc)
seems clearer, but that might just be me.
> + return error("Nothing to create?");
> +
> + for (i = start; i < argc; i++) {
> + if (check_refname_format(argv[i], REFNAME_ALLOW_ONELEVEL))
> + die("invalid ref format: %s", argv[i]);
> + }
> + for (i = start; i < argc; i++) {
> + if (safe_create_reflog(argv[i], &err, 1)) {
> + error("could not create reflog %s: %s", argv[i],
> + err.buf);
> + status = 1;
> + strbuf_release(&err);
> + }
> + }
> + return status;
> +}
> +
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.
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").
At the API level, it might make sense for the ref-transaction functions
to get a new "REF_FORCE_CREATE_REFLOG" flag or something.
> +static int cmd_reflog_exists(int argc, const char **argv, const char *prefix)
> +{
> + int i, start = 0;
> +
> + for (i = 1; i < argc; i++) {
> + const char *arg = argv[i];
> + if (!strcmp(arg, "--")) {
> + i++;
> + break;
> + }
> + else if (arg[0] == '-')
> + usage(reflog_exists_usage);
> + else
> + break;
> + }
> +
> + start = i;
> +
> + if (argc - start != 1)
> + usage(reflog_exists_usage);
> +
> + if (check_refname_format(argv[start], REFNAME_ALLOW_ONELEVEL))
> + die("invalid ref format: %s", argv[start]);
> + return !reflog_exists(argv[start]);
> +}
> +
> /*
> * main "reflog"
> */
>
> static const char reflog_usage[] =
> -"git reflog [ show | expire | delete ]";
> +"git reflog [ show | expire | delete | create | exists ]";
>
> int cmd_reflog(int argc, const char **argv, const char *prefix)
> {
> @@ -724,5 +791,11 @@ int cmd_reflog(int argc, const char **argv, const char *prefix)
> if (!strcmp(argv[1], "delete"))
> return cmd_reflog_delete(argc - 1, argv + 1, prefix);
>
> + if (!strcmp(argv[1], "create"))
> + return cmd_reflog_create(argc - 1, argv + 1, prefix);
> +
> + if (!strcmp(argv[1], "exists"))
> + return cmd_reflog_exists(argc - 1, argv + 1, prefix);
> +
> return cmd_log_reflog(argc, argv, prefix);
> }
> diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh
> index 6f47c0d..6e1abe7 100755
> --- a/t/t1411-reflog-show.sh
> +++ b/t/t1411-reflog-show.sh
> @@ -166,4 +166,16 @@ test_expect_success 'git log -g -p shows diffs vs. parents' '
> test_cmp expect actual
> '
>
> +test_expect_success 'reflog exists works' '
> + git reflog exists refs/heads/master &&
> + ! git reflog exists refs/heads/nonexistent
> +'
> +
> +test_expect_success 'reflog create works' '
> + git update-ref non-refs-dir HEAD &&
> + ! git reflog exists non-refs-dir &&
> + git reflog create non-refs-dir &&
> + git reflog exists non-refs-dir
> +'
> +
> test_done
>
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html