On Thu, Jun 22, 2017 at 01:32:44PM -0700, Junio C Hamano wrote:
> I do not think command line parser does not allow "log -g
> maint..master" so all the "limited" processing the remainder of
> get_revision_1() does shouldn't matter.
Yeah, I don't think negative endpoints work at all, and "foo...bar"
seems to also break (though with a confusing message). It seems clear to
me that multiple positive endpoints don't work well either, if they have
overlapping commits.
> I however think pathspec will affect simplify_commit() and suspect
> that "git log -g -20 HEAD path" will behave differently. Perhaps
> the difference is "it used to use path in an unexplainable way, now
> it ignores", in which case this is an improvement.
The current behavior there does seem like nonsense, because it's based
on the fake parents. For instance, if I set up a simple two-branch case:
commit() {
echo "$1" >"$1" && git add "$1" && git commit -m "$1"
}
git init repo
cd repo
commit base
commit master
git checkout -b side HEAD^
commit side
git merge --no-edit master
commit combined
Then I get:
$ git log -g --oneline --name-status -- master
f06c3cd HEAD@{1}: merge master: Merge made by the 'recursive' strategy.
5bf12c4 HEAD@{3}: checkout: moving from master to side
dfa408b HEAD@{4}: commit: master
A master
Even though only one of those commits touched master. But with my patch,
it's also somewhat confusing. We ignore the pathspec when picking which
commits to show, but still apply it for diffing. So:
03cf1ad HEAD@{0}: commit: combined
f06c3cd HEAD@{1}: merge master: Merge made by the 'recursive' strategy.
277042b HEAD@{2}: commit: side
5bf12c4 HEAD@{3}: checkout: moving from master to side
dfa408b HEAD@{4}: commit: master
A master
5bf12c4 HEAD@{5}: commit (initial): base
I think we'd want to just omit any entries that are TREESAME to their
parents. We don't actually care about true parent rewriting (since we're
not walking the parents), but if it happened as a side effect that would
probably be OK.
It looks like simplify_commit() is also where we apply bits like
--no-merges, which doesn't work with my patch. It _also_ behaves
nonsensically in the current code, because of course the fake reflog
parents are never merges.
Which really makes me feel like this patch is going in the right
direction, as it makes all of this behave conceptually like:
while read old new etc ...
do
git show $new
done <.git/logs/$ref
which is simple to explain and is what I'd expect (and is certainly the
direction we went with the "diff uses real parents" commit).
We just need to hit the simplify_commit() code path here.
-Peff
