[v4,0/3] libify reflog

Message

Philippe Blain via GitGitGadget March 2, 2022, 10:27 p.m. UTC

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 v3:

 * refactored test to have a smaller diff

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/[e-mail]/

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 |  43 ++++-
 7 files changed, 527 insertions(+), 467 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-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1218/john-cai/jc-libify-reflog-v4
Pull-Request: https://github.com/git/git/pull/1218

Range-diff vs v3:

 1:  33299825fc4 ! 1:  08bb8d3a9b9 stash: add tests to ensure reflog --rewrite --updatref behavior
     @@ t/t3903-stash.sh: diff_cmp () {
       }
       
      -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 &&
     + 	echo 1 >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
     + 	echo unrelated >other-file &&
     +@@ t/t3903-stash.sh: test_expect_success 'stash some dirty working directory' '
     + 	git stash &&
     + 	git diff-files --quiet &&
     + 	git diff-index --cached --quiet HEAD
      +}
      +
      +test_expect_success 'stash some dirty working directory' '
     @@ t/t3903-stash.sh: test_expect_success 'drop middle stash by index' '
      +
      +test_expect_success REFFILES 'drop stash reflog updates refs/stash with rewrite' '
      +	git init repo &&
     -+	setup_stash repo &&
     ++	(
     ++		cd repo &&
     ++		setup_stash
     ++	) &&
      +	echo 9 >repo/file &&
      +
      +	old_oid="$(git -C repo rev-parse stash@{0})" &&
 2:  33adfee4ca6 = 2:  50471c2ee6f reflog: libify delete reflog function and helpers
 3:  b17d8e5d43a = 3:  cb32d9bfe60 stash: call reflog_delete() in reflog.c

Comments

Junio C Hamano March 2, 2022, 11:34 p.m. UTC | #1

"John Cai via GitGitGadget" <[e-mail]> writes:

> This patch has three parts:
>
>  * add missing test coverage for git stash delete

This looking better, relative to the previous iteration.

>  * libify reflog's delete functionality and move some of the helpers into a
>    reflog.c library and call reflog_delete from builtin/reflog.c

This part hasn't changed since the implementation in the previous
round I've looked at; looked sane.

>  * call reflog_delete from builtin/stash.c

Likewise.

Thanks.  Will queue.