From: Mike Hommey <mh@glandium.org>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] blame: Allow to blame paths freshly added to the index
Date: Sat, 16 Jul 2016 08:16:56 +0900 [thread overview]
Message-ID: <20160715231656.iztvtftk5ttrzeow@glandium.org> (raw)
In-Reply-To: <xmqqr3auli2x.fsf@gitster.mtv.corp.google.com>
> 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?
It should, but that'd be copy/move detection, wouldn't it? :)
> > 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");
>
> I do not think the "conflicted renaming merge" example would not be
> a problem in practice iff "git merge" was used, because the fake
> working tree commit would look at both our tree and their tree, and
> find "path" in theirs inside the loop above this "die".
More than that, it seems that's a case that already works without the
patch (it doesn't complain that "no such path in HEAD", although it
only identifies the "theirs" part of the merge conflict when blaming
the file straight out of the merge failure, but that'd be a separate
issue.
> But the user can be in the same conflicted rename situation with
> "git am -3" or cherry-pick, and in these cases there won't be extra
> parent commits for the fake work tree commit, hence the conclusion
> does not change.
Indeed, with cherry-pick, the "no such path in HEAD" error is happening
with the patch.
Mike
next prev parent reply other threads:[~2016-07-15 23:17 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-15 2:42 [PATCH] blame: Allow to blame paths freshly added to the index Mike Hommey
2016-07-15 10:45 ` Johannes Schindelin
2016-07-15 12:32 ` Mike Hommey
2016-07-15 12:37 ` Jeff King
2016-07-15 12:42 ` Mike Hommey
2016-07-15 12:55 ` [PATCH v2] " Mike Hommey
2016-07-15 15:28 ` Johannes Schindelin
2016-07-15 20:58 ` [PATCH] " Junio C Hamano
2016-07-15 21:14 ` Junio C Hamano
2016-07-15 23:16 ` Mike Hommey [this message]
2016-07-18 18:49 ` Junio C Hamano
2016-07-15 23:23 ` [PATCH v3 1/2] " Mike Hommey
2016-07-15 23:23 ` [PATCH v3 2/2] t/t8003-blame-corner-cases.sh: Use here documents Mike Hommey
2016-07-16 13:05 ` Johannes Schindelin
2016-07-18 18:52 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160715231656.iztvtftk5ttrzeow@glandium.org \
--to=mh@glandium.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).