git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 0/3] libify reflog

From
John Cai via GitGitGadget <gitgitgadget@gmail.com>
Date
Feb 25, 2022, 19:30 UTC
Message-ID
<pull.1218.v3.git.git.1645817452.gitgitgadget@gmail.com>
In-Reply-To
<pull.1218.v2.git.git.1645554651.gitgitgadget@gmail.com>

In [1], there was a discussion around a bug report of stash not recovering in the middle of the process when killed with ctl-c. It turned out to not be a bug we need to fix. However, out of that discussion came the idea of libifying reflog. This can stand alone as a code improvement.

stash.c currently shells out to call reflog to delete reflogs. Libify reflog delete and call it from both builtin/reflog.c and builtin/stash.c.

This patch has three parts:
 * add missing test coverage for git stash delete
 * libify reflog's delete functionality and move some of the helpers into a
   reflog.c library and call reflog_delete from builtin/reflog.c
 * call reflog_delete from builtin/stash.c
Updates since v2:
 * removed unnecessary includes
 * adjusted wrapping/whitespace in reflog.h
 * adjusted test to be isolated from other tests since currently tests for
   stash depend on each other. There was some discussion around this and
   even a possibility to refactor the tests. However, it would have been a
   larger effort than is worth for this series, so instead I just made one
   of the tests I added be isolated from the others.
Updates since v1:
 * added missing test coverage
 * squashed 1/3 and 2/3 together
 * moved enum into reflog.c
 * updated object.h's flag allocation mapping
 1. https://lore.kernel.org/git/220126.86h79qe692.gmgdl@evledraar.gmail.com/
John Cai (3):
  stash: add tests to ensure reflog --rewrite --updatref behavior
  reflog: libify delete reflog function and helpers
  stash: call reflog_delete() in reflog.c
 Makefile         |   1 +
 builtin/reflog.c | 455 +----------------------------------------------
 builtin/stash.c  |  18 +-
 object.h         |   2 +-
 reflog.c         | 432 ++++++++++++++++++++++++++++++++++++++++++++
 reflog.h         |  43 +++++
 t/t3903-stash.sh |  65 +++++--
 7 files changed, 539 insertions(+), 477 deletions(-)
 create mode 100644 reflog.c
 create mode 100644 reflog.h
