Re: [PATCH 2/2] Setup working tree in describe

From: Duy Nguyen <pclouds@gmail.com>
Date: Jan 26, 2019, 11:01 UTC
Message-ID: <CACsJy8Df6XM55ExRfCjpUQsv2Vm0cOwrSAyNf6net__uztMOGQ@mail.gmail.com>
In-Reply-To: <CA+xP2Sax+thitfKv4hTtKTYPhfVXJxD_qoJxgkCyZFTzskP-Tw@mail.gmail.com>

On Sat, Jan 26, 2019 at 5:44 PM Sebastian Staudt <koraktor@gmail.com> wrote:

Show 18 quoted lines

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

Show 6 quoted lines

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

-- 
Duy