Re: [PATCH v6 6/7] git-reflog: add create and exists functions
From: Michael Haggerty <hidden>
Date: 2016-06-15 23:05:39
On 06/29/2015 10:17 PM, David Turner wrote:
quoted hunk ↗ jump to hunk
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 <redacted> --- 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.
quoted hunk ↗ jump to hunk
+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@alum.mit.edu