base-commit: e6ebfd0e8cbbd10878070c8a356b5ad1b3ca464e
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1218%2Fjohn-cai%2Fjc-libify-reflog-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1218/john-cai/jc-libify-reflog-v3
Pull-Request: https://github.com/git/git/pull/1218
Range-diff vs v2:
 1:  6e136b62ca4 ! 1:  33299825fc4 stash: add test to ensure reflog --rewrite --updatref behavior
     @@ Metadata
      Author: John Cai <johncai86@gmail.com>
      
       ## Commit message ##
     -    stash: add test to ensure reflog --rewrite --updatref behavior
     +    stash: add tests to ensure reflog --rewrite --updatref behavior
      
          There is missing test coverage to ensure that the resulting reflogs
          after a git stash drop has had its old oid rewritten if applicable, and
     @@ Commit message
          Signed-off-by: John Cai <johncai86@gmail.com>
      
       ## t/t3903-stash.sh ##
     +@@ t/t3903-stash.sh: diff_cmp () {
     + 	rm -f "$1.compare" "$2.compare"
     + }
     + 
     +-test_expect_success 'stash some dirty working directory' '
     +-	echo 1 >file &&
     +-	git add file &&
     +-	echo unrelated >other-file &&
     +-	git add other-file &&
     ++setup_stash() {
     ++	repo_dir=$1
     ++	if test -z $repo_dir; then
     ++		repo_dir="."
     ++	fi
     ++
     ++	echo 1 >$repo_dir/file &&
     ++	git -C $repo_dir add file &&
     ++	echo unrelated >$repo_dir/other-file &&
     ++	git -C $repo_dir add other-file &&
     + 	test_tick &&
     +-	git commit -m initial &&
     +-	echo 2 >file &&
     ++	git -C $repo_dir commit -m initial &&
     ++	echo 2 >$repo_dir/file &&
     + 	git add file &&
     +-	echo 3 >file &&
     ++	echo 3 >$repo_dir/file &&
     + 	test_tick &&
     +-	git stash &&
     +-	git diff-files --quiet &&
     +-	git diff-index --cached --quiet HEAD
     ++	git -C $repo_dir stash &&
     ++	git -C $repo_dir diff-files --quiet &&
     ++	git -C $repo_dir diff-index --cached --quiet HEAD
     ++}
     ++
     ++test_expect_success 'stash some dirty working directory' '
     ++	setup_stash
     + '
     + 
     + cat >expect <<EOF
      @@ t/t3903-stash.sh: test_expect_success 'drop middle stash by index' '
       	test 1 = $(git show HEAD:file)
       '
     @@ t/t3903-stash.sh: test_expect_success 'drop middle stash by index' '
      +	test_cmp expect actual
      +'
      +
     -+test_expect_success 'drop stash reflog updates refs/stash with rewrite' '
     -+	git reset --hard &&
     -+	echo 9 >file &&
     -+	git stash &&
     -+	oid="$(git rev-parse stash@{0})" &&
     -+	git stash drop stash@{1} &&
     -+	cut -d" " -f1-2 .git/logs/refs/stash >actual &&
     ++test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite' '
     ++	git init repo &&
     ++	setup_stash repo &&
     ++	echo 9 >repo/file &&
     ++
     ++	old_oid="$(git -C repo rev-parse stash@{0})" &&
     ++	git -C repo stash &&
     ++	new_oid="$(git -C repo rev-parse stash@{0})" &&
     ++
     ++	cat >expect <<-EOF &&
     ++	$(test_oid zero) $old_oid
     ++	$old_oid $new_oid
     ++	EOF
     ++	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
     ++	test_cmp expect actual &&
     ++
     ++	git -C repo stash drop stash@{1} &&
     ++	cut -d" " -f1-2 repo/.git/logs/refs/stash >actual &&
      +	cat >expect <<-EOF &&
     -+	$(test_oid zero) $oid
     ++	$(test_oid zero) $new_oid
      +	EOF
      +	test_cmp expect actual
      +'
     @@ t/t3903-stash.sh: test_expect_success 'drop middle stash by index' '
       test_expect_success 'stash pop' '
       	git reset --hard &&
       	git stash pop &&
     --	test 3 = $(cat file) &&
     -+	test 9 = $(cat file) &&
     - 	test 1 = $(git show :file) &&
     - 	test 1 = $(git show HEAD:file) &&
     - 	test 0 = $(git stash list | wc -l)
 2:  e7c950218b1 ! 2:  33adfee4ca6 reflog: libify delete reflog function and helpers
     @@ builtin/reflog.c
      @@
       #include "builtin.h"
       #include "config.h"
     - #include "lockfile.h"
     +-#include "lockfile.h"
      -#include "object-store.h"
     - #include "repository.h"
     +-#include "repository.h"
      -#include "commit.h"
      -#include "refs.h"
     - #include "dir.h"
     +-#include "dir.h"
      -#include "tree-walk.h"
     - #include "diff.h"
     +-#include "diff.h"
       #include "revision.h"
       #include "reachable.h"
       #include "worktree.h"
     @@ object.h: struct object_array {
       ## reflog.c (new) ##
      @@
      +#include "cache.h"
     -+#include "commit.h"
      +#include "object-store.h"
     -+#include "reachable.h"
      +#include "reflog.h"
      +#include "refs.h"
      +#include "revision.h"
     -+#include "tree-walk.h"
      +#include "worktree.h"
      +
      +/* Remember to update object flag allocation in object.h */
     @@ reflog.h (new)
      @@
      +#ifndef REFLOG_H
      +#define REFLOG_H
     -+
      +#include "refs.h"
      +
      +struct cmd_reflog_expire_cb {
     @@ reflog.h (new)
      +	unsigned int dry_run:1;
      +};
      +
     -+int reflog_delete(const char *rev, enum expire_reflog_flags flags, int verbose);
     -+
     ++int reflog_delete(const char *rev, enum expire_reflog_flags flags,
     ++		  int verbose);
      +void reflog_expiry_cleanup(void *cb_data);
     -+
      +void reflog_expiry_prepare(const char *refname, const struct object_id *oid,
      +			   void *cb_data);
     -+
      +int should_expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
      +			     const char *email, timestamp_t timestamp, int tz,
      +			     const char *message, void *cb_data);
     -+
      +int count_reflog_ent(struct object_id *ooid, struct object_id *noid,
      +		     const char *email, timestamp_t timestamp, int tz,
      +		     const char *message, void *cb_data);
     -+
      +int should_expire_reflog_ent_verbose(struct object_id *ooid,
      +				     struct object_id *noid,
      +				     const char *email,
      +				     timestamp_t timestamp, int tz,
      +				     const char *message, void *cb_data);
     -+
      +#endif /* REFLOG_H */
 3:  a023a70092b = 3:  b17d8e5d43a stash: call reflog_delete() in reflog.c
