[PATCH] prune --worktrees: fix expire vs worktree existence condition

[PATCH] prune --worktrees: fix expire vs worktree existence condition

[Max Kirillov]

`git prune --worktrees` was pruning worktrees which were non-existent OR
expired, while it rather should prune those which are orphaned AND
expired, as git-checkout documentation describes. Fix it.

Add test 'not prune proper checkouts', which uses valid but expired
worktree.

Modify test 'not prune recent checkouts' to remove the worktree before
pruning - link in worktrees still must survive. In older form it is
useless because would pass always when the other test passes.

Signed-off-by: Max Kirillov <max@max630.net>
---
 builtin/prune.c                   | 10 +++++++---
 t/t2026-prune-linked-checkouts.sh | 12 ++++++++++++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/builtin/prune.c b/builtin/prune.c
index 34a5ce1..fd31485 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -120,11 +120,15 @@ static int prune_worktree(const char *id, struct strbuf *reason)
 		if (!stat(git_path("worktrees/%s/link", id), &st_link) &&
 		    st_link.st_nlink > 1)
 			return 0;
-		strbuf_addf(reason, _("Removing worktrees/%s: gitdir file points to non-existent location"), id);
-		return 1;
+		if (st.st_mtime <= expire) {
+		    strbuf_addf(reason, _("Removing worktrees/%s: gitdir file points to non-existent location"), id);
+		    return 1;
+		} else {
+		    return 0;
+		}
 	}
 	free(path);
-	return st.st_mtime <= expire;
+	return 0;
 }
 
 static void prune_worktrees(void)
diff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh
index e885baf..1821a48 100755
--- a/t/t2026-prune-linked-checkouts.sh
+++ b/t/t2026-prune-linked-checkouts.sh
@@ -4,6 +4,10 @@ test_description='prune $GIT_DIR/worktrees'
 
 . ./test-lib.sh
 
+test_expect_success initialize '
+	git commit --allow-empty -m init
+'
+
 test_expect_success 'prune --worktrees on normal repo' '
 	git prune --worktrees &&
 	test_must_fail git prune --worktrees abc
@@ -77,8 +81,16 @@ test_expect_success 'not prune recent checkouts' '
 	mkdir zz &&
 	mkdir -p .git/worktrees/jlm &&
 	echo "$(pwd)"/zz >.git/worktrees/jlm/gitdir &&
+	rmdir zz &&
 	git prune --worktrees --verbose --expire=2.days.ago &&
 	test -d .git/worktrees/jlm
 '
 
+test_expect_success 'not prune proper checkouts' '
+	test_when_finished rm -r .git/worktrees &&
+	git checkout "--to=$PWD/nop" --detach master &&
+	git prune --worktrees &&
+	test -d .git/worktrees/nop
+'
+
 test_done
-- 
2.3.4.2801.g3d0809b

Re: [PATCH] prune --worktrees: fix expire vs worktree existence condition

[Duy Nguyen]

On Tue, Mar 31, 2015 at 3:47 AM, Max Kirillov <max@max630.net> wrote:
> `git prune --worktrees` was pruning worktrees which were non-existent OR
> expired, while it rather should prune those which are orphaned AND
> expired, as git-checkout documentation describes. Fix it.
>
> Add test 'not prune proper checkouts', which uses valid but expired
> worktree.
>
> Modify test 'not prune recent checkouts' to remove the worktree before
> pruning - link in worktrees still must survive. In older form it is
> useless because would pass always when the other test passes.

Urgh.. Ack.

> Signed-off-by: Max Kirillov <max@max630.net>
> ---
>  builtin/prune.c                   | 10 +++++++---
>  t/t2026-prune-linked-checkouts.sh | 12 ++++++++++++
>  2 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/prune.c b/builtin/prune.c
> index 34a5ce1..fd31485 100644
> --- a/builtin/prune.c
> +++ b/builtin/prune.c
> @@ -120,11 +120,15 @@ static int prune_worktree(const char *id, struct strbuf *reason)
>                 if (!stat(git_path("worktrees/%s/link", id), &st_link) &&
>                     st_link.st_nlink > 1)
>                         return 0;
> -               strbuf_addf(reason, _("Removing worktrees/%s: gitdir file points to non-existent location"), id);
> -               return 1;
> +               if (st.st_mtime <= expire) {
> +                   strbuf_addf(reason, _("Removing worktrees/%s: gitdir file points to non-existent location"), id);
> +                   return 1;
> +               } else {
> +                   return 0;
> +               }
>         }
>         free(path);
> -       return st.st_mtime <= expire;
> +       return 0;
>  }
>
>  static void prune_worktrees(void)
> diff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh
> index e885baf..1821a48 100755
> --- a/t/t2026-prune-linked-checkouts.sh
> +++ b/t/t2026-prune-linked-checkouts.sh
> @@ -4,6 +4,10 @@ test_description='prune $GIT_DIR/worktrees'
>
>  . ./test-lib.sh
>
> +test_expect_success initialize '
> +       git commit --allow-empty -m init
> +'
> +
>  test_expect_success 'prune --worktrees on normal repo' '
>         git prune --worktrees &&
>         test_must_fail git prune --worktrees abc
> @@ -77,8 +81,16 @@ test_expect_success 'not prune recent checkouts' '
>         mkdir zz &&
>         mkdir -p .git/worktrees/jlm &&
>         echo "$(pwd)"/zz >.git/worktrees/jlm/gitdir &&
> +       rmdir zz &&
>         git prune --worktrees --verbose --expire=2.days.ago &&
>         test -d .git/worktrees/jlm
>  '
>
> +test_expect_success 'not prune proper checkouts' '
> +       test_when_finished rm -r .git/worktrees &&
> +       git checkout "--to=$PWD/nop" --detach master &&
> +       git prune --worktrees &&
> +       test -d .git/worktrees/nop
> +'
> +
>  test_done
> --
> 2.3.4.2801.g3d0809b
>



-- 
Duy