git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
To: Francis Moreau <francis.moro@gmail.com>
Cc: git@vger.kernel.org, Johannes Sixt <j6t@kdbg.org>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: Can't find the revelant commit with git-log
Date: Fri, 28 Jan 2011 21:29:41 +0100	[thread overview]
Message-ID: <4D432735.8000208@lsrfire.ath.cx> (raw)
In-Reply-To: <4D4063EC.7090509@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

  reply	other threads:[~2011-01-28 20:30 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-25  9:01 Can't find the revelant commit with git-log Francis Moreau
2011-01-25 16:12 ` René Scharfe
2011-01-25 17:44   ` Francis Moreau
2011-01-26  8:36     ` Francis Moreau
2011-01-26 10:44       ` Johannes Sixt
2011-01-26 20:56         ` Francis Moreau
2011-01-26 21:03           ` Sverre Rabbelier
2011-01-26 21:08             ` Francis Moreau
2011-01-26 21:14               ` Sverre Rabbelier
2011-01-26 21:31                 ` Francis Moreau
2011-01-26 21:24               ` Junio C Hamano
2011-01-26 21:32                 ` Francis Moreau
2011-01-26 18:11       ` René Scharfe
2011-01-28 20:29         ` René Scharfe [this message]
2011-01-29  0:02           ` Junio C Hamano
2011-01-29  2:34             ` René Scharfe
2011-01-29  5:47               ` Junio C Hamano
2011-01-29 20:26                 ` René Scharfe
2011-02-01 21:28                   ` Junio C Hamano
2011-02-07 22:51                   ` Junio C Hamano
2011-02-10 18:50                     ` René Scharfe
2011-01-29 20:26               ` René Scharfe
2011-01-28 22:01         ` René Scharfe
2011-01-29 12:52           ` Francis Moreau
2011-01-29 13:02             ` René Scharfe
2011-01-29 13:57               ` Francis Moreau
2011-01-29 15:17                 ` René Scharfe
2011-01-26  9:01   ` Francis Moreau
2011-01-26 18:39     ` René Scharfe
2011-01-26 19:50       ` Francis Moreau

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=4D432735.8000208@lsrfire.ath.cx \
    --to=rene.scharfe@lsrfire.ath.cx \
    --cc=francis.moro@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    /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).