Re: [PATCH v2] blame: avoid checking if a file exists on the working tree if a revision is provided

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Nov 17, 2015, 05:11 UTC
Message-ID: <CAPig+cTjg8yvmLO0uuWZt4nATUnfxJOj4f_AmvRP9mHsGRmEOg@mail.gmail.com>
In-Reply-To: <1447723762-32309-1-git-send-email-eantoranz@gmail.com>

On Mon, Nov 16, 2015 at 8:29 PM, Edmundo Carmona Antoranz <eantoranz@gmail.com> wrote:

> blame: avoid checking if a file exists on the working tree
> if a revision is provided

This subject is a bit long; try to keep it to about 72 characters or less.

More importantly, though, it doesn't give us a high level overview of the purpose of the patch, which is that it is fixing blame to work on a file at a given revision even if the file no longer exists in the worktree.

> If a file has been deleted/renamed, blame refused to display

Imperative: s/refused/refuses/

> blame content even if the path provided does match an existing
> blob on said revision.

git-blame documentation does not advertise "blame <file> <rev>" as a valid invocation. It does advertise "blame <rev> -- <file>", and this case already works correctly even when <file> does not exist in the worktree.

git-annotate documentation, on the other hand, does advertise "annotate <file> <rev>", and it fails to work when <file> is absent from the worktree, so perhaps you want to sell this patch as fixing git-annotate instead?

Show 23 quoted lines

> $ git status
> On branch hide
> Changes not staged for commit:
>   (use "git add/rm <file>..." to update what will be committed)
>   (use "git checkout -- <file>..." to discard changes in working directory)
>
>         deleted:    testfile1.txt
>
> no changes added to commit (use "git add" and/or "git commit -a")
>
> Before:
> $ git blame testfile1.txt
> fatal: cannot stat path 'testfile1.txt': No such file or directory
> $ git blame testfile1.txt HEAD
> fatal: cannot stat path 'testfile1.txt': No such file or directory
>
> After:
> $ git blame testfile1.txt
> fatal: Cannot lstat 'testfile1.txt': No such file or directory
> $ git blame testfile1.txt HEAD
> ^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 1) testfile 2
> ^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 2)
> ^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 3) Some content

This example is certainly illustrative, but an even more common case may be trying to blame/annotate a file which existed in an older revision but doesn't exist anymore at HEAD, thus is absent from the worktree. Such a case could likely be described in a sentence or two without resorting to verbose examples (though, not a big deal if you keep the example).

A new test or two would be welcome, possibly in t/annotate-tests.sh if you consider this also fixing git-blame, or in t8001-annotate.sh if you're selling it only as a fix for git-annotate.

Show 21 quoted lines

> Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
> ---
> diff --git a/builtin/blame.c b/builtin/blame.c
> @@ -2683,12 +2683,13 @@ parse_done:
>                 argv[argc - 1] = "--";
>
>                 setup_work_tree();
> -               if (!file_exists(path))
> -                       die_errno("cannot stat path '%s'", path);
>         }
>
>         revs.disable_stdin = 1;
>         setup_revisions(argc, argv, &revs, NULL);
> +       if (!revs.pending.nr && !file_exists(path))
> +               die_errno("cannot stat path '%s'", path);
> +
>         memset(&sb, 0, sizeof(sb));
>
>         sb.revs = &revs;
> --
> 2.6.2