{"thread":{"id":"38947","subject":"[PATCH] prune --worktrees: fix expire vs worktree existence condition","startedAt":"2015-03-30T20:47:47Z","lastAt":"2015-03-31T12:25:01Z","messageCount":2,"participants":["Max Kirillov","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"258691","messageId":"1427748467-27786-1-git-send-email-max@max630.net","threadId":"38947","inReplyTo":null,"subject":"[PATCH] prune --worktrees: fix expire vs worktree existence condition","fromName":"Max Kirillov","fromEmail":"max@max630.net","sentAt":"2015-03-30T20:47:47Z","receivedAt":"2015-03-30T20:47:47Z","isPatch":true,"sender":{"key":"max@max630.net","avatar":"https://avatars.githubusercontent.com/u/381560?v=4"},"body":"`git prune --worktrees` was pruning worktrees which were non-existent OR\nexpired, while it rather should prune those which are orphaned AND\nexpired, as git-checkout documentation describes. Fix it.\n\nAdd test 'not prune proper checkouts', which uses valid but expired\nworktree.\n\nModify test 'not prune recent checkouts' to remove the worktree before\npruning - link in worktrees still must survive. In older form it is\nuseless because would pass always when the other test passes.\n\nSigned-off-by: Max Kirillov <max@max630.net>\n---\n builtin/prune.c                   | 10 +++++++---\n t/t2026-prune-linked-checkouts.sh | 12 ++++++++++++\n 2 files changed, 19 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/prune.c b/builtin/prune.c\nindex 34a5ce1..fd31485 100644\n--- a/builtin/prune.c\n+++ b/builtin/prune.c\n@@ -120,11 +120,15 @@ static int prune_worktree(const char *id, struct strbuf *reason)\n \t\tif (!stat(git_path(\"worktrees/%s/link\", id), &st_link) &&\n \t\t    st_link.st_nlink > 1)\n \t\t\treturn 0;\n-\t\tstrbuf_addf(reason, _(\"Removing worktrees/%s: gitdir file points to non-existent location\"), id);\n-\t\treturn 1;\n+\t\tif (st.st_mtime <= expire) {\n+\t\t    strbuf_addf(reason, _(\"Removing worktrees/%s: gitdir file points to non-existent location\"), id);\n+\t\t    return 1;\n+\t\t} else {\n+\t\t    return 0;\n+\t\t}\n \t}\n \tfree(path);\n-\treturn st.st_mtime <= expire;\n+\treturn 0;\n }\n \n static void prune_worktrees(void)\ndiff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh\nindex e885baf..1821a48 100755\n--- a/t/t2026-prune-linked-checkouts.sh\n+++ b/t/t2026-prune-linked-checkouts.sh\n@@ -4,6 +4,10 @@ test_description='prune $GIT_DIR/worktrees'\n \n . ./test-lib.sh\n \n+test_expect_success initialize '\n+\tgit commit --allow-empty -m init\n+'\n+\n test_expect_success 'prune --worktrees on normal repo' '\n \tgit prune --worktrees &&\n \ttest_must_fail git prune --worktrees abc\n@@ -77,8 +81,16 @@ test_expect_success 'not prune recent checkouts' '\n \tmkdir zz &&\n \tmkdir -p .git/worktrees/jlm &&\n \techo \"$(pwd)\"/zz >.git/worktrees/jlm/gitdir &&\n+\trmdir zz &&\n \tgit prune --worktrees --verbose --expire=2.days.ago &&\n \ttest -d .git/worktrees/jlm\n '\n \n+test_expect_success 'not prune proper checkouts' '\n+\ttest_when_finished rm -r .git/worktrees &&\n+\tgit checkout \"--to=$PWD/nop\" --detach master &&\n+\tgit prune --worktrees &&\n+\ttest -d .git/worktrees/nop\n+'\n+\n test_done\n-- \n2.3.4.2801.g3d0809b\n"},{"id":"258740","messageId":"CACsJy8DjmSvatifN3s4eyNz5uGuJ=BaTAJrC6aKVa+2Q=MZjeg@mail.gmail.com","threadId":"38947","inReplyTo":"1427748467-27786-1-git-send-email-max@max630.net","subject":"Re: [PATCH] prune --worktrees: fix expire vs worktree existence condition","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-03-31T12:25:01Z","receivedAt":"2015-03-31T12:25:01Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Mar 31, 2015 at 3:47 AM, Max Kirillov <max@max630.net> wrote:\n> `git prune --worktrees` was pruning worktrees which were non-existent OR\n> expired, while it rather should prune those which are orphaned AND\n> expired, as git-checkout documentation describes. Fix it.\n>\n> Add test 'not prune proper checkouts', which uses valid but expired\n> worktree.\n>\n> Modify test 'not prune recent checkouts' to remove the worktree before\n> pruning - link in worktrees still must survive. In older form it is\n> useless because would pass always when the other test passes.\n\nUrgh.. Ack.\n\n> Signed-off-by: Max Kirillov <max@max630.net>\n> ---\n>  builtin/prune.c                   | 10 +++++++---\n>  t/t2026-prune-linked-checkouts.sh | 12 ++++++++++++\n>  2 files changed, 19 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/prune.c b/builtin/prune.c\n> index 34a5ce1..fd31485 100644\n> --- a/builtin/prune.c\n> +++ b/builtin/prune.c\n> @@ -120,11 +120,15 @@ static int prune_worktree(const char *id, struct strbuf *reason)\n>                 if (!stat(git_path(\"worktrees/%s/link\", id), &st_link) &&\n>                     st_link.st_nlink > 1)\n>                         return 0;\n> -               strbuf_addf(reason, _(\"Removing worktrees/%s: gitdir file points to non-existent location\"), id);\n> -               return 1;\n> +               if (st.st_mtime <= expire) {\n> +                   strbuf_addf(reason, _(\"Removing worktrees/%s: gitdir file points to non-existent location\"), id);\n> +                   return 1;\n> +               } else {\n> +                   return 0;\n> +               }\n>         }\n>         free(path);\n> -       return st.st_mtime <= expire;\n> +       return 0;\n>  }\n>\n>  static void prune_worktrees(void)\n> diff --git a/t/t2026-prune-linked-checkouts.sh b/t/t2026-prune-linked-checkouts.sh\n> index e885baf..1821a48 100755\n> --- a/t/t2026-prune-linked-checkouts.sh\n> +++ b/t/t2026-prune-linked-checkouts.sh\n> @@ -4,6 +4,10 @@ test_description='prune $GIT_DIR/worktrees'\n>\n>  . ./test-lib.sh\n>\n> +test_expect_success initialize '\n> +       git commit --allow-empty -m init\n> +'\n> +\n>  test_expect_success 'prune --worktrees on normal repo' '\n>         git prune --worktrees &&\n>         test_must_fail git prune --worktrees abc\n> @@ -77,8 +81,16 @@ test_expect_success 'not prune recent checkouts' '\n>         mkdir zz &&\n>         mkdir -p .git/worktrees/jlm &&\n>         echo \"$(pwd)\"/zz >.git/worktrees/jlm/gitdir &&\n> +       rmdir zz &&\n>         git prune --worktrees --verbose --expire=2.days.ago &&\n>         test -d .git/worktrees/jlm\n>  '\n>\n> +test_expect_success 'not prune proper checkouts' '\n> +       test_when_finished rm -r .git/worktrees &&\n> +       git checkout \"--to=$PWD/nop\" --detach master &&\n> +       git prune --worktrees &&\n> +       test -d .git/worktrees/nop\n> +'\n> +\n>  test_done\n> --\n> 2.3.4.2801.g3d0809b\n>\n\n\n\n-- \nDuy\n"}]}