patch, 2 partsSetup working tree in describe

>
> This ensures the given working tree is used for --dirty and --broken.
>
> Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
> ---
>  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();

> +
>      if (argc == 0) {
>          if (broken) {
>              struct child_process cp = CHILD_PROCESS_INIT;
> --
> 2.20.1

>
> On Sat, Jan 26, 2019 at 5:44 PM Sebastian Staudt <koraktor@gmail.com> wrote:
> >
> > This ensures the given working tree is used for --dirty and --broken.
> >
> > Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
> > ---
> >  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

>
> On Sat, Jan 26, 2019 at 5:44 PM Sebastian Staudt <koraktor@gmail.com> wrote:
> >
> > This ensures the given working tree is used for --dirty and --broken.
> >
> > Signed-off-by: Sebastian Staudt <koraktor@gmail.com>
> > ---
> >  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

> 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.