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

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

Am 26.01.2011 19:11, schrieb René Scharfe:
> So far we have two action items, I think:
> 
> - Make git grep report non-matching path specs (new feature).
> 
> - Find out why removing the last path component made a difference in
> the case above (looks like a bug, but I don't understand what's going
> on).

OK, regarding the second point:

Merges that have at least one parent without changes in the selected
subset of files won't be displayed, not even with --full-history.  That
explains why removing the last path component made a difference: all the
merges ended up with a version of the file that matched one of their
parents, but there were other changes in the directory.

This is a feature: since the version of the file picked by the merge
must have been introduced by an earlier commit (a regular one,
presumably), you'll find it there anyway.

And this history simplification takes precedence over pickaxe (-S).

The patch below turns down its aggressiveness when the pickaxe is swung
at the same time.  Here's what it does to your use case:

	$ revs="v2.6.26..v2.6.29"
	$ opts="-Sblacklist_iommu --oneline -m --full-history"

	# This takes quite a while...
	$ git log $opts $revs | wc -l
	160

	# Without the patch:
	$ git log $opts $revs -- drivers/pci/intel-iommu.c | wc -l
	2

	# With the patch (really just its first hunk):
	$ git log $opts $revs -- drivers/pci/intel-iommu.c | wc -l
	160

	$ opts="-Sblacklist_iommu --oneline -m"

	# This takes quite a while...
	$ git log $opts $revs | wc -l
	160

	# Without the patch:
	$ git log $opts $revs -- drivers/pci/intel-iommu.c | wc -l
	0

	# With the patch:
	$ git log $opts $revs -- drivers/pci/intel-iommu.c | wc -l
	160

	$ opts="-Sblacklist_iommu --oneline"

	# This takes a bit, but not too long.
	$ git log $opts $revs | wc -l
	1

	# Without the patch:
	$ git log $opts $revs -- drivers/pci/intel-iommu.c | wc -l
	0

	# With the patch:
	$ git log $opts $revs -- drivers/pci/intel-iommu.c | wc -l
	1

The full output matches exactly if the number of lines match.  That's to
be expected, as the string "blacklist_iommu" only ever appears in the
file drivers/pci/intel-iommu.c.

It wasn't mentioned before v2.6.26 or after v2.6.29.

There is only one regular commit, namely the initial one that introduced
the function.  Some merges are reported more than once, each for every
parent where -S hit.  135 unique commits are reported.

-- >8 --
Subject: pickaxe: don't simplify history too much

If pickaxe is used, turn off history simplification and make sure to keep
merges with at least one interesting parent.

If path specs are used, merges that have at least one parent whose files
match those in the specified subset are edited out.  This is good in
general, but leads to unexpectedly few results if used together with
pickaxe.  Merges that also have an interesting parent (in terms of -S or
-G) are dropped, too.

This change makes sure pickaxe takes precedence over history
simplification.  This means path specs won't change the results as long
as they contain all the files that pickaxe turns up.  E.g. these two
commands now report the same single commit that added the function
blacklist_iommu to the specified file in the Linux kernel repo:

   $ git log -Sblacklist_iommu v2.6.26..v2.6.29 --
   $ git log -Sblacklist_iommu v2.6.26..v2.6.29 -- drivers/pci/intel-iommu.c

Previously the second one came up empty.

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

diff --git a/revision.c b/revision.c
index 7b9eaef..cacf60c 100644
--- a/revision.c
+++ b/revision.c
@@ -441,6 +441,8 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)
 	}
 	if (tree_changed && !tree_same)
 		return;
+	if (tree_changed && revs->diffopt.pickaxe)
+		return;
 	commit->object.flags |= TREESAME;
 }
 
@@ -1647,6 +1649,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	    revs->diffopt.filter ||
 	    DIFF_OPT_TST(&revs->diffopt, FOLLOW_RENAMES))
 		revs->diff = 1;
+
+	if (revs->diffopt.pickaxe)
+		revs->simplify_history = 0;
 
 	if (revs->topo_order)
 		revs->limited = 1;
diff --git a/t/t6012-rev-list-simplify.sh b/t/t6012-rev-list-simplify.sh
index af34a1e..b4fb8d0 100755
--- a/t/t6012-rev-list-simplify.sh
+++ b/t/t6012-rev-list-simplify.sh
@@ -86,5 +86,7 @@ check_result 'I H E C B A' --full-history --date-order -- file
 check_result 'I E C B A' --simplify-merges -- file
 check_result 'I B A' -- file
 check_result 'I B A' --topo-order -- file
+check_result 'I C B' -SHello
+check_result 'I C B' -SHello -- file
 
 test_done
-- 
1.7.3.4