{"thread":{"id":"50326","subject":"[PATCH 2/2] Setup working tree in describe","startedAt":"2019-01-26T10:41:18Z","lastAt":"2019-01-26T14:28:57Z","messageCount":4,"participants":["Sebastian Staudt","Duy Nguyen","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"367704","messageId":"CA+xP2Sax+thitfKv4hTtKTYPhfVXJxD_qoJxgkCyZFTzskP-Tw@mail.gmail.com","threadId":"50326","inReplyTo":null,"subject":"[PATCH 2/2] Setup working tree in describe","fromName":"Sebastian Staudt","fromEmail":"koraktor@gmail.com","sentAt":"2019-01-26T10:40:37Z","receivedAt":"2019-01-26T10:41:18Z","isPatch":true,"sender":{"key":"koraktor@gmail.com","avatar":"https://avatars.githubusercontent.com/u/66112?v=4"},"body":"This ensures the given working tree is used for --dirty and --broken.\n\nSigned-off-by: Sebastian Staudt \n---\n builtin/describe.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/describe.c b/builtin/describe.c\nindex cc118448ee..ba1a0b199b 100644\n--- a/builtin/describe.c\n+++ b/builtin/describe.c\n@@ -601,6 +601,8 @@ int cmd_describe(int argc, const char **argv,\nconst char *prefix)\n if (!hashmap_get_size(&names) && !always)\n die(_(\"No names found, cannot describe anything.\"));\n\n+ setup_work_tree();\n+\n if (argc == 0) {\n if (broken) {\n struct child_process cp = CHILD_PROCESS_INIT;\n-- \n2.20.1\n"},{"id":"367705","messageId":"CACsJy8Df6XM55ExRfCjpUQsv2Vm0cOwrSAyNf6net__uztMOGQ@mail.gmail.com","threadId":"50326","inReplyTo":"CA+xP2Sax+thitfKv4hTtKTYPhfVXJxD_qoJxgkCyZFTzskP-Tw@mail.gmail.com","subject":"Re: [PATCH 2/2] Setup working tree in describe","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-01-26T11:01:29Z","receivedAt":"2019-01-26T11:01:58Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Jan 26, 2019 at 5:44 PM Sebastian Staudt wrote:\n>\n> This ensures the given working tree is used for --dirty and --broken.\n>\n> Signed-off-by: Sebastian Staudt \n> ---\n> builtin/describe.c | 2 ++\n> 1 file changed, 2 insertions(+)\n>\n> diff --git a/builtin/describe.c b/builtin/describe.c\n> index cc118448ee..ba1a0b199b 100644\n> --- a/builtin/describe.c\n> +++ b/builtin/describe.c\n> @@ -601,6 +601,8 @@ int cmd_describe(int argc, const char **argv,\n> const char *prefix)\n> if (!hashmap_get_size(&names) && !always)\n> die(_(\"No names found, cannot describe anything.\"));\n>\n> + setup_work_tree();\n\nThis forces worktree's presence in all cases and will die() if\nworktree is not available. You need to check if broken or dirty is set\nand only call this function in that case.\n\nThough in my opinion it's better to call before we need it in the \"if\n(broke)\" and \"else if (dirty)\" code blocks. That way you don't even\nneed to check if it's \"dirty\" or \"broken\". Does \"broken\" really need\nthis though? If it runs \"git diff-index\" separately, that command\nshould handle this setup_work_tree() already, or we may need to fix it\nthere, not here.\n\n> +\n> if (argc == 0) {\n> if (broken) {\n> struct child_process cp = CHILD_PROCESS_INIT;\n> --\n> 2.20.1\n\n\n\n-- \nDuy\n"},{"id":"367708","messageId":"CA+xP2SaJYD+OKC18y0zz2V6A9J8rAB0-cxMnrDmWt==8v2_uBA@mail.gmail.com","threadId":"50326","inReplyTo":"CACsJy8Df6XM55ExRfCjpUQsv2Vm0cOwrSAyNf6net__uztMOGQ@mail.gmail.com","subject":"Re: [PATCH 2/2] Setup working tree in describe","fromName":"Sebastian Staudt","fromEmail":"koraktor@gmail.com","sentAt":"2019-01-26T14:07:54Z","receivedAt":"2019-01-26T14:08:35Z","isPatch":true,"sender":{"key":"koraktor@gmail.com","avatar":"https://avatars.githubusercontent.com/u/66112?v=4"},"body":"Am Sa., 26. Jan. 2019 um 12:01 Uhr schrieb Duy Nguyen :\n>\n> On Sat, Jan 26, 2019 at 5:44 PM Sebastian Staudt wrote:\n> >\n> > This ensures the given working tree is used for --dirty and --broken.\n> >\n> > Signed-off-by: Sebastian Staudt \n> > ---\n> > builtin/describe.c | 2 ++\n> > 1 file changed, 2 insertions(+)\n> >\n> > diff --git a/builtin/describe.c b/builtin/describe.c\n> > index cc118448ee..ba1a0b199b 100644\n> > --- a/builtin/describe.c\n> > +++ b/builtin/describe.c\n> > @@ -601,6 +601,8 @@ int cmd_describe(int argc, const char **argv,\n> > const char *prefix)\n> > if (!hashmap_get_size(&names) && !always)\n> > die(_(\"No names found, cannot describe anything.\"));\n> >\n> > + setup_work_tree();\n>\n> This forces worktree's presence in all cases and will die() if\n> worktree is not available. You need to check if broken or dirty is set\n> and only call this function in that case.\n>\n> Though in my opinion it's better to call before we need it in the \"if\n> (broke)\" and \"else if (dirty)\" code blocks. That way you don't even\n> need to check if it's \"dirty\" or \"broken\". Does \"broken\" really need\n> this though? If it runs \"git diff-index\" separately, that command\n> should handle this setup_work_tree() already, or we may need to fix it\n> there, not here.\n>\n\nThanks for your feedback.\nAre you sure that it will fail without a working tree?\nIs it even possible to have *no* working tree?\n\nI already tested this with some real life examples, e.g.\n\n git --git-dir /some/path/.git describe\n\nFrom inside and outside of other repositories.\nI didn‘t hit any errors so far.\n\n> > +\n> > if (argc == 0) {\n> > if (broken) {\n> > struct child_process cp = CHILD_PROCESS_INIT;\n> > --\n> > 2.20.1\n>\n>\n>\n> --\n> Duy\n\nBest regards,\n Sebastian\n\nAm Sa., 26. Jan. 2019 um 12:01 Uhr schrieb Duy Nguyen :\n>\n> On Sat, Jan 26, 2019 at 5:44 PM Sebastian Staudt wrote:\n> >\n> > This ensures the given working tree is used for --dirty and --broken.\n> >\n> > Signed-off-by: Sebastian Staudt \n> > ---\n> > builtin/describe.c | 2 ++\n> > 1 file changed, 2 insertions(+)\n> >\n> > diff --git a/builtin/describe.c b/builtin/describe.c\n> > index cc118448ee..ba1a0b199b 100644\n> > --- a/builtin/describe.c\n> > +++ b/builtin/describe.c\n> > @@ -601,6 +601,8 @@ int cmd_describe(int argc, const char **argv,\n> > const char *prefix)\n> > if (!hashmap_get_size(&names) && !always)\n> > die(_(\"No names found, cannot describe anything.\"));\n> >\n> > + setup_work_tree();\n>\n> This forces worktree's presence in all cases and will die() if\n> worktree is not available. You need to check if broken or dirty is set\n> and only call this function in that case.\n>\n> Though in my opinion it's better to call before we need it in the \"if\n> (broke)\" and \"else if (dirty)\" code blocks. That way you don't even\n> need to check if it's \"dirty\" or \"broken\". Does \"broken\" really need\n> this though? If it runs \"git diff-index\" separately, that command\n> should handle this setup_work_tree() already, or we may need to fix it\n> there, not here.\n>\n> > +\n> > if (argc == 0) {\n> > if (broken) {\n> > struct child_process cp = CHILD_PROCESS_INIT;\n> > --\n> > 2.20.1\n>\n>\n>\n> --\n> Duy\n"},{"id":"367709","messageId":"20190126142854.GA20692@sigill.intra.peff.net","threadId":"50326","inReplyTo":"CA+xP2SaJYD+OKC18y0zz2V6A9J8rAB0-cxMnrDmWt==8v2_uBA@mail.gmail.com","subject":"Re: [PATCH 2/2] Setup working tree in describe","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-01-26T14:28:54Z","receivedAt":"2019-01-26T14:28:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jan 26, 2019 at 03:07:54PM +0100, Sebastian Staudt wrote:\n\n> Are you sure that it will fail without a working tree?\n> Is it even possible to have *no* working tree?\n> \n> I already tested this with some real life examples, e.g.\n> \n> git --git-dir /some/path/.git describe\n> \n> From inside and outside of other repositories.\n> I didn‘t hit any errors so far.\n\nTry:\n\n git clone --bare . bare.git\n cd bare.git\n git describe\n\nThat works with current versions of Git, but yields \"fatal: this\noperation must be run in a work tree\" with your patch.\n\n-Peff\n\nPS Your patches seem whitespace-damaged. You might want to look into\n using git-send-email.\n"}]}