Re: Can't find the revelant commit with git-log

["René Scharfe" <rene.scharfe@lsrfire.ath.cx>]

Am 29.01.2011 06:47, schrieb Junio C Hamano:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 
>> Perhaps we should check my underlying assumption first: is it
>> reasonable to expect a git log command to show the same commits
>> with and without a path spec that covers all changed files?
> 
> The simplest case would be "git log ." vs "git log" from the root
> level of the repository, right?  Traditionally, the former is "please
> show _one_ simplest history that can explain how the current commit
> came to be" (i.e. with merge simplification), while the latter is
> "please list everything that is behind the current commit" (i.e.
> without), I think.
> 
> It feels unintuitive, but my understanding of the rationale behind
> the design is that, the expectation Linus had when he first did the
> pathspec limited traversal was that most of the time "git log $path"
> is used to get an explanation.  It follows that having to say "git
> log --simplify $path" would have been a nuisance, so "with pathspec,
> we simplify" was thought to be a reasonable default.

I think simplifying the history whenever a pathspec restricts the set of
interesting commits makes sense.

I'm not so sure about "." vs. none, and it feels odd that the only way
to turn off simplification is to not use pathspecs, as --full-history
will still remove merges if tree_same in revision.c is true.
Simplification by default (even without a pathspec) and --full-history
reporting, well, the full history seems more intuitive to me.

So currently pickaxe can't be used reliable to search for strings that
have been removed: either one has to refrain from using pathspecs, which
is prohibitively slow in the kernel repo, or git is free to take a
short-cut through a branch of history that never contained the string at
all.

Your patch extends --full-history coverage sufficiently to make pickaxe
work in Francis' use case.  One just has to remember to specify this
option if one hunts for removed strings using -S.  Below is an equivalent
patch, only with the simplify_history test moved into the loop and the
needed test script modification.

-- >8 --
Subject: revision: let --full-history keep half-interesting merges

Don't simplify merges away that have at least one parent with changes in
the interesting subset of files if --full-history has been specified.
The previous behaviour hid merges with one uninteresting parent, which
could lead to history that removed code to become undetectable.

E.g., this command run against the Linux kernel repo only found one
merge that brought the specified function in (and the regular commit
which added it in the first place), but missed the 92 merges that
removed it, as well as 67 other merges that added it back:

	$ git log -m --full-history -Sblacklist_iommu \
		v2.6.26..v2.6.29 -- drivers/pci/intel-iommu.c

Reported-by: Francis Moreau <francis.moro@gmail.com>
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 revision.c                                 |    2 ++
 t/t6016-rev-list-graph-simplify-history.sh |    1 +
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/revision.c b/revision.c
index 7b9eaef..84c231b 100644
--- a/revision.c
+++ b/revision.c
@@ -434,6 +434,8 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
 		case REV_TREE_OLD:
 		case REV_TREE_DIFFERENT:
 			tree_changed = 1;
+			if (!revs->simplify_history)
+				return;
 			pp = &parent->next;
 			continue;
 		}
diff --git a/t/t6016-rev-list-graph-simplify-history.sh b/t/t6016-rev-list-graph-simplify-history.sh
index f7181d1..50ffcf4 100755
--- a/t/t6016-rev-list-graph-simplify-history.sh
+++ b/t/t6016-rev-list-graph-simplify-history.sh
@@ -168,6 +168,7 @@ test_expect_success '--graph --full-history --simplify-merges -- bar.txt' '
 	echo "|\\  " >> expected &&
 	echo "| * $C4" >> expected &&
 	echo "* | $A5" >> expected &&
+	echo "* | $A4" >> expected &&
 	echo "* | $A3" >> expected &&
 	echo "|/  " >> expected &&
 	echo "* $A2" >> expected &&
-- 
1.7.3.4