Am Sa., 26. Jan. 2019 um 12:01 Uhr schrieb Duy Nguyen <pclouds@xxxxxxxxx>: > > On Sat, Jan 26, 2019 at 5:44 PM Sebastian Staudt <koraktor@xxxxxxxxx> wrote: > > > > This ensures the given working tree is used for --dirty and --broken. > > > > Signed-off-by: Sebastian Staudt <koraktor@xxxxxxxxx> > > --- > > builtin/describe.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/builtin/describe.c b/builtin/describe.c > > index cc118448ee..ba1a0b199b 100644 > > --- a/builtin/describe.c > > +++ b/builtin/describe.c > > @@ -601,6 +601,8 @@ int cmd_describe(int argc, const char **argv, > > const char *prefix) > > if (!hashmap_get_size(&names) && !always) > > die(_("No names found, cannot describe anything.")); > > > > + setup_work_tree(); > > This forces worktree's presence in all cases and will die() if > worktree is not available. You need to check if broken or dirty is set > and only call this function in that case. > > Though in my opinion it's better to call before we need it in the "if > (broke)" and "else if (dirty)" code blocks. That way you don't even > need to check if it's "dirty" or "broken". Does "broken" really need > this though? If it runs "git diff-index" separately, that command > should handle this setup_work_tree() already, or we may need to fix it > there, not here. > Thanks for your feedback. Are you sure that it will fail without a working tree? Is it even possible to have *no* working tree? I already tested this with some real life examples, e.g. git --git-dir /some/path/.git describe >From inside and outside of other repositories. I didn‘t hit any errors so far. > > + > > if (argc == 0) { > > if (broken) { > > struct child_process cp = CHILD_PROCESS_INIT; > > -- > > 2.20.1 > > > > -- > Duy Best regards, Sebastian Am Sa., 26. Jan. 2019 um 12:01 Uhr schrieb Duy Nguyen <pclouds@xxxxxxxxx>: > > On Sat, Jan 26, 2019 at 5:44 PM Sebastian Staudt <koraktor@xxxxxxxxx> wrote: > > > > This ensures the given working tree is used for --dirty and --broken. > > > > Signed-off-by: Sebastian Staudt <koraktor@xxxxxxxxx> > > --- > > builtin/describe.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/builtin/describe.c b/builtin/describe.c > > index cc118448ee..ba1a0b199b 100644 > > --- a/builtin/describe.c > > +++ b/builtin/describe.c > > @@ -601,6 +601,8 @@ int cmd_describe(int argc, const char **argv, > > const char *prefix) > > if (!hashmap_get_size(&names) && !always) > > die(_("No names found, cannot describe anything.")); > > > > + setup_work_tree(); > > This forces worktree's presence in all cases and will die() if > worktree is not available. You need to check if broken or dirty is set > and only call this function in that case. > > Though in my opinion it's better to call before we need it in the "if > (broke)" and "else if (dirty)" code blocks. That way you don't even > need to check if it's "dirty" or "broken". Does "broken" really need > this though? If it runs "git diff-index" separately, that command > should handle this setup_work_tree() already, or we may need to fix it > there, not here. > > > + > > if (argc == 0) { > > if (broken) { > > struct child_process cp = CHILD_PROCESS_INIT; > > -- > > 2.20.1 > > > > -- > Duy