Re: [PATCH] blame: Allow to blame paths freshly added to the index

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Mike Hommey <mh@xxxxxxxxxxxx> writes:

> When blaming files, changes in the work tree are taken into account
> and displayed as being "Not Committed Yet".
>
> However, when blaming a file that is not known to the current HEAD,
> git blame fails with `no such path 'foo' in HEAD`, even when the file
> was git add'ed.
>
> This would seem uninteresting with the plain `git blame` case, which
> it is, but it becomes useful when using copy detection, and the new file
> was created from pieces already in HEAD, moved or copied from other
> files.

I suspect that this would be useful without copy detection.  If you
"git mv fileA fileB" (optionally followed by "edit fileB"), fileB
would not be in HEAD but you should be able to trace the lineage of
the lines in it back through the renaming event, and this change
also allows that use case, no?

> ---

Missing sign-off?

>  builtin/blame.c               |  4 +++-
>  t/t8003-blame-corner-cases.sh | 23 +++++++++++++++++++++++
>  2 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 1e214bd..0858b18 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -2240,7 +2240,9 @@ static void verify_working_tree_path(struct commit *work_tree, const char *path)
>  		    sha1_object_info(blob_sha1, NULL) == OBJ_BLOB)
>  			return;
>  	}
> -	die("no such path '%s' in HEAD", path);
> +
> +	if (cache_name_pos(path, strlen(path)) < 0)
> +		die("no such path '%s' in HEAD", path);
>  }

This is a surprisingly small change to bring a big difference at the
usage level.  I am sort-of surprised that the "fake working tree
commit" mechanism was already prepared to handle this, even though I
am responsible for the introduction of it at 1cfe7733 (git-blame: no
rev means start from the working tree file., 2007-01-30).

Having said that, do we act differently when the index is unmerged
at path in any way?  When path exists but is unmerged, you get
negative index from cache_name_pos(), and IIUC, "blaming of working
tree file" does not care if the index is unmerged.  It creates the
fake working tree file commit, pretends HEAD is its parent, and digs
the lineage from there.

I suspect that the above change needs to be updated further if the
user wants to run "blame path" during a conflicted renaming merge,
i.e.

 0. Before two histories diverged, there was old_path.
 1. Our side updated contents of that file and kept it at old_path.
 2. Their side updated contents of that file and moved it to path.
 3. "git merge" notcied the rename, created three stages at "path",
    with the result of conflicted content-level merge in the working
    tree at "path".
 4. The user edits "path" and resolves the conflict, but wants to
    double check before running "git add path".
 5. "git blame path"

Perhaps something like this should suffice:

    pos = cache_name_pos(path, strlen(path));
    if (0 <= pos)
    	; /* ok */
    else if (!strcmp(active_cache[-1 - pos]->name), path)
        ; /* ok -- just unmerged */
    else
    	die("no such path in HEAD");

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]