git/cscope with x86 merge

git/cscope with x86 merge

[Yinghai Lu]

after the merge:
1. git
git log -p arch/x86/kernel/io_apic_64.c
only can show the log from the merge..., and can not get log before
merge for x86_64/kernel/io_apic.c
Any git update for that?

2. cscope
on x86_64, it is good to see file in arch/x86/pci/*
but will show other _32.c too.
So is it possible to change find-sources to make filter out _32.* and
mach-* dirs?

YH

Re: git/cscope with x86 merge

[Linus Torvalds]

On Mon, 15 Oct 2007, Yinghai Lu wrote:
>
> after the merge:
> 1. git
> git log -p arch/x86/kernel/io_apic_64.c
> only can show the log from the merge..., and can not get log before
> merge for x86_64/kernel/io_apic.c
> Any git update for that?

Use

	git log -p --follow arch/x86/kernel/io_apic_64.c

where the "--follow" tells git to follow renames.

And, of course, "git blame -C" will follow renames and copying of code 
across file boundaries too.

NOTE! In both cases you may actually have to tell git to not limit its 
rename detection when it sees lots of files. You can do that 
once-and-for-all with

	git config --global diff.renamelimit 0

which should take care of it (although it seems that due to unlucky 
timing, the current stable git release does not honor the renamelimit for 
merging, so if you actually need to have git merge data across a rename, 
you should use the current "master" branch of git. Junio is sadly away 
for two weeks right now)

> 2. cscope

No idea on cscope..

		Linus

Re: git/cscope with x86 merge

[Yinghai Lu]

On 10/15/07, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
>
> On Mon, 15 Oct 2007, Yinghai Lu wrote:
> >
> > after the merge:
> > 1. git
> > git log -p arch/x86/kernel/io_apic_64.c
> > only can show the log from the merge..., and can not get log before
> > merge for x86_64/kernel/io_apic.c
> > Any git update for that?
>
> Use
>
>         git log -p --follow arch/x86/kernel/io_apic_64.c
>
> where the "--follow" tells git to follow renames.
>
> And, of course, "git blame -C" will follow renames and copying of code
> across file boundaries too.
>
> NOTE! In both cases you may actually have to tell git to not limit its
> rename detection when it sees lots of files. You can do that
> once-and-for-all with
>
>         git config --global diff.renamelimit 0
>
> which should take care of it (although it seems that due to unlucky
> timing, the current stable git release does not honor the renamelimit for
> merging, so if you actually need to have git merge data across a rename,
> you should use the current "master" branch of git. Junio is sadly away
> for two weeks right now)

git log -p --follow arch/x86/kernel/vmlinux_64.lds.S

can not trace to arch/x86_64/kernel/vmlinux.lds.S

YH

Re: git/cscope with x86 merge

[Linus Torvalds]

On Sat, 20 Oct 2007, Yinghai Lu wrote:
> 
> git log -p --follow arch/x86/kernel/vmlinux_64.lds.S
> 
> can not trace to arch/x86_64/kernel/vmlinux.lds.S

Hmm. I get:

	[torvalds@woody linux]$ git log --stat --follow arch/x86/kernel/vmlinux_64.lds.S
	commit 250c22777fe1ccd7ac588579a6c16db4c0161cc5
	Author: Thomas Gleixner <tglx@linutronix.de>
	Date:   Thu Oct 11 11:17:24 2007 +0200
	
	    x86_64: move kernel
	
	    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
	    Signed-off-by: Ingo Molnar <mingo@elte.hu>
	
	 arch/{x86_64 => x86}/kernel/vmlinux_64.lds.S |    0
	 1 files changed, 0 insertions(+), 0 deletions(-)
	
	commit 13a9cd42466e12113859c4d7b41561e8bbcaf09d
	Author: Thomas Gleixner <tglx@linutronix.de>
	Date:   Thu Oct 11 11:14:21 2007 +0200
	
	    x86_64: prepare shared kernel/vmlinux.lds.S
	
	    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
	    Signed-off-by: Ingo Molnar <mingo@elte.hu>
	
	 arch/x86_64/kernel/vmlinux_64.lds.S |  235 +++++++++++++++++++++++++++++++++++
	 1 files changed, 235 insertions(+), 0 deletions(-)


so it definitely works for me.

But it might be the same old rename limit issue. Try doing

	git config --global diff.renamelimit 0

first,

		Linus

Re: git/cscope with x86 merge

[Sam Ravnborg]

On Sat, Oct 20, 2007 at 08:56:18AM -0700, Linus Torvalds wrote:
> 
> 
> On Sat, 20 Oct 2007, Yinghai Lu wrote:
> > 
> > git log -p --follow arch/x86/kernel/vmlinux_64.lds.S
> > 
> > can not trace to arch/x86_64/kernel/vmlinux.lds.S
> 
> Hmm. I get:
> 
> 	[torvalds@woody linux]$ git log --stat --follow arch/x86/kernel/vmlinux_64.lds.S
> 	commit 250c22777fe1ccd7ac588579a6c16db4c0161cc5
> 	Author: Thomas Gleixner <tglx@linutronix.de>
> 	Date:   Thu Oct 11 11:17:24 2007 +0200
> 	
> 	    x86_64: move kernel
> 	
> 	    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> 	    Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 	
> 	 arch/{x86_64 => x86}/kernel/vmlinux_64.lds.S |    0
> 	 1 files changed, 0 insertions(+), 0 deletions(-)
> 	
> 	commit 13a9cd42466e12113859c4d7b41561e8bbcaf09d
> 	Author: Thomas Gleixner <tglx@linutronix.de>
> 	Date:   Thu Oct 11 11:14:21 2007 +0200
> 	
> 	    x86_64: prepare shared kernel/vmlinux.lds.S
> 	
> 	    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> 	    Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 	
> 	 arch/x86_64/kernel/vmlinux_64.lds.S |  235 +++++++++++++++++++++++++++++++++++
> 	 1 files changed, 235 insertions(+), 0 deletions(-)
> 
> 
> so it definitely works for me.

But you do not see the rename arch/x86_64/kernel/{vmlinux.lds.S => vmlinux.lds.S}
And this is I thing the important step here.

For vmlinux.lds.S we have two renames.
First the vmlinux.lds.S => vmlinux_64.lds.S
But this may be a copy instead of a rename thus preventing us
to see the old history of vmlinux.lds.S?

	Sam

Re: git/cscope with x86 merge

[Thomas Gleixner]

On Sat, 20 Oct 2007, Sam Ravnborg wrote:
> On Sat, Oct 20, 2007 at 08:56:18AM -0700, Linus Torvalds wrote:
> > so it definitely works for me.
> 
> But you do not see the rename arch/x86_64/kernel/{vmlinux.lds.S => vmlinux.lds.S}
> And this is I thing the important step here.
> 
> For vmlinux.lds.S we have two renames.
> First the vmlinux.lds.S => vmlinux_64.lds.S
> But this may be a copy instead of a rename thus preventing us
> to see the old history of vmlinux.lds.S?

Yes, it's a copy. vmlinux.lds.S is one of the few source files, which
needed a stub, which includes the 32/64 bit one to avoid a complete
mess in tons of Kbuild/Makefiles.

     tglx

Re: git/cscope with x86 merge

[Linus Torvalds]

On Sat, 20 Oct 2007, Sam Ravnborg wrote:
> 
> But you do not see the rename arch/x86_64/kernel/{vmlinux.lds.S => vmlinux.lds.S}

Umm. What you are describing isn't a rename - that's the same name. Do you 
perhaps mean vmlinux.lds.S => vmlinux_64.lds.S ?

And yes, it doesn't show that as a rename, because of the fact that the

	arch/x86_64/kernel/vmlinux.lds.S

file actually *remained*, so it wasn't really a rename. It just got almost 
all of its data changed.

So there was never really a rename: there was a "copy" and a "rewrite". 
And "git --follow" doesn't follow copies.

However, "git blame" does do so. So if you do

	git blame -C arch/x86/kernel/vmlinux_64.lds.S

(where that -C tells it to follow data across file copies), it will 
actually show the history down, line for line!

But when you try to follow the history of the whole *file*, git sees that 
the filename still existed of the source, so it won't consider that a 
candidate for renames!

I could perhaps look at making "git log --follow" also break up files that 
got totally rewritten (git already has a notion of "-B" to do that), but 
no, we don't do it right now. (But one of the advantages of the git model 
is that none of this is hardcoded in the repository data itself, so we can 
improve the rename following and it will automatically work with any repo, 
even one created with older git versions)

		Linus

Re: git/cscope with x86 merge

[Linus Torvalds]

On Sat, 20 Oct 2007, Linus Torvalds wrote:
> 
> I could perhaps look at making "git log --follow" also break up files that 
> got totally rewritten (git already has a notion of "-B" to do that), but 
> no, we don't do it right now. 

Ok, if you guys have a current git source, and want to try something out, 
this fairly small patch does this.

As mentioned, git already supports the notion of "try to break files with 
the same name if the contents are too dissimilar". In other words, even if 
a file exists under the same name in both the old revision and in the 
newer one, we'll look at just how big the changes are, and if git decides 
that it looks like the whole file was rewritten, then git will split up 
the diff into a "delete old contents" and "create new contents". That then 
allows it to consider the file for rename detection.

(The rename detection may, of course, decide that the original file was 
the best source after all.. ;)

However, "git log --follow" didn't ever actually enable that for the logic 
that tries to figure out where a file came from, so you would only see 
this when generating the diffs, never in the file history logic. That's 
an easy one-liner to tree-diff.c: try_to_follow_renames().

However, when I actually tried it, it turns out that the break logic was 
broken - nobody has ever really depended on it. So while it was a 
one-liner to make "git log --follow" understand to break files that seem 
to be totally rewritten, it still didn't actually work, because the 
changes that totally rewrote vmlinux.lds.S wouldn't trigger the break 
logic.

So most of this (still fairly small patch) is just fixing the break logic 
in diffcore-break.c:should_break().

It hasn't gotten a lot of testing, but it does actually improve other 
cases too, so I think this is the right thign to do. I'll bring it up on 
the git lists.

Oh, and with this patch, the "break same filename" is still off by 
default. You need to do

	git log --follow -B arch/x86/kernel/vmlinux_64.lds.S

to enable the file-rewritten-so-break-associations detection. I suspect 
it makes sense to enable -B by default when using --follow (--follow 
already obviously implies rename detection), but that's a separate and 
independent issue.

		Linus

----
 diffcore-break.c |   11 +++++++----
 tree-diff.c      |    1 +
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/diffcore-break.c b/diffcore-break.c
index ae8a7d0..c71a226 100644
--- a/diffcore-break.c
+++ b/diffcore-break.c
@@ -45,8 +45,8 @@ static int should_break(struct diff_filespec *src,
 	 * The value we return is 1 if we want the pair to be broken,
 	 * or 0 if we do not.
 	 */