-- 
gitgitgadget
Previous: John Cai via GitGitGadgetNext: John Cai via GitGitGadget
Message 43 of 63 in “libify reflog”
  1. 0/3 libify reflogJohn Cai via GitGitGadget, Feb 18, 2022
  2. 2/3 reflog: call reflog_delete from reflog.cJohn Cai via GitGitGadget, Feb 18, 2022
  3. Ævar Arnfjörð BjarmasonFeb 18, 2022
  4. Junio C HamanoFeb 18, 2022
  5. 3/3 stash: call reflog_delete from reflog.cJohn Cai via GitGitGadget, Feb 18, 2022
  6. Ævar Arnfjörð BjarmasonFeb 18, 2022
  7. Taylor BlauFeb 19, 2022
  8. John CaiFeb 22, 2022
  9. Ævar Arnfjörð BjarmasonFeb 22, 2022
  10. 1/3 reflog: libify delete reflog function and helpersJohn Cai via GitGitGadget, Feb 18, 2022
  11. Ævar Arnfjörð BjarmasonFeb 18, 2022
  12. Taylor BlauFeb 18, 2022
  13. Ævar Arnfjörð BjarmasonFeb 18, 2022
  14. Taylor BlauFeb 18, 2022
  15. John CaiFeb 21, 2022
  16. Taylor BlauFeb 21, 2022
  17. John CaiFeb 23, 2022
  18. Junio C HamanoFeb 18, 2022
  19. Ævar Arnfjörð BjarmasonFeb 19, 2022
  20. Taylor BlauFeb 19, 2022
  21. Junio C HamanoFeb 20, 2022
  22. Junio C HamanoFeb 18, 2022
  23. Ævar Arnfjörð BjarmasonFeb 18, 2022
  24. 0/3 libify reflogJohn Cai via GitGitGadget, Feb 22, 2022
  25. 1/3 stash: add test to ensure reflog --rewrite --updatref behaviorJohn Cai via GitGitGadget, Feb 22, 2022
  26. Ævar Arnfjörð BjarmasonFeb 23, 2022
  27. Junio C HamanoFeb 23, 2022
  28. John CaiFeb 23, 2022
  29. Ævar Arnfjörð BjarmasonFeb 23, 2022
  30. John CaiFeb 24, 2022
  31. Ævar Arnfjörð BjarmasonFeb 25, 2022
  32. Junio C HamanoFeb 25, 2022
  33. Junio C HamanoFeb 23, 2022
  34. John CaiFeb 23, 2022
  35. Junio C HamanoFeb 23, 2022
  36. John CaiFeb 24, 2022
  37. Ævar Arnfjörð BjarmasonFeb 23, 2022
  38. 2/3 reflog: libify delete reflog function and helpersJohn Cai via GitGitGadget, Feb 22, 2022
  39. Ævar Arnfjörð BjarmasonFeb 23, 2022
  40. John CaiFeb 23, 2022
  41. Junio C HamanoFeb 23, 2022
  42. 3/3 stash: call reflog_delete() in reflog.cJohn Cai via GitGitGadget, Feb 22, 2022
  43. 0/3 libify reflogJohn Cai via GitGitGadget, Feb 25, 2022
  44. 1/3 stash: add tests to ensure reflog --rewrite --updatref behaviorJohn Cai via GitGitGadget, Feb 25, 2022
  45. Ævar Arnfjörð BjarmasonMar 2, 2022
  46. 3/3 stash: call reflog_delete() in reflog.cJohn Cai via GitGitGadget, Feb 25, 2022
  47. 2/3 reflog: libify delete reflog function and helpersJohn Cai via GitGitGadget, Feb 25, 2022
  48. Taylor BlauFeb 25, 2022
  49. John CaiMar 2, 2022
  50. Ævar Arnfjörð BjarmasonMar 2, 2022
  51. 0/3 libify reflogJohn Cai via GitGitGadget, Mar 2, 2022
  52. 1/3 stash: add tests to ensure reflog --rewrite --updatref behaviorJohn Cai via GitGitGadget, Mar 2, 2022
  53. Junio C HamanoMar 2, 2022
  54. John CaiMar 3, 2022
  55. Phillip WoodMar 3, 2022
  56. Ævar Arnfjörð BjarmasonMar 3, 2022
  57. Phillip WoodMar 3, 2022
  58. John CaiMar 3, 2022
  59. Phillip WoodMar 8, 2022
  60. John CaiMar 8, 2022
  61. 3/3 stash: call reflog_delete() in reflog.cJohn Cai via GitGitGadget, Mar 2, 2022
  62. 2/3 reflog: libify delete reflog function and helpersJohn Cai via GitGitGadget, Mar 2, 2022
  63. Junio C HamanoMar 2, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.