| Message ID | cddb2de9394a07e405682e9ccdfdf5de92bb9092.1708353264.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Commit | b02aa72b8162208fae5e669a187df1548e899474 |
| Headers | show |
| Series | reflog: introduce subcommand to list reflogs | expand |
Patrick Steinhardt <ps@pks.im> writes: > diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh > index d2f5f42e67..6d8d5a253d 100755 > --- a/t/t1410-reflog.sh > +++ b/t/t1410-reflog.sh > @@ -436,4 +436,73 @@ test_expect_success 'empty reflog' ' > test_must_be_empty err > ' > > +test_expect_success 'list reflogs' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + git reflog list >actual && > + test_must_be_empty actual && > + > + test_commit A && > + cat >expect <<-EOF && > + HEAD > + refs/heads/main > + EOF > + git reflog list >actual && > + test_cmp expect actual && > + > + git branch b && > + cat >expect <<-EOF && > + HEAD > + refs/heads/b > + refs/heads/main > + EOF > + git reflog list >actual && > + test_cmp expect actual > + ) > +' OK. This is a quite boring baseline. > +test_expect_success 'reflog list returns error with additional args' ' > + cat >expect <<-EOF && > + error: list does not accept arguments: ${SQ}bogus${SQ} > + EOF > + test_must_fail git reflog list bogus 2>err && > + test_cmp expect err > +' Makes sense. > +test_expect_success 'reflog for symref with unborn target can be listed' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit A && > + git symbolic-ref HEAD refs/heads/unborn && > + cat >expect <<-EOF && > + HEAD > + refs/heads/main > + EOF > + git reflog list >actual && > + test_cmp expect actual > + ) > +' Should this be under REFFILES? Ah, no, "git symbolic-ref" is valid under reftable as well, so there is no need to. Without [5/6], would it have failed to show the reflog for HEAD? > +test_expect_success 'reflog with invalid object ID can be listed' ' > + test_when_finished "rm -rf repo" && > + git init repo && > + ( > + cd repo && > + test_commit A && > + test-tool ref-store main update-ref msg refs/heads/missing \ > + $(test_oid deadbeef) "$ZERO_OID" REF_SKIP_OID_VERIFICATION && > + cat >expect <<-EOF && > + HEAD > + refs/heads/main > + refs/heads/missing > + EOF > + git reflog list >actual && > + test_cmp expect actual > + ) > +' OK. > test_done It would have been "interesting" to see an example of "there is a reflog but the underlying ref for it is missing" case, but I think that falls into a minor repository corruption category, so lack of such a test is also fine.
On Mon, Feb 19, 2024 at 04:32:43PM -0800, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh > > index d2f5f42e67..6d8d5a253d 100755 > > --- a/t/t1410-reflog.sh > > +++ b/t/t1410-reflog.sh > > @@ -436,4 +436,73 @@ test_expect_success 'empty reflog' ' > > test_must_be_empty err > > ' > > > > +test_expect_success 'list reflogs' ' > > + test_when_finished "rm -rf repo" && > > + git init repo && > > + ( > > + cd repo && > > + git reflog list >actual && > > + test_must_be_empty actual && > > + > > + test_commit A && > > + cat >expect <<-EOF && > > + HEAD > > + refs/heads/main > > + EOF > > + git reflog list >actual && > > + test_cmp expect actual && > > + > > + git branch b && > > + cat >expect <<-EOF && > > + HEAD > > + refs/heads/b > > + refs/heads/main > > + EOF > > + git reflog list >actual && > > + test_cmp expect actual > > + ) > > +' > > OK. This is a quite boring baseline. > > > +test_expect_success 'reflog list returns error with additional args' ' > > + cat >expect <<-EOF && > > + error: list does not accept arguments: ${SQ}bogus${SQ} > > + EOF > > + test_must_fail git reflog list bogus 2>err && > > + test_cmp expect err > > +' > > Makes sense. > > > +test_expect_success 'reflog for symref with unborn target can be listed' ' > > + test_when_finished "rm -rf repo" && > > + git init repo && > > + ( > > + cd repo && > > + test_commit A && > > + git symbolic-ref HEAD refs/heads/unborn && > > + cat >expect <<-EOF && > > + HEAD > > + refs/heads/main > > + EOF > > + git reflog list >actual && > > + test_cmp expect actual > > + ) > > +' > > Should this be under REFFILES? Ah, no, "git symbolic-ref" is valid > under reftable as well, so there is no need to. > > Without [5/6], would it have failed to show the reflog for HEAD? I initially thought so, but no. `refs_resolve_ref_unsafe()` is weird as it returns successfully even if a symref cannot be resolved unless you pass `RESOLVE_REF_READING`, which we didn't. The case where it does make a difference is if we had a corrupt ref. So if you "echo garbage >.git/refs/heads/branch", then the corresponding reflog would not have been listed. Even worse, even after this patch series it's still impossible to `git reflog show` the reflog because we fail to resolve the ref itself, which basically breaks the whole point of the reflog. This is something that I plan to address in a follow-up patch series. > > +test_expect_success 'reflog with invalid object ID can be listed' ' > > + test_when_finished "rm -rf repo" && > > + git init repo && > > + ( > > + cd repo && > > + test_commit A && > > + test-tool ref-store main update-ref msg refs/heads/missing \ > > + $(test_oid deadbeef) "$ZERO_OID" REF_SKIP_OID_VERIFICATION && > > + cat >expect <<-EOF && > > + HEAD > > + refs/heads/main > > + refs/heads/missing > > + EOF > > + git reflog list >actual && > > + test_cmp expect actual > > + ) > > +' > > OK. > > > test_done > > It would have been "interesting" to see an example of "there is a > reflog but the underlying ref for it is missing" case, but I think > that falls into a minor repository corruption category, so lack of > such a test is also fine. The reason why I didn't include such a test is that it's by necessity specific to the backend: we don't have any way to delete a ref without also deleting the corresponding reflog. So we'd have to manually delete it, which only works with the REFFILES backend. Patrick
diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt index ec64cbff4c..a929c52982 100644 --- a/Documentation/git-reflog.txt +++ b/Documentation/git-reflog.txt @@ -10,6 +10,7 @@ SYNOPSIS -------- [verse] 'git reflog' [show] [<log-options>] [<ref>] +'git reflog list' 'git reflog expire' [--expire=<time>] [--expire-unreachable=<time>] [--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] [--verbose] [--all [--single-worktree] | <refs>...] @@ -39,6 +40,8 @@ actions, and in addition the `HEAD` reflog records branch switching. `git reflog show` is an alias for `git log -g --abbrev-commit --pretty=oneline`; see linkgit:git-log[1] for more information. +The "list" subcommand lists all refs which have a corresponding reflog. + The "expire" subcommand prunes older reflog entries. Entries older than `expire` time, or entries older than `expire-unreachable` time and not reachable from the current tip, are removed from the reflog. diff --git a/builtin/reflog.c b/builtin/reflog.c index 3a0c4d4322..63cd4d8b29 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -7,11 +7,15 @@ #include "wildmatch.h" #include "worktree.h" #include "reflog.h" +#include "refs.h" #include "parse-options.h" #define BUILTIN_REFLOG_SHOW_USAGE \ N_("git reflog [show] [<log-options>] [<ref>]") +#define BUILTIN_REFLOG_LIST_USAGE \ + N_("git reflog list") + #define BUILTIN_REFLOG_EXPIRE_USAGE \ N_("git reflog expire [--expire=<time>] [--expire-unreachable=<time>]\n" \ " [--rewrite] [--updateref] [--stale-fix]\n" \ @@ -29,6 +33,11 @@ static const char *const reflog_show_usage[] = { NULL, }; +static const char *const reflog_list_usage[] = { + BUILTIN_REFLOG_LIST_USAGE, + NULL, +}; + static const char *const reflog_expire_usage[] = { BUILTIN_REFLOG_EXPIRE_USAGE, NULL @@ -46,6 +55,7 @@ static const char *const reflog_exists_usage[] = { static const char *const reflog_usage[] = { BUILTIN_REFLOG_SHOW_USAGE, + BUILTIN_REFLOG_LIST_USAGE, BUILTIN_REFLOG_EXPIRE_USAGE, BUILTIN_REFLOG_DELETE_USAGE, BUILTIN_REFLOG_EXISTS_USAGE, @@ -238,6 +248,29 @@ static int cmd_reflog_show(int argc, const char **argv, const char *prefix) return cmd_log_reflog(argc, argv, prefix); } +static int show_reflog(const char *refname, void *cb_data UNUSED) +{ + printf("%s\n", refname); + return 0; +} + +static int cmd_reflog_list(int argc, const char **argv, const char *prefix) +{ + struct option options[] = { + OPT_END() + }; + struct ref_store *ref_store; + + argc = parse_options(argc, argv, prefix, options, reflog_list_usage, 0); + if (argc) + return error(_("%s does not accept arguments: '%s'"), + "list", argv[0]); + + ref_store = get_main_ref_store(the_repository); + + return refs_for_each_reflog(ref_store, show_reflog, NULL); +} + static int cmd_reflog_expire(int argc, const char **argv, const char *prefix) { struct cmd_reflog_expire_cb cmd = { 0 }; @@ -417,6 +450,7 @@ int cmd_reflog(int argc, const char **argv, const char *prefix) parse_opt_subcommand_fn *fn = NULL; struct option options[] = { OPT_SUBCOMMAND("show", &fn, cmd_reflog_show), + OPT_SUBCOMMAND("list", &fn, cmd_reflog_list), OPT_SUBCOMMAND("expire", &fn, cmd_reflog_expire), OPT_SUBCOMMAND("delete", &fn, cmd_reflog_delete), OPT_SUBCOMMAND("exists", &fn, cmd_reflog_exists), diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh index d2f5f42e67..6d8d5a253d 100755 --- a/t/t1410-reflog.sh +++ b/t/t1410-reflog.sh @@ -436,4 +436,73 @@ test_expect_success 'empty reflog' ' test_must_be_empty err ' +test_expect_success 'list reflogs' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + git reflog list >actual && + test_must_be_empty actual && + + test_commit A && + cat >expect <<-EOF && + HEAD + refs/heads/main + EOF + git reflog list >actual && + test_cmp expect actual && + + git branch b && + cat >expect <<-EOF && + HEAD + refs/heads/b + refs/heads/main + EOF + git reflog list >actual && + test_cmp expect actual + ) +' + +test_expect_success 'reflog list returns error with additional args' ' + cat >expect <<-EOF && + error: list does not accept arguments: ${SQ}bogus${SQ} + EOF + test_must_fail git reflog list bogus 2>err && + test_cmp expect err +' + +test_expect_success 'reflog for symref with unborn target can be listed' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit A && + git symbolic-ref HEAD refs/heads/unborn && + cat >expect <<-EOF && + HEAD + refs/heads/main + EOF + git reflog list >actual && + test_cmp expect actual + ) +' + +test_expect_success 'reflog with invalid object ID can be listed' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit A && + test-tool ref-store main update-ref msg refs/heads/missing \ + $(test_oid deadbeef) "$ZERO_OID" REF_SKIP_OID_VERIFICATION && + cat >expect <<-EOF && + HEAD + refs/heads/main + refs/heads/missing + EOF + git reflog list >actual && + test_cmp expect actual + ) +' + test_done
While the git-reflog(1) command has subcommands to show reflog entries or check for reflog existence, it does not have any subcommands that would allow the user to enumerate all existing reflogs. This makes it quite hard to discover which reflogs a repository has. While this can be worked around with the "files" backend by enumerating files in the ".git/logs" directory, users of the "reftable" backend don't enjoy such a luxury. Introduce a new subcommand `git reflog list` that lists all reflogs the repository knows of to fill this gap. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- Documentation/git-reflog.txt | 3 ++ builtin/reflog.c | 34 ++++++++++++++++++ t/t1410-reflog.sh | 69 ++++++++++++++++++++++++++++++++++++ 3 files changed, 106 insertions(+)