-	unsigned long delta_size, base_size, src_copied, literal_added,
-		src_removed;
+	unsigned long delta_size, base_size, max_size;
+	unsigned long src_copied, literal_added, src_removed;
 
 	*merge_score_p = 0; /* assume no deletion --- "do not break"
 			     * is the default.
@@ -63,7 +63,8 @@ static int should_break(struct diff_filespec *src,
 		return 0; /* error but caught downstream */
 
 	base_size = ((src->size < dst->size) ? src->size : dst->size);
-	if (base_size < MINIMUM_BREAK_SIZE)
+	max_size = ((src->size > dst->size) ? src->size : dst->size);
+	if (max_size < MINIMUM_BREAK_SIZE)
 		return 0; /* we do not break too small filepair */
 
 	if (diffcore_count_changes(src, dst,
@@ -89,12 +90,14 @@ static int should_break(struct diff_filespec *src,
 	 * less than the minimum, after rename/copy runs.
 	 */
 	*merge_score_p = (int)(src_removed * MAX_SCORE / src->size);
+	if (*merge_score_p > break_score)
+		return 1;
 
 	/* Extent of damage, which counts both inserts and
 	 * deletes.
 	 */
 	delta_size = src_removed + literal_added;
-	if (delta_size * MAX_SCORE / base_size < break_score)
+	if (delta_size * MAX_SCORE / max_size < break_score)
 		return 0;
 
 	/* If you removed a lot without adding new material, that is
diff --git a/tree-diff.c b/tree-diff.c
index 26bdbdd..7c261fd 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -319,6 +319,7 @@ static void try_to_follow_renames(struct tree_desc *t1, struct tree_desc *t2, co
 	diff_opts.detect_rename = DIFF_DETECT_RENAME;
 	diff_opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_opts.single_follow = opt->paths[0];
+	diff_opts.break_opt = opt->break_opt;
 	paths[0] = NULL;
 	diff_tree_setup_paths(paths, &diff_opts);
 	if (diff_setup_done(&diff_opts) < 0)

Re: git/cscope with x86 merge

[Sam Ravnborg]

On Sat, Oct 20, 2007 at 11:49:51AM -0700, Linus Torvalds wrote:
> 
> 
> On Sat, 20 Oct 2007, Linus Torvalds wrote:
> > 
> > I could perhaps look at making "git log --follow" also break up files that 
> > got totally rewritten (git already has a notion of "-B" to do that), but 
> > no, we don't do it right now. 
> 
> Ok, if you guys have a current git source, and want to try something out, 
> this fairly small patch does this.

I pulled next branch of git and applied your patch.

When running
 	git log --follow -B arch/x86/kernel/vmlinux_64.lds.S

I got no output at all (in a newly pulled linux kernel dir).
When I ran
 	git log arch/x86/kernel/vmlinux_64.lds.S
I got:

commit 250c22777fe1ccd7ac588579a6c16db4c0161cc5
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Thu Oct 11 11:17:24 2007 +0200

    x86_64: move kernel
    
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Signed-off-by: Ingo Molnar <mingo@elte.hu>

With -B alone I got same output.
When I add --follow I get no output.

I could not get back to my previous git binary - I replaced it
with the new one.

cat ~/.gitconfig

[diff]
	renamelimit = 0


	Sam

Re: git/cscope with x86 merge

[Linus Torvalds]

On Sat, 20 Oct 2007, Sam Ravnborg wrote:
> 
> I pulled next branch of git and applied your patch.
> 
> When running
>  	git log --follow -B arch/x86/kernel/vmlinux_64.lds.S
> 
> I got no output at all (in a newly pulled linux kernel dir).

Try with "-p".

It's possible (nay, likely) that "next" has the bug where "--follow" 
without a patch generating thing (-p or --stat or one of the other flags 
that enable diffs) doesn't work at all.

It should be fixed in "master".

Sadly, Junio has been away for personal reasons for two weeks, so we 
haven't had that known bug fixed in the regular git tree. Shawn Pearce 
maintains a repo with known fixes at "git://repo.or.cz/git/spearce.git" in 
the meantime.

			Linus

Re: git/cscope with x86 merge

[Sam Ravnborg]

On Sat, Oct 20, 2007 at 12:36:17PM -0700, Linus Torvalds wrote:
> 
> 
> On Sat, 20 Oct 2007, Sam Ravnborg wrote:
> > 
> > I pulled next branch of git and applied your patch.
> > 
> > When running
> >  	git log --follow -B arch/x86/kernel/vmlinux_64.lds.S
> > 
> > I got no output at all (in a newly pulled linux kernel dir).
> 
> Try with "-p".
> 
> It's possible (nay, likely) that "next" has the bug where "--follow" 
> without a patch generating thing (-p or --stat or one of the other flags 
> that enable diffs) doesn't work at all.

Yep - that did the trick, thanks.

	Sam

Re: git/cscope with x86 merge

[Andi Kleen]

> Try with "-p".
> 
> It's possible (nay, likely) that "next" has the bug where "--follow" 
> without a patch generating thing (-p or --stat or one of the other flags 
> that enable diffs) doesn't work at all.

It's not only next. The latest release (1.5.3.4) has this problem.

-Andi

Re: git/cscope with x86 merge

[Linus Torvalds]

On Sat, 20 Oct 2007, Andi Kleen wrote:
> 
> It's not only next. The latest release (1.5.3.4) has this problem.

Yes. It's ok in the master branch, but due to unlucky timing with noticing 
this bug, and Junio being away, no releases got cut with the fix. 

In general, the kernel people haven't done many renames (I think it's 
something we have avoided historically, just because it makes diffs almost 
totally unreadable - even while git supports "rename diffs", they are 
turned off by default just to be compatible with people who use quilt and 
raw patch).

So we've basically had a "perfect storm" of (a) Junio being away, (b) a 
really stupid bug that got introduced recently, (c) a few broken 
heuristics that had little testing because the kernel almost never does 
renames anyway and (d) suddenly lots of renames in the kernel all at once.

(It's not even just the x86 stuff - we had all the watchdog drivers being 
moved around too, so we probably had almost as many renames the last two 
weeks than we've had in the two years preceding it ;)

[ Just for fun, I checked. Yup. In the last two weeks, git finds 1120 
  copies and renames. Over the last two and a half years (ie the whole git 
  timeframe), we have 2136 of them. So we really have done more renames in 
  the last two weeks than we have over the whole rest of the git history,
  and it wasn't just my imagination ]

			Linus

Re: git/cscope with x86 merge

[Dave Jones]

On Mon, Oct 15, 2007 at 12:45:27PM -0700, Yinghai Lu wrote:
 > after the merge:
 > 1. git
 > git log -p arch/x86/kernel/io_apic_64.c
 > only can show the log from the merge..., and can not get log before
 > merge for x86_64/kernel/io_apic.c
 > Any git update for that?

add --follow to your command line

 > 2. cscope
 > on x86_64, it is good to see file in arch/x86/pci/*
 > but will show other _32.c too.
 > So is it possible to change find-sources to make filter out _32.* and
 > mach-* dirs?
 
I see this as a positive thing personally.  You could regenerate the
cscope files by hand. Or maybe introduce a 'cscope_minimal' Makefile
target perhaps.

	Dave

-- 
http://www.codemonkey.org.uk