threads / patch / 40810

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

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

## tl;dr

3 messages between Nov 17, 2015 and Nov 17, 2015. Diffs are folded; open one to read it.

replies: 2people: 1as markdown or json

Edmundo Carmona Antoranz· Nov 17, 2015, 01:07 UTC · lore

If a file has been deleted/renamed, blame refused to display blame content even if the path provided does match an existing blob on said revision.

$ 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

Signed-off-by: Edmundo Carmona Antoranz <eantoranz@gmail.com>
---
 builtin/blame.c | 2 --
 1 file changed, 2 deletions(-)

Show changes to builtin/blame.c +0 −2

diff --git a/builtin/blame.c b/builtin/blame.c
index 83612f5..db430d5 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2683,8 +2683,6 @@ parse_done:
 		argv[argc - 1] = "--";
 
 		setup_work_tree();
-		if (!file_exists(path))
-			die_errno("cannot stat path '%s'", path);
 	}
 
 	revs.disable_stdin = 1;

-- 
2.6.2

Edmundo Carmona Antoranz· Nov 17, 2015, 01:16 UTC · re: Edmundo Carmona Antoranz · lore

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

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

> -               if (!file_exists(path))
> -                       die_errno("cannot stat path '%s'", path);

Two things: 1 What I would love to do is check if revisions were provided. Something like:

if (no_revisions() && !file_exists(path))
    die_errno("cannot stat path '%s'", path);

_but_ revisions are set up a little bit later. I don't know right now if I could just move it up (I don't think it would be that simple because I see there's some messing up with argv and argc in that 'if' that encloses the lines I'm removing). Maybe it would make sense to move the check for file existence to _after_ revisions have been set up? Even without the check for revisions, it's behaving kind of the way I mean it to. But I'm sure it'd be more elegant if I checked if revisions were provided.

2 It makes sense to create test cases for this patch, right?

Looking forward to your comments. Thanks in advance.

Edmundo Carmona Antoranz· Nov 17, 2015, 01:25 UTC · re: Edmundo Carmona Antoranz · lore

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

Ok... I think I got how to check revisions / file existence after revisions have been set up.

Sending next patch version

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

Show 24 quoted lines

> On Mon, Nov 16, 2015 at 7:07 PM, Edmundo Carmona Antoranz
> <eantoranz@gmail.com> wrote:
>> -               if (!file_exists(path))
>> -                       die_errno("cannot stat path '%s'", path);
>
> Two things:
> 1
> What I would love to do is check if revisions were provided. Something like:
>
> if (no_revisions() && !file_exists(path))
>     die_errno("cannot stat path '%s'", path);
>
> _but_ revisions are set up a little bit later. I don't know right now
> if I could just move it up (I don't think it would be that simple
> because I see there's some messing up with argv and argc in that 'if'
> that encloses the lines I'm removing). Maybe it would make sense to
> move the check for file existence to _after_ revisions have been set
> up? Even without the check for revisions, it's behaving kind of the
> way I mean it to. But I'm sure it'd be more elegant if I checked if
> revisions were provided.
>
> 2 It makes sense to create test cases for this patch, right?
>
> Looking forward to your comments. Thanks in advance.