2007-10-15 19:45:39

by Yinghai Lu

[permalink] [raw]
Subject: git/cscope with x86 merge

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


2007-10-15 20:02:33

by Dave Jones

[permalink] [raw]
Subject: Re: git/cscope with x86 merge

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

2007-10-15 20:02:53

by Linus Torvalds

[permalink] [raw]
Subject: Re: git/cscope with x86 merge



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

2007-10-20 09:40:31

by Yinghai Lu

[permalink] [raw]
Subject: Re: git/cscope with x86 merge

On 10/15/07, Linus Torvalds <[email protected]> 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

2007-10-20 15:57:41

by Linus Torvalds

[permalink] [raw]
Subject: Re: git/cscope with x86 merge



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 <[email protected]>
Date: Thu Oct 11 11:17:24 2007 +0200

x86_64: move kernel

Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>

arch/{x86_64 => x86}/kernel/vmlinux_64.lds.S | 0
1 files changed, 0 insertions(+), 0 deletions(-)

commit 13a9cd42466e12113859c4d7b41561e8bbcaf09d
Author: Thomas Gleixner <[email protected]>
Date: Thu Oct 11 11:14:21 2007 +0200

x86_64: prepare shared kernel/vmlinux.lds.S

Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>

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

2007-10-20 16:46:27

by Sam Ravnborg

[permalink] [raw]
Subject: Re: git/cscope with x86 merge

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 <[email protected]>
> Date: Thu Oct 11 11:17:24 2007 +0200
>
> x86_64: move kernel
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Signed-off-by: Ingo Molnar <[email protected]>
>
> arch/{x86_64 => x86}/kernel/vmlinux_64.lds.S | 0
> 1 files changed, 0 insertions(+), 0 deletions(-)
>
> commit 13a9cd42466e12113859c4d7b41561e8bbcaf09d
> Author: Thomas Gleixner <[email protected]>
> Date: Thu Oct 11 11:14:21 2007 +0200
>
> x86_64: prepare shared kernel/vmlinux.lds.S
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Signed-off-by: Ingo Molnar <[email protected]>
>
> 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

2007-10-20 17:40:24

by Thomas Gleixner

[permalink] [raw]
Subject: Re: git/cscope with x86 merge

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

2007-10-20 18:20:43

by Linus Torvalds

[permalink] [raw]
Subject: Re: git/cscope with x86 merge



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

2007-10-20 18:50:32

by Linus Torvalds

[permalink] [raw]
Subject: Re: git/cscope with x86 merge



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)

2007-10-20 19:14:21

by Sam Ravnborg

[permalink] [raw]
Subject: Re: git/cscope with x86 merge

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 <[email protected]>
Date: Thu Oct 11 11:17:24 2007 +0200

x86_64: move kernel

Signed-off-by: Thomas Gleixner <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>

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

2007-10-21 01:38:27

by Linus Torvalds

[permalink] [raw]
Subject: Re: git/cscope with x86 merge



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

2007-10-21 01:38:42

by Linus Torvalds

[permalink] [raw]
Subject: Re: git/cscope with x86 merge



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

2007-10-21 01:59:42

by Andi Kleen

[permalink] [raw]
Subject: Re: git/cscope with x86 merge


> 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

2007-10-21 02:24:52

by Sam Ravnborg

[permalink] [raw]
Subject: Re: git/cscope with x86 merge

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