Git development
 help / color / mirror / Atom feed
* Re: [PATCH v3 3/4] t7800: modernize tests
From: Johannes Sixt @ 2013-03-21  7:41 UTC (permalink / raw)
  To: David Aguilar; +Cc: Sitaram Chamarty, Junio C Hamano, git, Jonathan Nieder
In-Reply-To: <CAJDDKr4O-OU57j0f7F2E2_4EvtCTE1gFP5uLv+fvD6VcpPiifg@mail.gmail.com>

Am 3/20/2013 23:59, schrieb David Aguilar:
> I started digging in and the @worktree_files (aka @worktree above)
> is populated from the output of "git diff --raw ...".
> 
> Seeing the "output" filename in "diff --raw" implies that one of the
> tests added "output" to the index somehow.  I do not see that
> happening anywhere, though, so I do not know how it would end up in
> the @worktree array if it is not reported by "diff --raw".
> 
> 
> My current understanding of how it could possibly be open twice:
> 
> 1. via the >output redirect
> 2. via the copy() perl code which is fed by @worktree
> 
> So I'm confused.  Why would we get different results on Windows?

I tracked down the difference between Windows and Linux, and it is...

	for my $file (@worktree) {
		next if $symlinks && -l "$b/$file";

... this line in sub dir_diff. On Linux, we take the short-cut, but on
Windows we proceed through the rest of the loop, which ultimately finds a
difference here:

		my $diff = compare("$b/$file", "$workdir/$file");

and attempts to copy a file here:

			copy("$b/$file", "$workdir/$file") or

where one of the files is the locked "output" file.

I don't know how essential symlinks are for the operation of git-difftool
and whether something can be done about it. The immediate fix is
apparently to protect the tests with SYMLINKS.

-- Hannes

^ permalink raw reply

* Re: Bug? git show behaves incorrectly when called on a stash object
From: Kirill Likhodedov @ 2013-03-21  8:10 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <vpqsj3qoyj2.fsf@grenoble-inp.fr>


On 20.03.2013, at 19:56, Matthieu Moy wrote:

> stash objects are commits with 2 parents (ie. merge commits). One commit
> is the HEAD you stashed from, and the other is the saved state of the
> index.
...
> Use git show --first-parent --name-status, it should do what you expect

Thanks a lot for clarifying that. It is actually mentioned in git help stash, but I've missed this.
With "--first-parent" I get the expected result. Thanks for your help.

> I'm not sure from the doc what the semantics of --name-status is
> for merge commits, but it seems it shows only files modified in both
> parents.

I think I've found the reason of this behavior.
git help show mentiones that:
> It also presents the merge commit in a special
> format as produced by git diff-tree --cc.
And git help diff-tree says that --cc implies -c, while -c "lists only files which were modified from all parents".

-- Kirill.

^ permalink raw reply

* [BUG] git send-email and SSL_verify_mode
From: Ramkumar Ramachandra @ 2013-03-21  9:48 UTC (permalink / raw)
  To: Git List

Hi,

With recent (I'm how recent exactly) versions of IO::Socket::SSL, git
send-email:1169 spits out the following warning, which originates in
Net::SMTP::SSL:

*******************************************************************
 Using the default of SSL_verify_mode of SSL_VERIFY_NONE for client
 is depreciated! Please set SSL_verify_mode to SSL_VERIFY_PEER
 together with SSL_ca_file|SSL_ca_path for verification.
 If you really don't want to verify the certificate and keep the
 connection open to Man-In-The-Middle attacks please set
 SSL_verify_mode explicitly to SSL_VERIFY_NONE in your application.
*******************************************************************

Nevertheless, it does manage to send the email successfully.  I'm not
sure how to fix this bug, since my Perl-foo is weak.  Please advise.

Thanks.

Ram

^ permalink raw reply

* Re: [RFC] Add posibility to preload stat information.
From: Thomas Rast @ 2013-03-21 10:41 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Fredrik Gustafsson, spearce, git, pclouds
In-Reply-To: <20130320174759.GA29349@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Wed, Mar 20, 2013 at 10:15:39AM -0700, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > So maybe just run "git status >/dev/null"?
>> 
>> In the background?  How often would it run?  I do not think a single
>> lockfile solves anything.  It may prevent simultaneous runs of two
>> such "prime the well" processes, but the same user may be working in
>> two separate repositories.
>
> Yes, in the background (he invokes __git_recursive_stat already in the
> background). I'd think you would want to run it whenever you enter a
> repository.
>
>> I do not see anything that prevents it from running in the same
>> repository over and over again, either.  "prompt" is a bad place to
>> do this kind of thing.
>
> Yeah, I did not look closely at that. The commit message claims "When
> entering a git working dir", but the implementation runs it on each
> prompt invocation, which is awful. I think you'd want to check to use
> rev-parse to see if you have changed into a new git repo, and only run
> it once then.

I think it would actually be a somewhat interesting feature if it
interacted with GIT_PS1_SHOW*.  If you use these settings (I personally
use SHOWDIRTYSTATE but not SHOWUNTRACKEDFILES), the prompt hangs while
__git_ps1 runs git-status.  It should be possible to run a git-status
process in the background when entering a repository, and displaying
some marker ('??' maybe) in the prompt instead of the dirty-state info
until git-status has finished.  That way the user doesn't have his shell
blocked by cding to a big repo.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* [PATCH 0/4] drop some "int x = x" hacks to silence gcc warnings
From: Jeff King @ 2013-03-21 11:03 UTC (permalink / raw)
  To: git

I was fooling around with clang and noticed that it complains about the
"int x = x" construct under -Wall. That is IMHO a deficiency in clang,
since the idiom has a well-defined use in silencing -Wuninitialized
warnings. But I've also always been nervous about the idiom, because
it's easy to get the analysis wrong (after all, the compiler gets these
cases wrong because they're complex), and it's possible for the code to
change later, introducing a new problem.

So I investigated our uses of the idiom. Many of them are correct and
still necessary. Some are correct but no longer necessary with modern
gcc. And some are technically correct, but the code and its assumptions
can be made clearer (to both a reader and the compiler) with a simple
rewrite. Patches are below for the latter two types.

Note that none of these fixes an actual bug in the current code; this is
purely maintenance hygiene. Nor do any of the patches depend on each
other; we can drop any of them that do not look they are providing a net
benefit.

  [1/4]: wt-status: fix possible use of uninitialized variable
  [2/4]: fast-import: use pointer-to-pointer to keep list tail
  [3/4]: drop some obsolete "x = x" compiler warning hacks
  [4/4]: transport: drop "int cmp = cmp" hack

-Peff

^ permalink raw reply

* [PATCH 1/4] wt-status: fix possible use of uninitialized variable
From: Jeff King @ 2013-03-21 11:05 UTC (permalink / raw)
  To: git
In-Reply-To: <20130321110338.GA18552@sigill.intra.peff.net>

In wt_status_print_change_data, we accept a change_type flag
that is meant to be either WT_STATUS_UPDATED or
WT_STATUS_CHANGED.  We then switch() on this value to set
the local variable "status" for each case, but do not
provide a fallback "default" label to the switch statement.

As a result, the compiler realizes that "status" might be
unset, and complains with a warning. To silence this
warning, we use the "int status = status" trick.  This is
correct with the current code, as all callers provide one of
the two expected change_type flags. However, it's also a
maintenance trap, as there is nothing to prevent future
callers from passing another flag, nor to document this
assumption.

Instead of using the "x = x" hack, let's handle the default
case in the switch() statement with a die("BUG"). That tells
the compiler and any readers of the code exactly what the
function's input assumptions are.

We could also convert the flag to an enum, which would
provide a compile-time check on the function input. However,
since these flags are part of a larger enum, that would make
the code unnecessarily complex (we would have to make a new
enum with just the two flags, and then convert it to the old
enum for passing to sub-functions).

Signed-off-by: Jeff King <peff@peff.net>
---
 wt-status.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/wt-status.c b/wt-status.c
index ef405d0..7555817 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -264,7 +264,7 @@ static void wt_status_print_change_data(struct wt_status *s,
 {
 	struct wt_status_change_data *d = it->util;
 	const char *c = color(change_type, s);
-	int status = status;
+	int status;
 	char *one_name;
 	char *two_name;
 	const char *one, *two;
@@ -292,6 +292,9 @@ static void wt_status_print_change_data(struct wt_status *s,
 		}
 		status = d->worktree_status;
 		break;
+	default:
+		die("BUG: unhandled change_type %d in wt_status_print_change_data",
+		    change_type);
 	}
 
 	one = quote_path(one_name, -1, &onebuf, s->prefix);
-- 
1.8.2.rc2.8.g2161951

^ permalink raw reply related

* [PATCH 2/4] fast-import: use pointer-to-pointer to keep list tail
From: Jeff King @ 2013-03-21 11:08 UTC (permalink / raw)
  To: git
In-Reply-To: <20130321110338.GA18552@sigill.intra.peff.net>

This is shorter, idiomatic, and it means the compiler does
not get confused about whether our "e" pointer is valid,
letting us drop the "e = e" hack.

Signed-off-by: Jeff King <peff@peff.net>
---
And it fixes an instance of Linus's "people do not understand pointers"
from here:

  http://meta.slashdot.org/story/12/10/11/0030249/linus-torvalds-answers-your-questions

 fast-import.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index c2a814e..583a439 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2613,7 +2613,7 @@ static struct hash_list *parse_merge(unsigned int *count)
 
 static struct hash_list *parse_merge(unsigned int *count)
 {
-	struct hash_list *list = NULL, *n, *e = e;
+	struct hash_list *list = NULL, **tail = &list, *n;
 	const char *from;
 	struct branch *s;
 
@@ -2641,11 +2641,9 @@ static struct hash_list *parse_merge(unsigned int *count)
 			die("Invalid ref name or SHA1 expression: %s", from);
 
 		n->next = NULL;
-		if (list)
-			e->next = n;
-		else
-			list = n;
-		e = n;
+		*tail = n;
+		tail = &n->next;
+
 		(*count)++;
 		read_next_command();
 	}
-- 
1.8.2.rc2.8.g2161951

^ permalink raw reply related

* [PATCH 3/4] drop some obsolete "x = x" compiler warning hacks
From: Jeff King @ 2013-03-21 11:10 UTC (permalink / raw)
  To: git
In-Reply-To: <20130321110338.GA18552@sigill.intra.peff.net>

In cases where the setting and access of a variable are
protected by the same conditional flag, older versions of
gcc would generate a "might be used unitialized" warning. We
silence the warning by initializing the variable to itself,
a hack that gcc recognizes.

Modern versions of gcc are smart enough to get this right,
going back to at least version 4.3.5. gcc 4.1 does get it
wrong in both cases, but is sufficiently old that we
probably don't need to care about it anymore.

Signed-off-by: Jeff King <peff@peff.net>
---
gcc 4.2 is conspicuously missing because no current Debian system even
has a backwards-compatibility package for it, making it harder to test.
And 4.3 was old enough for me to say "I do not care if you can run with
-Wall -Werror or not", let alone 4.2.

 builtin/cat-file.c | 2 +-
 fast-import.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 00528dd..ad29000 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -193,7 +193,7 @@ static int batch_one_object(const char *obj_name, int print_contents)
 	unsigned char sha1[20];
 	enum object_type type = 0;
 	unsigned long size;
-	void *contents = contents;
+	void *contents;
 
 	if (!obj_name)
 	   return 1;
diff --git a/fast-import.c b/fast-import.c
index 583a439..e12a8b8 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2434,7 +2434,7 @@ static void note_change_n(struct branch *b, unsigned char *old_fanout)
 {
 	const char *p = command_buf.buf + 2;
 	static struct strbuf uq = STRBUF_INIT;
-	struct object_entry *oe = oe;
+	struct object_entry *oe;
 	struct branch *s;
 	unsigned char sha1[20], commit_sha1[20];
 	char path[60];
-- 
1.8.2.rc2.8.g2161951

^ permalink raw reply related

* [PATCH 4/4] transport: drop "int cmp = cmp" hack
From: Jeff King @ 2013-03-21 11:13 UTC (permalink / raw)
  To: git
In-Reply-To: <20130321110338.GA18552@sigill.intra.peff.net>

According to 47ec794, this initialization is meant to
squelch an erroneous uninitialized variable warning from gcc
4.0.1.  That version is quite old at this point, and gcc 4.1
and up handle it fine, with one exception. There seems to be
a regression in gcc 4.6.3, which produces the warning;
however, gcc versions 4.4.7 and 4.7.2 do not.

Signed-off-by: Jeff King <peff@peff.net>
---
We probably _don't_ want to apply this one right now. The regression in
4.6 means some people on reasonably modern systems probably would still
see the warning. Debian stable ships with 4.4, and testing/unstable
defaults to 4.7 (though you can install a gcc-4.6 compatibility
package). But I have no clue if other distros made releases with 4.6.

 transport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/transport.c b/transport.c
index 886ffd8..87b8f14 100644
--- a/transport.c
+++ b/transport.c
@@ -106,7 +106,7 @@ static void insert_packed_refs(const char *packed_refs, struct ref **list)
 		return;
 
 	for (;;) {
-		int cmp = cmp, len;
+		int cmp, len;
 
 		if (!fgets(buffer, sizeof(buffer), f)) {
 			fclose(f);
-- 
1.8.2.rc2.8.g2161951

^ permalink raw reply related

* Re: [PATCH 0/4] drop some "int x = x" hacks to silence gcc warnings
From: Johannes Sixt @ 2013-03-21 11:45 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20130321110338.GA18552@sigill.intra.peff.net>

Am 3/21/2013 12:03, schrieb Jeff King:
> I was fooling around with clang and noticed that it complains about the
> "int x = x" construct under -Wall. That is IMHO a deficiency in clang,
> since the idiom has a well-defined use in silencing -Wuninitialized
> warnings.

IMO, that's a myth. The construct invokes undefined behavior at least
since C99, and the compilers are right to complain about it.

But you might just say that standards are not worth the paper they are
printed on, and you may possibly be right for practical reasons. But I
still consider it a myth that "int x = x" is an idiom. I'm in the C
business since more than 25 years, and the first time I saw the "idiom"
was in git code. Is there any evidence that the construct is used
elsewhere? Have I been in the wrong corner of the C world for such a long
time?

-- Hannes

^ permalink raw reply

* Re: [PATCH 0/4] drop some "int x = x" hacks to silence gcc warnings
From: Jeff King @ 2013-03-21 11:55 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Junio C Hamano, git
In-Reply-To: <514AF2E1.7020409@viscovery.net>

On Thu, Mar 21, 2013 at 12:45:37PM +0100, Johannes Sixt wrote:

> Am 3/21/2013 12:03, schrieb Jeff King:
> > I was fooling around with clang and noticed that it complains about the
> > "int x = x" construct under -Wall. That is IMHO a deficiency in clang,
> > since the idiom has a well-defined use in silencing -Wuninitialized
> > warnings.
> 
> IMO, that's a myth. The construct invokes undefined behavior at least
> since C99, and the compilers are right to complain about it.

While undefined behavior does leave the compiler free to do anything,
including nasal demons, it would be a very poor implementation that did
anything except leave random bytes in the value. And it also means that
gcc is free to take it as a hint to silence the warning; given that
clang tries to be compatible with gcc, I'd think it would want to do the
same. But I may be wrong that the behavior from gcc is intentional or
common (see below).

> But you might just say that standards are not worth the paper they are
> printed on, and you may possibly be right for practical reasons. But I
> still consider it a myth that "int x = x" is an idiom. I'm in the C
> business since more than 25 years, and the first time I saw the "idiom"
> was in git code. Is there any evidence that the construct is used
> elsewhere? Have I been in the wrong corner of the C world for such a long
> time?

Git code was my introduction to it, too, and I was led to believe it was
idiomatic, so I can't speak further on that. I think it was Junio who
introduced me to it, so maybe he can shed more light on the history.

-Peff

^ permalink raw reply

* Re: Which file is older in history?
From: Ramkumar Ramachandra @ 2013-03-21 11:59 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7vobeddcjq.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> I want to compare two files in a git repository and tell which one was
>> introduced into the repository earlier, assuming that they're in the
>> same history line (by ancestry, not timestamp).  The naive way to do
>> this is to find the individual commits that introduced the files
>> (`rev-list HEAD -- <filename> | tail -n 1`), and...
>
> This must be a trick question but the naïve way I think of is
>
>         git log --diff-filter=A -- path1 path2

Thanks, I didn't know about --diff-filter.  I'll need one extra step
to figure out which commit corresponds to the introduction of which
file, no?

^ permalink raw reply

* Re: Which file is older in history?
From: Jeff King @ 2013-03-21 12:21 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Junio C Hamano, Git List
In-Reply-To: <CALkWK0nRgzNYViKSx97OyHVOQmFN61Ex9EUvr1JK-0=GX=uX0w@mail.gmail.com>

On Thu, Mar 21, 2013 at 05:29:39PM +0530, Ramkumar Ramachandra wrote:

> > This must be a trick question but the naïve way I think of is
> >
> >         git log --diff-filter=A -- path1 path2
> 
> Thanks, I didn't know about --diff-filter.  I'll need one extra step
> to figure out which commit corresponds to the introduction of which
> file, no?

Maybe

  git log --format=%H --name-status --diff-filter=A -- path1 path2 |
  perl -lne '
    if (/[0-9a-f]{40}/) { $commit = $& }
    elsif (/^A\t(.*)/) { $h{$1} = $commit }
    END { print "$h{$_} $_" for keys(%h) }
  '

?

-Peff

^ permalink raw reply

* Re: Which file is older in history?
From: Jeff King @ 2013-03-21 12:24 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Junio C Hamano, Git List
In-Reply-To: <20130321122130.GA22607@sigill.intra.peff.net>

On Thu, Mar 21, 2013 at 08:21:30AM -0400, Jeff King wrote:

> On Thu, Mar 21, 2013 at 05:29:39PM +0530, Ramkumar Ramachandra wrote:
> 
> > > This must be a trick question but the naïve way I think of is
> > >
> > >         git log --diff-filter=A -- path1 path2
> > 
> > Thanks, I didn't know about --diff-filter.  I'll need one extra step
> > to figure out which commit corresponds to the introduction of which
> > file, no?
> 
> Maybe
> 
>   git log --format=%H --name-status --diff-filter=A -- path1 path2 |
>   perl -lne '
>     if (/[0-9a-f]{40}/) { $commit = $& }
>     elsif (/^A\t(.*)/) { $h{$1} = $commit }
>     END { print "$h{$_} $_" for keys(%h) }
>   '

Actually, I only looked at your question, not the original point, which
was not which commit was which, but which one was older. If you just
want to know which is older, then just:

  git log --format=%H --name-status --diff-filter=A -- path1 path2 |
  grep ^A |
  tail -1

which uses the regular traversal order (i.e., mostly following commit
timestamps). You can use --topo-order if you want to make sure it
follows the graph structure.

-Peff

^ permalink raw reply

* [PATCH] Run git status in the background.
From: Fredrik Gustafsson @ 2013-03-21 12:29 UTC (permalink / raw)
  To: spearce; +Cc: artagnon, git, peff, pclouds, iveqy

If core.preload is set to a non-zero value, every time a git command is
executed, git status will be runned in the background if the value of
core.preload is lower than the number of seconds since last run.

Please see this thread:
http://article.gmane.org/gmane.comp.version-control.git/218587

This solution solves many of the problems discussed there, but
introduces new ones. For example, it does have a bigger impact.

With this solution beeing functional but a bit gross, it's not sure that
it should be placed here at all. However, it's a good place to place it
for all git-tools to be able to use it without knowing about it. (It
would speed up all git wrappers and not just bash-prompt like the
previous solution).

There's a few more things to address before shipping this if this is
considered a good approach. Such as:
	* Don't run if a "git status"-like git command has been runned. Or a
	  non-repo git command (lite git status or git help) is runned.
	* Better names for settings and files.
	* Better(?) invokation of git status (a forked internal call instead
	  of a system call?).

Signed-off-by: Fredrik Gustafsson <iveqy@iveqy.com>
---
 git.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/git.c b/git.c
index 39ba6b1..829aa9c 100644
--- a/git.c
+++ b/git.c
@@ -231,6 +231,14 @@ static int handle_alias(int *argcp, const char ***argv)
 	return ret;
 }
 
+static int preload_rate = 0;
+static int preload_cb(const char *k, const char *v, void *cb)
+{
+	if (strcmp(k, "core.preload") == 0)
+		preload_rate = git_config_int(k, v);
+	return 0;
+}
+
 #define RUN_SETUP		(1<<0)
 #define RUN_SETUP_GENTLY	(1<<1)
 #define USE_PAGER		(1<<2)
@@ -278,6 +286,28 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 
 	trace_argv_printf(argv, "trace: built-in: git");
 
+	/* Check if we shall run git status in the background */
+	git_config(preload_cb, NULL);
+	if (preload_rate > 0) {
+		const char * git_dir = get_git_dir();
+		char lastrun[512];
+		strcpy(lastrun, git_dir);
+		strcat(lastrun, "/lastrun");
+
+		struct stat * lr = malloc(sizeof(struct stat));
+		stat(lastrun, lr);
+
+		if ((time(NULL) - lr->st_mtime) > preload_rate) {
+			system("git status > /dev/null 2>&1 &");
+			printf("RUN\n");
+		}
+
+		// This should be done for a few other commands as well.
+		// So that we don't spawn git-status if the user just runned that command.
+		FILE * touch = fopen(lastrun, "w");
+		fclose(touch);
+	}
+
 	status = p->fn(argc, argv, prefix);
 	if (status)
 		return status;
-- 
1.8.1.5

^ permalink raw reply related

* Re: Which file is older in history?
From: Ramkumar Ramachandra @ 2013-03-21 12:32 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Git List
In-Reply-To: <20130321122428.GB22607@sigill.intra.peff.net>

Jeff King wrote:
> On Thu, Mar 21, 2013 at 08:21:30AM -0400, Jeff King wrote:
>
>> On Thu, Mar 21, 2013 at 05:29:39PM +0530, Ramkumar Ramachandra wrote:
>>
>> > > This must be a trick question but the naïve way I think of is
>> > >
>> > >         git log --diff-filter=A -- path1 path2
>> >
>> > Thanks, I didn't know about --diff-filter.  I'll need one extra step
>> > to figure out which commit corresponds to the introduction of which
>> > file, no?
>>
>> Maybe
>>
>>   git log --format=%H --name-status --diff-filter=A -- path1 path2 |
>>   perl -lne '
>>     if (/[0-9a-f]{40}/) { $commit = $& }
>>     elsif (/^A\t(.*)/) { $h{$1} = $commit }
>>     END { print "$h{$_} $_" for keys(%h) }
>>   '
>
> Actually, I only looked at your question, not the original point, which
> was not which commit was which, but which one was older. If you just
> want to know which is older, then just:
>
>   git log --format=%H --name-status --diff-filter=A -- path1 path2 |
>   grep ^A |
>   tail -1

Great!  I just learnt about --name-status now.
Nit: tail -<n> is deprecated in favor of tail -n <n>, I think.  It's
nicer to have definite fixed command line options, as opposed to
parsing an arbitrary -(*) and deciding if \1 is a \d+.

^ permalink raw reply

* [PATCH] git-send-email.perl: implement suggestions made by perlcritic
From: Ramkumar Ramachandra @ 2013-03-21 12:39 UTC (permalink / raw)
  To: Git List

Running perlcritic with gentle severity reports six problems.  The
following lists the line numbers on which the problems occur, along
with a description of the problem.  This patch fixes them all.

516: Contrary to common belief, subroutine prototypes do not enable
compile-time checks for proper arguments.  They serve the singular
purpose of defining functions that behave like built-in functions, but
check_file_rev_conflict was never intended to behave like one.

714, 836, 855, 1487: Returning `undef' upon failure from a subroutine
is pretty common. But if the subroutine is called in list context, an
explicit `return undef;' statement will return a one-element list
containing `(undef)'. Now if that list is subsequently put in a
boolean context to test for failure, then it evaluates to true. But
you probably wanted it to be false.  The solution is to just use a
bare `return' statement whenever you want to return failure. In list
context, Perl will then give you an empty list (which is false), and
`undef' in scalar context (which is also false).

1441: The three-argument form of `open' (introduced in Perl 5.6)
prevents subtle bugs that occur when the filename starts with funny
characters like '>' or '<'.  It's also more explicitly clear to define
the input mode of the file.  This policy will not complain if the file
explicitly states that it is compatible with a version of Perl prior
to 5.6 via an include statement (see 'use 5.008' near the top of the
file).

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 I was looking at git-send-email.perl after I posted that bug report
 about SSL_verify.  What better way to get started than Perl Best
 Practices?  Here's a nice patch to fix some obvious style issues.  I
 didn't run perlcritic on a higher severity, because I don't know
 enough Perl to have a well-formed opinion of my own; I'm just fixing
 the glaringly obvious problems.

 git-send-email.perl | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index be809e5..e974b11 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -513,7 +513,7 @@ if (@alias_files and $aliasfiletype and defined $parse_alias{$aliasfiletype}) {
 ($sender) = expand_aliases($sender) if defined $sender;
 
 # returns 1 if the conflict must be solved using it as a format-patch argument
-sub check_file_rev_conflict($) {
+sub check_file_rev_conflict {
 	return unless $repo;
 	my $f = shift;
 	try {
@@ -711,7 +711,7 @@ sub ask {
 			}
 		}
 	}
-	return undef;
+	return;
 }
 
 my %broken_encoding;
@@ -833,7 +833,7 @@ sub extract_valid_address {
 	# less robust/correct than the monster regexp in Email::Valid,
 	# but still does a 99% job, and one less dependency
 	return $1 if $address =~ /($local_part_regexp\@$domain_regexp)/;
-	return undef;
+	return;
 }
 
 sub extract_valid_address_or_die {
@@ -852,7 +852,7 @@ sub validate_address {
 			valid_re => qr/^(?:quit|q|drop|d|edit|e)/i,
 			default => 'q');
 		if (/^d/i) {
-			return undef;
+			return;
 		} elsif (/^q/i) {
 			cleanup_compose_files();
 			exit(0);
@@ -1438,7 +1438,7 @@ sub recipients_cmd {
 
 	my $sanitized_sender = sanitize_address($sender);
 	my @addresses = ();
-	open my $fh, "$cmd \Q$file\E |"
+	open my $fh, q{-|}, "$cmd \Q$file\E"
 	    or die "($prefix) Could not execute '$cmd'";
 	while (my $address = <$fh>) {
 		$address =~ s/^\s*//g;
@@ -1484,7 +1484,7 @@ sub validate_patch {
 			return "$.: patch contains a line longer than 998 characters";
 		}
 	}
-	return undef;
+	return;
 }
 
 sub file_has_nonascii {
-- 
1.8.2.62.ga35d936.dirty

^ permalink raw reply related

* [PATCH v9 0/5] git log -L
From: Thomas Rast @ 2013-03-21 12:52 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Bo Yang, Zbigniew Jędrzejewski-Szmek,
	Will Palmer

Compared to the previous round at

  http://thread.gmane.org/gmane.comp.version-control.git/217236

I made the following changes (numbers from v8):

[1/5] Refactor parse_loc

  Based on Junio's suggestions, the refactoring is now to a new file
  line-range.c that line-log.c will later use.  (It still irks me too
  much to call straight into builtin/blame.c.)

  I also re-copied the existing functions, to make sure that they are
  the same.  Junio correctly spotted some differences, and since we
  claim to only refactor those are bad.

[2/5] Export rewrite_parents() for 'log -L'

  Unchanged.

[2.5/5] blame: introduce $ as "end of file" in -L syntax

  Dropped.  As Junio remarked, omitting the end is already a valid way
  of saying the same.

[3/5] Implement line-history search (git log -L)

  Integrated Ramsay's fixes and Junio's remarks.  Also made some minor
  cleanups.  Added a perf test.

[4/5] log -L: :pattern:file syntax to find by funcname

  Adjusted for changes in the earlier patches; otherwise the same.

[5/5] Speed up log -L... -M

  Added to this series, I posted the same patch earlier in the side
  thread about the bad performance.  The perf test from 4/5 neatly
  shows the difference.

Bo Yang (2):
  Refactor parse_loc
  Export rewrite_parents() for 'log -L'

Thomas Rast (3):
  Implement line-history search (git log -L)
  log -L: :pattern:file syntax to find by funcname
  Speed up log -L... -M

 Documentation/blame-options.txt     |   21 +-
 Documentation/git-blame.txt         |    6 +-
 Documentation/git-log.txt           |   23 +
 Documentation/line-range-format.txt |   25 +
 Makefile                            |    4 +
 builtin/blame.c                     |   99 +--
 builtin/log.c                       |   31 +
 line-log.c                          | 1228 +++++++++++++++++++++++++++++++++++
 line-log.h                          |   49 ++
 line-range.c                        |  237 +++++++
 line-range.h                        |   36 +
 log-tree.c                          |    4 +
 revision.c                          |   22 +-
 revision.h                          |   16 +-
 t/perf/p4211-line-log.sh            |   34 +
 t/t4211-line-log.sh                 |   40 ++
 t/t4211/expect.beginning-of-file    |   43 ++
 t/t4211/expect.end-of-file          |   62 ++
 t/t4211/expect.move-support-f       |   40 ++
 t/t4211/expect.simple-f             |   59 ++
 t/t4211/expect.simple-main          |   68 ++
 t/t4211/expect.two-ranges           |  102 +++
 t/t4211/expect.vanishes-early       |   39 ++
 t/t4211/history.export              |  330 ++++++++++
 t/t8003-blame-corner-cases.sh       |    6 +
 25 files changed, 2501 insertions(+), 123 deletions(-)
 create mode 100644 Documentation/line-range-format.txt
 create mode 100644 line-log.c
 create mode 100644 line-log.h
 create mode 100644 line-range.c
 create mode 100644 line-range.h
 create mode 100755 t/perf/p4211-line-log.sh
 create mode 100755 t/t4211-line-log.sh
 create mode 100644 t/t4211/expect.beginning-of-file
 create mode 100644 t/t4211/expect.end-of-file
 create mode 100644 t/t4211/expect.move-support-f
 create mode 100644 t/t4211/expect.simple-f
 create mode 100644 t/t4211/expect.simple-main
 create mode 100644 t/t4211/expect.two-ranges
 create mode 100644 t/t4211/expect.vanishes-early
 create mode 100644 t/t4211/history.export

-- 
1.8.2.241.gee8bb87

^ permalink raw reply

* [PATCH v9 4/5] log -L: :pattern:file syntax to find by funcname
From: Thomas Rast @ 2013-03-21 12:52 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Bo Yang, Zbigniew Jędrzejewski-Szmek,
	Will Palmer
In-Reply-To: <cover.1363865444.git.trast@student.ethz.ch>

This new syntax finds a funcname matching /pattern/, and then takes from there
up to (but not including) the next funcname.  So you can say

  git log -L:main:main.c

and it will dig up the main() function and show its line-log, provided
there are no other funcnames matching 'main'.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Documentation/blame-options.txt     |   2 +-
 Documentation/git-blame.txt         |   6 +-
 Documentation/git-log.txt           |  11 +--
 Documentation/line-range-format.txt |   7 ++
 builtin/blame.c                     |   2 +-
 line-log.c                          |   5 +-
 line-range.c                        | 130 +++++++++++++++++++++++++++++++++++-
 line-range.h                        |   3 +-
 t/t4211-line-log.sh                 |   2 +
 9 files changed, 154 insertions(+), 14 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 6998d9f..e9f984b 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -9,7 +9,7 @@
 --show-stats::
 	Include additional statistics at the end of blame output.
 
--L <start>,<end>::
+-L <start>,<end>, -L :<regex>::
 	Annotate only the given line range.  <start> and <end> can take
 	one of these forms:
 
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index 9a05c2b..6cea7f1 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -8,9 +8,9 @@ git-blame - Show what revision and author last modified each line of a file
 SYNOPSIS
 --------
 [verse]
-'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental] [-L n,m]
-	    [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>] [--abbrev=<n>]
-	    [<rev> | --contents <file> | --reverse <rev>] [--] <file>
+'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] [--incremental]
+	    [-L n,m | -L :fn] [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
+	    [--abbrev=<n>] [<rev> | --contents <file> | --reverse <rev>] [--] <file>
 
 DESCRIPTION
 -----------
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 8727c60..4850226 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -69,12 +69,13 @@ produced by --stat etc.
 	Note that only message is considered, if also a diff is shown
 	its size is not included.
 
--L <start>,<end>:<file>::
+-L <start>,<end>:<file>, -L :<regex>:<file>::
+
 	Trace the evolution of the line range given by "<start>,<end>"
-	within the <file>.  You may not give any pathspec limiters.
-	This is currently limited to a walk starting from a single
-	revision, i.e., you may only give zero or one positive
-	revision arguments.
+	(or the funcname regex <regex>) within the <file>.  You may
+	not give any pathspec limiters.  This is currently limited to
+	a walk starting from a single revision, i.e., you may only
+	give zero or one positive revision arguments.
 
 <start> and <end> can take one of these forms:
 
diff --git a/Documentation/line-range-format.txt b/Documentation/line-range-format.txt
index 265bc23..3e7ce72 100644
--- a/Documentation/line-range-format.txt
+++ b/Documentation/line-range-format.txt
@@ -16,3 +16,10 @@ starting at the line given by <start>.
 This is only valid for <end> and will specify a number
 of lines before or after the line given by <start>.
 +
+
+- :regex
++
+If the option's argument is of the form :regex, it denotes the range
+from the first funcname line that matches <regex>, up to the next
+funcname line.
++
diff --git a/builtin/blame.c b/builtin/blame.c
index 20eb439..1c09d55 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1940,7 +1940,7 @@ static void prepare_blame_range(struct scoreboard *sb,
 				long lno,
 				long *bottom, long *top)
 {
-	if (parse_range_arg(bottomtop, nth_line_cb, sb, lno, bottom, top))
+	if (parse_range_arg(bottomtop, nth_line_cb, sb, lno, bottom, top, sb->path))
 		usage(blame_usage);
 }
 
diff --git a/line-log.c b/line-log.c
index 29162fc..81c8d74 100644
--- a/line-log.c
+++ b/line-log.c
@@ -12,6 +12,7 @@
 #include "strbuf.h"
 #include "log-tree.h"
 #include "graph.h"
+#include "userdiff.h"
 #include "line-log.h"
 
 static void range_set_grow(struct range_set *rs, size_t extra)
@@ -438,7 +439,6 @@ static void range_set_map_across_diff(struct range_set *out,
 	*touched_out = touched;
 }
 
-
 static struct commit *check_single_commit(struct rev_info *revs)
 {
 	struct object *commit = NULL;
@@ -559,7 +559,8 @@ static const char *nth_line(void *data, long line)
 		cb_data.line_ends = ends;
 
 		if (parse_range_arg(range_part, nth_line, &cb_data,
-				    lines, &begin, &end))
+				    lines, &begin, &end,
+				    spec->path))
 			die("malformed -L argument '%s'", range_part);
 		if (begin < 1)
 			begin = 1;
diff --git a/line-range.c b/line-range.c
index d0c7dac..ea141ab 100644
--- a/line-range.c
+++ b/line-range.c
@@ -1,5 +1,8 @@
 #include "git-compat-util.h"
 #include "line-range.h"
+#include "xdiff-interface.h"
+#include "strbuf.h"
+#include "userdiff.h"
 
 /*
  * Parse one item in the -L option
@@ -84,9 +87,131 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,
 	}
 }
 
+static int match_funcname(xdemitconf_t *xecfg, const char *bol, const char *eol)
+{
+	if (xecfg) {
+		char buf[1];
+		return xecfg->find_func(bol, eol - bol, buf, 1,
+					xecfg->find_func_priv) >= 0;
+	}
+
+	if (bol == eol)
+		return 0;
+	if (isalpha(*bol) || *bol == '_' || *bol == '$')
+		return 1;
+	return 0;
+}
+
+static const char *find_funcname_matching_regexp(xdemitconf_t *xecfg, const char *start,
+						 regex_t *regexp)
+{
+	int reg_error;
+	regmatch_t match[1];
+	while (1) {
+		const char *bol, *eol;
+		reg_error = regexec(regexp, start, 1, match, 0);
+		if (reg_error == REG_NOMATCH)
+			return NULL;
+		else if (reg_error) {
+			char errbuf[1024];
+			regerror(reg_error, regexp, errbuf, 1024);
+			die("-L parameter: regexec() failed: %s", errbuf);
+		}
+		/* determine extent of line matched */
+		bol = start+match[0].rm_so;
+		eol = start+match[0].rm_eo;
+		while (bol > start && *bol != '\n')
+			bol--;
+		if (*bol == '\n')
+			bol++;
+		while (*eol && *eol != '\n')
+			eol++;
+		if (*eol == '\n')
+			eol++;
+		/* is it a funcname line? */
+		if (match_funcname(xecfg, (char*) bol, (char*) eol))
+			return bol;
+		start = eol;
+	}
+}
+
+static const char *parse_range_funcname(const char *arg, nth_line_fn_t nth_line_cb,
+					void *cb_data, long lines, long *begin, long *end,
+					const char *path)
+{
+	const char *pattern;
+	const char *term;
+	struct userdiff_driver *drv;
+	xdemitconf_t *xecfg = NULL;
+	const char *start;
+	const char *p;
+	int reg_error;
+	regex_t regexp;
+
+	pattern = arg+1;
+	term = (char*) strchr(pattern, ':');
+	if (term) {
+		assert(!begin);
+		return term;
+	}
+	/* all of the rest is the regex */
+	term = pattern + strlen(pattern);
+
+	start = nth_line_cb(cb_data, 0);
+
+	drv = userdiff_find_by_path(path);
+	if (drv && drv->funcname.pattern) {
+		const struct userdiff_funcname *pe = &drv->funcname;
+		xecfg = xcalloc(1, sizeof(*xecfg));
+		xdiff_set_find_func(xecfg, pe->pattern, pe->cflags);
+	}
+
+	reg_error = regcomp(&regexp, pattern, REG_NEWLINE);
+	if (reg_error) {
+		char errbuf[1024];
+		regerror(reg_error, &regexp, errbuf, 1024);
+		die("-L parameter '%s': %s", pattern, errbuf);
+	}
+
+	p = find_funcname_matching_regexp(xecfg, (char*) start, &regexp);
+	if (!p)
+		die("-L parameter '%s': no match", pattern);
+	*begin = 0;
+	while (p > nth_line_cb(cb_data, *begin))
+		(*begin)++;
+
+	if (*begin >= lines)
+		die("-L parameter '%s' matches at EOF", pattern);
+
+	*end = *begin+1;
+	while (*end < lines) {
+		const char *bol = nth_line_cb(cb_data, *end);
+		const char *eol = nth_line_cb(cb_data, *end+1);
+		if (match_funcname(xecfg, bol, eol))
+			break;
+		(*end)++;
+	}
+
+	regfree(&regexp);
+	free(xecfg);
+
+	/* compensate for 1-based numbering */
+	(*begin)++;
+
+	return term;
+}
+
 int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb,
-		    void *cb_data, long lines, long *begin, long *end)
+		    void *cb_data, long lines, long *begin, long *end,
+		    const char *path)
 {
+	if (*arg == ':') {
+		arg = parse_range_funcname(arg, nth_line_cb, cb_data, lines, begin, end, path);
+		if (*arg)
+			return -1;
+		return 0;
+	}
+
 	arg = parse_loc(arg, nth_line_cb, cb_data, lines, 1, begin);
 
 	if (*arg == ',')
@@ -100,6 +225,9 @@ int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb,
 
 const char *skip_range_arg(const char *arg)
 {
+	if (*arg == ':')
+		return parse_range_funcname(arg, NULL, NULL, 0, NULL, NULL, NULL);
+
 	arg = parse_loc(arg, NULL, NULL, 0, -1, NULL);
 
 	if (*arg == ',')
diff --git a/line-range.h b/line-range.h
index 88aaf08..ae3d012 100644
--- a/line-range.h
+++ b/line-range.h
@@ -19,7 +19,8 @@
 extern int parse_range_arg(const char *arg,
 			   nth_line_fn_t nth_line_cb,
 			   void *cb_data, long lines,
-			   long *begin, long *end);
+			   long *begin, long *end,
+			   const char *path);
 
 /*
  * Scan past a range argument that could be parsed by
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
index 286390d..096679d 100755
--- a/t/t4211-line-log.sh
+++ b/t/t4211-line-log.sh
@@ -18,8 +18,10 @@ canned_test () {
 canned_test "-L 4,12:a.c simple" simple-f
 canned_test "-L 4,+9:a.c simple" simple-f
 canned_test "-L '/long f/,/^}/:a.c' simple" simple-f
+canned_test "-L :f:a.c simple" simple-f-to-main
 
 canned_test "-L '/main/,/^}/:a.c' simple" simple-main
+canned_test "-L :main:a.c simple" simple-main-to-end
 
 canned_test "-L 1,+4:a.c simple" beginning-of-file
 
-- 
1.8.2.241.gee8bb87

^ permalink raw reply related

* [PATCH v9 1/5] Refactor parse_loc
From: Thomas Rast @ 2013-03-21 12:52 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Bo Yang, Zbigniew Jędrzejewski-Szmek,
	Will Palmer
In-Reply-To: <cover.1363865444.git.trast@student.ethz.ch>

From: Bo Yang <struggleyb.nku@gmail.com>

We want to use the same style of -L n,m argument for 'git log -L' as
for git-blame.  Refactor the argument parsing of the range arguments
from builtin/blame.c to the (new) file that will hold the 'git log -L'
logic.

To accommodate different data structures in blame and log -L, the file
contents are abstracted away; parse_range_arg takes a callback that it
uses to get the contents of a line of the (notional) file.

The new test is for a case that made me pause during debugging: the
'blame -L with invalid end' test was the only one that noticed an
outright failure to parse the end *at all*.  So make a more explicit
test for that.

Signed-off-by: Bo Yang <struggleyb.nku@gmail.com>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Documentation/blame-options.txt     | 19 +------
 Documentation/line-range-format.txt | 18 +++++++
 Makefile                            |  2 +
 builtin/blame.c                     | 99 +++----------------------------------
 line-range.c                        | 92 ++++++++++++++++++++++++++++++++++
 line-range.h                        | 24 +++++++++
 t/t8003-blame-corner-cases.sh       |  6 +++
 7 files changed, 151 insertions(+), 109 deletions(-)
 create mode 100644 Documentation/line-range-format.txt
 create mode 100644 line-range.c
 create mode 100644 line-range.h

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index b0d31df..6998d9f 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -13,24 +13,7 @@
 	Annotate only the given line range.  <start> and <end> can take
 	one of these forms:
 
-	- number
-+
-If <start> or <end> is a number, it specifies an
-absolute line number (lines count from 1).
-+
-
-- /regex/
-+
-This form will use the first line matching the given
-POSIX regex.  If <end> is a regex, it will search
-starting at the line given by <start>.
-+
-
-- +offset or -offset
-+
-This is only valid for <end> and will specify a number
-of lines before or after the line given by <start>.
-+
+include::line-range-format.txt[]
 
 -l::
 	Show long rev (Default: off).
diff --git a/Documentation/line-range-format.txt b/Documentation/line-range-format.txt
new file mode 100644
index 0000000..265bc23
--- /dev/null
+++ b/Documentation/line-range-format.txt
@@ -0,0 +1,18 @@
+- number
++
+If <start> or <end> is a number, it specifies an
+absolute line number (lines count from 1).
++
+
+- /regex/
++
+This form will use the first line matching the given
+POSIX regex.  If <end> is a regex, it will search
+starting at the line given by <start>.
++
+
+- +offset or -offset
++
+This is only valid for <end> and will specify a number
+of lines before or after the line given by <start>.
++
diff --git a/Makefile b/Makefile
index 598d631..e83f64b 100644
--- a/Makefile
+++ b/Makefile
@@ -667,6 +667,7 @@ LIB_H += help.h
 LIB_H += http.h
 LIB_H += kwset.h
 LIB_H += levenshtein.h
+LIB_H += line-range.h
 LIB_H += list-objects.h
 LIB_H += ll-merge.h
 LIB_H += log-tree.h
@@ -795,6 +796,7 @@ LIB_OBJS += hex.o
 LIB_OBJS += ident.o
 LIB_OBJS += kwset.o
 LIB_OBJS += levenshtein.o
+LIB_OBJS += line-range.o
 LIB_OBJS += list-objects.o
 LIB_OBJS += ll-merge.o
 LIB_OBJS += lockfile.o
diff --git a/builtin/blame.c b/builtin/blame.c
index 86100e9..20eb439 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -21,6 +21,7 @@
 #include "parse-options.h"
 #include "utf8.h"
 #include "userdiff.h"
+#include "line-range.h"
 
 static char blame_usage[] = N_("git blame [options] [rev-opts] [rev] [--] file");
 
@@ -566,11 +567,16 @@ static void dup_entry(struct blame_entry *dst, struct blame_entry *src)
 	dst->score = 0;
 }
 
-static const char *nth_line(struct scoreboard *sb, int lno)
+static const char *nth_line(struct scoreboard *sb, long lno)
 {
 	return sb->final_buf + sb->lineno[lno];
 }
 
+static const char *nth_line_cb(void *data, long lno)
+{
+	return nth_line((struct scoreboard *)data, lno);
+}
+
 /*
  * It is known that lines between tlno to same came from parent, and e
  * has an overlap with that range.  it also is known that parent's
@@ -1927,83 +1933,6 @@ static const char *add_prefix(const char *prefix, const char *path)
 }
 
 /*
- * Parsing of (comma separated) one item in the -L option
- */
-static const char *parse_loc(const char *spec,
-			     struct scoreboard *sb, long lno,
-			     long begin, long *ret)
-{
-	char *term;
-	const char *line;
-	long num;
-	int reg_error;
-	regex_t regexp;
-	regmatch_t match[1];
-
-	/* Allow "-L <something>,+20" to mean starting at <something>
-	 * for 20 lines, or "-L <something>,-5" for 5 lines ending at
-	 * <something>.
-	 */
-	if (1 < begin && (spec[0] == '+' || spec[0] == '-')) {
-		num = strtol(spec + 1, &term, 10);
-		if (term != spec + 1) {
-			if (spec[0] == '-')
-				num = 0 - num;
-			if (0 < num)
-				*ret = begin + num - 2;
-			else if (!num)
-				*ret = begin;
-			else
-				*ret = begin + num;
-			return term;
-		}
-		return spec;
-	}
-	num = strtol(spec, &term, 10);
-	if (term != spec) {
-		*ret = num;
-		return term;
-	}
-	if (spec[0] != '/')
-		return spec;
-
-	/* it could be a regexp of form /.../ */
-	for (term = (char *) spec + 1; *term && *term != '/'; term++) {
-		if (*term == '\\')
-			term++;
-	}
-	if (*term != '/')
-		return spec;
-
-	/* try [spec+1 .. term-1] as regexp */
-	*term = 0;
-	begin--; /* input is in human terms */
-	line = nth_line(sb, begin);
-
-	if (!(reg_error = regcomp(&regexp, spec + 1, REG_NEWLINE)) &&
-	    !(reg_error = regexec(&regexp, line, 1, match, 0))) {
-		const char *cp = line + match[0].rm_so;
-		const char *nline;
-
-		while (begin++ < lno) {
-			nline = nth_line(sb, begin);
-			if (line <= cp && cp < nline)
-				break;
-			line = nline;
-		}
-		*ret = begin;
-		regfree(&regexp);
-		*term++ = '/';
-		return term;
-	}
-	else {
-		char errbuf[1024];
-		regerror(reg_error, &regexp, errbuf, 1024);
-		die("-L parameter '%s': %s", spec + 1, errbuf);
-	}
-}
-
-/*
  * Parsing of -L option
  */
 static void prepare_blame_range(struct scoreboard *sb,
@@ -2011,15 +1940,7 @@ static void prepare_blame_range(struct scoreboard *sb,
 				long lno,
 				long *bottom, long *top)
 {
-	const char *term;
-
-	term = parse_loc(bottomtop, sb, lno, 1, bottom);
-	if (*term == ',') {
-		term = parse_loc(term + 1, sb, lno, *bottom + 1, top);
-		if (*term)
-			usage(blame_usage);
-	}
-	if (*term)
+	if (parse_range_arg(bottomtop, nth_line_cb, sb, lno, bottom, top))
 		usage(blame_usage);
 }
 
@@ -2569,10 +2490,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	bottom = top = 0;
 	if (bottomtop)
 		prepare_blame_range(&sb, bottomtop, lno, &bottom, &top);
-	if (bottom && top && top < bottom) {
-		long tmp;
-		tmp = top; top = bottom; bottom = tmp;
-	}
 	if (bottom < 1)
 		bottom = 1;
 	if (top < 1)
diff --git a/line-range.c b/line-range.c
new file mode 100644
index 0000000..0af43bf
--- /dev/null
+++ b/line-range.c
@@ -0,0 +1,92 @@
+#include "git-compat-util.h"
+#include "line-range.h"
+
+/*
+ * Parse one item in the -L option
+ */
+static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,
+			     void *data, long lines, long begin, long *ret)
+{
+	char *term;
+	const char *line;
+	long num;
+	int reg_error;
+	regex_t regexp;
+	regmatch_t match[1];
+
+	/* Allow "-L <something>,+20" to mean starting at <something>
+	 * for 20 lines, or "-L <something>,-5" for 5 lines ending at
+	 * <something>.
+	 */
+	if (1 < begin && (spec[0] == '+' || spec[0] == '-')) {
+		num = strtol(spec + 1, &term, 10);
+		if (term != spec + 1) {
+			if (spec[0] == '-')
+				num = 0 - num;
+			if (0 < num)
+				*ret = begin + num - 2;
+			else if (!num)
+				*ret = begin;
+			else
+				*ret = begin + num;
+			return term;
+		}
+		return spec;
+	}
+	num = strtol(spec, &term, 10);
+	if (term != spec) {
+		*ret = num;
+		return term;
+	}
+	if (spec[0] != '/')
+		return spec;
+
+	/* it could be a regexp of form /.../ */
+	for (term = (char *) spec + 1; *term && *term != '/'; term++) {
+		if (*term == '\\')
+			term++;
+	}
+	if (*term != '/')
+		return spec;
+
+	/* try [spec+1 .. term-1] as regexp */
+	*term = 0;
+	begin--; /* input is in human terms */
+	line = nth_line(data, begin);
+
+	if (!(reg_error = regcomp(&regexp, spec + 1, REG_NEWLINE)) &&
+	    !(reg_error = regexec(&regexp, line, 1, match, 0))) {
+		const char *cp = line + match[0].rm_so;
+		const char *nline;
+
+		while (begin++ < lines) {
+			nline = nth_line(data, begin);
+			if (line <= cp && cp < nline)
+				break;
+			line = nline;
+		}
+		*ret = begin;
+		regfree(&regexp);
+		*term++ = '/';
+		return term;
+	}
+	else {
+		char errbuf[1024];
+		regerror(reg_error, &regexp, errbuf, 1024);
+		die("-L parameter '%s': %s", spec + 1, errbuf);
+	}
+}
+
+int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb,
+		    void *cb_data, long lines, long *begin, long *end)
+{
+	arg = parse_loc(arg, nth_line_cb, cb_data, lines, 1, begin);
+
+	if (*arg == ',')
+		arg = parse_loc(arg+1, nth_line_cb, cb_data, lines, *begin+1, end);
+
+	if (*arg)
+		return -1;
+
+	return 0;
+}
diff --git a/line-range.h b/line-range.h
new file mode 100644
index 0000000..830f25b
--- /dev/null
+++ b/line-range.h
@@ -0,0 +1,24 @@
+#ifndef LINE_RANGE_H
+#define LINE_RANGE_H
+
+/*
+ * Parse one item in an -L begin,end option w.r.t. the notional file
+ * object 'cb_data' consisting of 'lines' lines.
+ *
+ * The 'nth_line_cb' callback is used to determine the start of the
+ * line 'lno' inside the 'cb_data'.  The caller is expected to already
+ * have a suitable map at hand to make this a constant-time lookup.
+ *
+ * Returns 0 in case of success and -1 if there was an error.  The
+ * actual range is stored in *begin and *end.  The counting starts
+ * at 1!  In case of error, the caller should show usage message.
+ */
+
+typedef const char *(*nth_line_fn_t)(void *data, long lno);
+
+extern int parse_range_arg(const char *arg,
+			   nth_line_fn_t nth_line_cb,
+			   void *cb_data, long lines,
+			   long *begin, long *end);
+
+#endif /* LINE_RANGE_H */
diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
index 230143c..e7cac1d 100755
--- a/t/t8003-blame-corner-cases.sh
+++ b/t/t8003-blame-corner-cases.sh
@@ -175,6 +175,12 @@ test_expect_success 'blame -L with invalid end' '
 	grep "has only 2 lines" errors
 '
 
+test_expect_success 'blame parses <end> part of -L' '
+	git blame -L1,1 tres >out &&
+	cat out &&
+	test $(wc -l < out) -eq 1
+'
+
 test_expect_success 'indent of line numbers, nine lines' '
 	git blame nine_lines >actual &&
 	test $(grep -c "  " actual) = 0
-- 
1.8.2.241.gee8bb87

^ permalink raw reply related

* [PATCH v9 3/5] Implement line-history search (git log -L)
From: Thomas Rast @ 2013-03-21 12:52 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Bo Yang, Zbigniew Jędrzejewski-Szmek,
	Will Palmer
In-Reply-To: <cover.1363865444.git.trast@student.ethz.ch>

This is a rewrite of much of Bo's work, mainly in an effort to split
it into smaller, easier to understand routines.

The algorithm is built around the struct range_set, which encodes a
series of line ranges as intervals [a,b).  This is used in two
contexts:

* A set of lines we are tracking (which will change as we dig through
  history).
* To encode diffs, as pairs of ranges.

The main routine is range_set_map_across_diff().  It processes the
diff between a commit C and some parent P.  It determines which diff
hunks are relevant to the ranges tracked in C, and computes the new
ranges for P.

The algorithm is then simply to process history in topological order
from newest to oldest, computing ranges and (partial) diffs.  At
branch points, we need to merge the ranges we are watching.  We will
find that many commits do not affect the chosen ranges, and mark them
TREESAME (in addition to those already filtered by pathspec limiting).
Another pass of history simplification then gets rid of such commits.

This is wired as an extra filtering pass in the log machinery.  This
currently only reduces code duplication, but should allow for other
simplifications and options to be used.

Finally, we hook a diff printer into the output chain.  Ideally we
would wire directly into the diff logic, to optionally use features
like word diff.  However, that will require some major reworking of
the diff chain, so we completely replace the output with our own diff
for now.

As this was a GSoC project, and has quite some history by now, many
people have helped.  In no particular order, thanks go to

  Jakub Narebski <jnareb@gmail.com>
  Jens Lehmann <Jens.Lehmann@web.de>
  Jonathan Nieder <jrnieder@gmail.com>
  Junio C Hamano <gitster@pobox.com>
  Ramsay Jones <ramsay@ramsay1.demon.co.uk>
  Will Palmer <wmpalmer@gmail.com>

Apologies to everyone I forgot.

Signed-off-by: Bo Yang <struggleyb.nku@gmail.com>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Documentation/git-log.txt        |   22 +
 Makefile                         |    2 +
 builtin/log.c                    |   31 +
 line-log.c                       | 1179 ++++++++++++++++++++++++++++++++++++++
 line-log.h                       |   49 ++
 line-range.c                     |   19 +-
 line-range.h                     |   11 +
 log-tree.c                       |    4 +
 revision.c                       |    9 +
 revision.h                       |    6 +-
 t/perf/p4211-line-log.sh         |   34 ++
 t/t4211-line-log.sh              |   38 ++
 t/t4211/expect.beginning-of-file |   43 ++
 t/t4211/expect.end-of-file       |   62 ++
 t/t4211/expect.move-support-f    |   40 ++
 t/t4211/expect.simple-f          |   59 ++
 t/t4211/expect.simple-main       |   68 +++
 t/t4211/expect.two-ranges        |  102 ++++
 t/t4211/expect.vanishes-early    |   39 ++
 t/t4211/history.export           |  330 +++++++++++
 20 files changed, 2145 insertions(+), 2 deletions(-)
 create mode 100644 line-log.c
 create mode 100644 line-log.h
 create mode 100755 t/perf/p4211-line-log.sh
 create mode 100755 t/t4211-line-log.sh
 create mode 100644 t/t4211/expect.beginning-of-file
 create mode 100644 t/t4211/expect.end-of-file
 create mode 100644 t/t4211/expect.move-support-f
 create mode 100644 t/t4211/expect.simple-f
 create mode 100644 t/t4211/expect.simple-main
 create mode 100644 t/t4211/expect.two-ranges
 create mode 100644 t/t4211/expect.vanishes-early
 create mode 100644 t/t4211/history.export

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 69db578..8727c60 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -69,6 +69,23 @@ produced by --stat etc.
 	Note that only message is considered, if also a diff is shown
 	its size is not included.
 
+-L <start>,<end>:<file>::
+	Trace the evolution of the line range given by "<start>,<end>"
+	within the <file>.  You may not give any pathspec limiters.
+	This is currently limited to a walk starting from a single
+	revision, i.e., you may only give zero or one positive
+	revision arguments.
+
+<start> and <end> can take one of these forms:
+
+include::line-range-format.txt[]
+You can specify this option more than once.
+
+
+--full-line-diff::
+	Always print the interesting range even if the current commit
+	does not change any line of the range.
+
 [\--] <path>...::
 	Show only commits that are enough to explain how the files
 	that match the specified paths came to be.  See "History
@@ -138,6 +155,11 @@ Examples
 	This makes sense only when following a strict policy of merging all
 	topic branches when staying on a single integration branch.
 
+git log -L '/int main/',/^}/:main.c::
+
+	Shows how the function `main()` in the file 'main.c' evolved
+	over time.
+
 `git log -3`::
 	Limits the number of commits to show to 3.
 
diff --git a/Makefile b/Makefile
index e83f64b..2156fa7 100644
--- a/Makefile
+++ b/Makefile
@@ -667,6 +667,7 @@ LIB_H += help.h
 LIB_H += http.h
 LIB_H += kwset.h
 LIB_H += levenshtein.h
+LIB_H += line-log.h
 LIB_H += line-range.h
 LIB_H += list-objects.h
 LIB_H += ll-merge.h
@@ -796,6 +797,7 @@ LIB_OBJS += hex.o
 LIB_OBJS += ident.o
 LIB_OBJS += kwset.o
 LIB_OBJS += levenshtein.o
+LIB_OBJS += line-log.o
 LIB_OBJS += line-range.o
 LIB_OBJS += list-objects.o
 LIB_OBJS += ll-merge.o
diff --git a/builtin/log.c b/builtin/log.c
index 8f0b2e8..d584ff1 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -19,6 +19,7 @@
 #include "remote.h"
 #include "string-list.h"
 #include "parse-options.h"
+#include "line-log.h"
 #include "branch.h"
 #include "streaming.h"
 #include "version.h"
@@ -41,6 +42,12 @@
 	NULL
 };
 
+struct line_opt_callback_data {
+	struct rev_info *rev;
+	const char *prefix;
+	struct string_list args;
+};
+
 static int parse_decoration_style(const char *var, const char *value)
 {
 	switch (git_config_maybe_bool(var, value)) {
@@ -75,6 +82,19 @@ static int decorate_callback(const struct option *opt, const char *arg, int unse
 	return 0;
 }
 
+static int log_line_range_callback(const struct option *option, const char *arg, int unset)
+{
+	struct line_opt_callback_data *data = option->value;
+
+	if (!arg)
+		return -1;
+
+	data->rev->line_level_traverse = 1;
+	string_list_append(&data->args, arg);
+
+	return 0;
+}
+
 static void cmd_log_init_defaults(struct rev_info *rev)
 {
 	if (fmt_pretty)
@@ -97,6 +117,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 {
 	struct userformat_want w;
 	int quiet = 0, source = 0, mailmap = 0;
+	static struct line_opt_callback_data line_cb = {NULL, NULL, STRING_LIST_INIT_DUP};
 
 	const struct option builtin_log_options[] = {
 		OPT_BOOLEAN(0, "quiet", &quiet, N_("suppress diff output")),
@@ -104,9 +125,15 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 		OPT_BOOLEAN(0, "use-mailmap", &mailmap, N_("Use mail map file")),
 		{ OPTION_CALLBACK, 0, "decorate", NULL, NULL, N_("decorate options"),
 		  PARSE_OPT_OPTARG, decorate_callback},
+		OPT_CALLBACK('L', NULL, &line_cb, "n,m:file",
+			     "Process line range n,m in file, counting from 1",
+			     log_line_range_callback),
 		OPT_END()
 	};
 
+	line_cb.rev = rev;
+	line_cb.prefix = prefix;
+
 	mailmap = use_mailmap_config;
 	argc = parse_options(argc, argv, prefix,
 			     builtin_log_options, builtin_log_usage,
@@ -160,6 +187,10 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
 		rev->show_decorations = 1;
 		load_ref_decorations(decoration_style);
 	}
+
+	if (rev->line_level_traverse)
+		line_log_init(rev, line_cb.prefix, &line_cb.args);
+
 	setup_pager();
 }
 
diff --git a/line-log.c b/line-log.c
new file mode 100644
index 0000000..29162fc
--- /dev/null
+++ b/line-log.c
@@ -0,0 +1,1179 @@
+#include "git-compat-util.h"
+#include "line-range.h"
+#include "cache.h"
+#include "tag.h"
+#include "blob.h"
+#include "tree.h"
+#include "diff.h"
+#include "commit.h"
+#include "decorate.h"
+#include "revision.h"
+#include "xdiff-interface.h"
+#include "strbuf.h"
+#include "log-tree.h"
+#include "graph.h"
+#include "line-log.h"
+
+static void range_set_grow(struct range_set *rs, size_t extra)
+{
+	ALLOC_GROW(rs->ranges, rs->nr + extra, rs->alloc);
+}
+
+/* Either initialization would be fine */
+#define RANGE_SET_INIT {0}
+
+static void range_set_init(struct range_set *rs, size_t prealloc)
+{
+	rs->alloc = rs->nr = 0;
+	rs->ranges = NULL;
+	if (prealloc)
+		range_set_grow(rs, prealloc);
+}
+
+static void range_set_release(struct range_set *rs)
+{
+	free(rs->ranges);
+	rs->alloc = rs->nr = 0;
+	rs->ranges = NULL;
+}
+
+/* dst must be uninitialized! */
+static void range_set_copy(struct range_set *dst, struct range_set *src)
+{
+	range_set_init(dst, src->nr);
+	memcpy(dst->ranges, src->ranges, src->nr*sizeof(struct range_set));
+	dst->nr = src->nr;
+}
+static void range_set_move(struct range_set *dst, struct range_set *src)
+{
+	range_set_release(dst);
+	dst->ranges = src->ranges;
+	dst->nr = src->nr;
+	dst->alloc = src->alloc;
+	src->ranges = NULL;
+	src->alloc = src->nr = 0;
+}
+
+/* tack on a _new_ range _at the end_ */
+static void range_set_append(struct range_set *rs, long a, long b)
+{
+	assert(a <= b);
+	assert(rs->nr == 0 || rs->ranges[rs->nr-1].end <= a);
+	range_set_grow(rs, 1);
+	rs->ranges[rs->nr].start = a;
+	rs->ranges[rs->nr].end = b;
+	rs->nr++;
+}
+
+static int range_cmp(const void *_r, const void *_s)
+{
+	const struct range *r = _r;
+	const struct range *s = _s;
+
+	/* this could be simply 'return r.start-s.start', but for the types */
+	if (r->start == s->start)
+		return 0;
+	if (r->start < s->start)
+		return -1;
+	return 1;
+}
+
+/*
+ * Helper: In-place pass of sorting and merging the ranges in the
+ * range set, to re-establish the invariants after another operation
+ *
+ * NEEDSWORK currently not needed
+ */
+static void sort_and_merge_range_set(struct range_set *rs)
+{
+	int i;
+	int o = 1; /* output cursor */
+
+	qsort(rs->ranges, rs->nr, sizeof(struct range), range_cmp);
+
+	for (i = 1; i < rs->nr; i++) {
+		if (rs->ranges[i].start <= rs->ranges[o-1].end) {
+			rs->ranges[o-1].end = rs->ranges[i].end;
+		} else {
+			rs->ranges[o].start = rs->ranges[i].start;
+			rs->ranges[o].end = rs->ranges[i].end;
+			o++;
+		}
+	}
+	assert(o <= rs->nr);
+	rs->nr = o;
+}
+
+/*
+ * Union of range sets (i.e., sets of line numbers).  Used to merge
+ * them when searches meet at a common ancestor.
+ *
+ * This is also where the ranges are consolidated into canonical form:
+ * overlapping and adjacent ranges are merged, and empty ranges are
+ * removed.
+ */
+static void range_set_union(struct range_set *out,
+			     struct range_set *a, struct range_set *b)
+{
+	int i = 0, j = 0, o = 0;
+	struct range *ra = a->ranges;
+	struct range *rb = b->ranges;
+	/* cannot make an alias of out->ranges: it may change during grow */
+
+	assert(out->nr == 0);
+	while (i < a->nr || j < b->nr) {
+		struct range *new;
+		if (i < a->nr && j < b->nr) {
+			if (ra[i].start < rb[j].start)
+				new = &ra[i++];
+			else if (ra[i].start > rb[j].start)
+				new = &rb[j++];
+			else if (ra[i].end < rb[j].end)
+				new = &ra[i++];
+			else
+				new = &rb[j++];
+		} else if (i < a->nr)      /* b exhausted */
+			new = &ra[i++];
+		else                       /* a exhausted */
+			new = &rb[j++];
+		if (new->start == new->end)
+			; /* empty range */
+		else if (!o || out->ranges[o-1].end < new->start) {
+			range_set_grow(out, 1);
+			out->ranges[o].start = new->start;
+			out->ranges[o].end = new->end;
+			o++;
+		} else if (out->ranges[o-1].end < new->end) {
+			out->ranges[o-1].end = new->end;
+		}
+	}
+	out->nr = o;
+}
+
+/*
+ * Difference of range sets (out = a \ b).  Pass the "interesting"
+ * ranges as 'a' and the target side of the diff as 'b': it removes
+ * the ranges for which the commit is responsible.
+ */
+static void range_set_difference(struct range_set *out,
+				  struct range_set *a, struct range_set *b)
+{
+	int i, j =  0;
+	for (i = 0; i < a->nr; i++) {
+		long start = a->ranges[i].start;
+		long end = a->ranges[i].end;
+		while (start < end) {
+			while (j < b->nr && start >= b->ranges[j].end)
+				/*
+				 * a:         |-------
+				 * b: ------|
+				 */
+				j++;
+			if (j >= b->nr || end < b->ranges[j].start) {
+				/*
+				 * b exhausted, or
+				 * a:  ----|
+				 * b:         |----
+				 */
+				range_set_append(out, start, end);
+				break;
+			}
+			if (start >= b->ranges[j].start) {
+				/*
+				 * a:     |--????
+				 * b: |------|
+				 */
+				start = b->ranges[j].end;
+			} else if (end > b->ranges[j].start) {
+				/*
+				 * a: |-----|
+				 * b:    |--?????
+				 */
+				if (start < b->ranges[j].start)
+					range_set_append(out, start, b->ranges[j].start);
+				start = b->ranges[j].end;
+			}
+		}
+	}
+}
+
+static void diff_ranges_init(struct diff_ranges *diff)
+{
+	range_set_init(&diff->parent, 0);
+	range_set_init(&diff->target, 0);
+}
+
+static void diff_ranges_release(struct diff_ranges *diff)
+{
+	range_set_release(&diff->parent);
+	range_set_release(&diff->target);
+}
+
+void line_log_data_init(struct line_log_data *r)
+{
+	memset(r, 0, sizeof(struct line_log_data));
+	range_set_init(&r->ranges, 0);
+}
+
+static void line_log_data_clear(struct line_log_data *r)
+{
+	range_set_release(&r->ranges);
+	if (r->pair)
+		diff_free_filepair(r->pair);
+}
+
+static void free_line_log_data(struct line_log_data *r)
+{
+	while (r) {
+		struct line_log_data *next = r->next;
+		line_log_data_clear(r);
+		free(r);
+		r = next;
+	}
+}
+
+static struct line_log_data *
+search_line_log_data(struct line_log_data *list, const char *path,
+		     struct line_log_data **insertion_point)
+{
+	struct line_log_data *p = list;
+	if (insertion_point)
+		*insertion_point = NULL;
+	while (p) {
+		int cmp = strcmp(p->spec->path, path);
+		if (!cmp)
+			return p;
+		if (insertion_point && cmp < 0)
+			*insertion_point = p;
+		p = p->next;
+	}
+	return NULL;
+}
+
+static void line_log_data_insert(struct line_log_data **list,
+				 struct diff_filespec *spec,
+				 long begin, long end)
+{
+	struct line_log_data *ip;
+	struct line_log_data *p = search_line_log_data(*list, spec->path, &ip);
+
+	if (p) {
+		range_set_append(&p->ranges, begin, end);
+		sort_and_merge_range_set(&p->ranges);
+		free_filespec(spec);
+		return;
+	}
+
+	p = xcalloc(1, sizeof(struct line_log_data));
+	p->spec = spec;
+	range_set_append(&p->ranges, begin, end);
+	if (ip) {
+		p->next = ip->next;
+		ip->next = p;
+	} else {
+		p->next = *list;
+		*list = p;
+	}
+}
+
+struct collect_diff_cbdata {
+	struct diff_ranges *diff;
+};
+
+static int collect_diff_cb(long start_a, long count_a,
+			   long start_b, long count_b,
+			   void *data)
+{
+	struct collect_diff_cbdata *d = data;
+
+	if (count_a >= 0)
+		range_set_append(&d->diff->parent, start_a, start_a + count_a);
+	if (count_b >= 0)
+		range_set_append(&d->diff->target, start_b, start_b + count_b);
+
+	return 0;
+}
+
+static void collect_diff(mmfile_t *parent, mmfile_t *target, struct diff_ranges *out)
+{
+	struct collect_diff_cbdata cbdata = {NULL};
+	xpparam_t xpp;
+	xdemitconf_t xecfg;
+	xdemitcb_t ecb;
+
+	memset(&xpp, 0, sizeof(xpp));
+	memset(&xecfg, 0, sizeof(xecfg));
+	xecfg.ctxlen = xecfg.interhunkctxlen = 0;
+
+	cbdata.diff = out;
+	xecfg.hunk_func = collect_diff_cb;
+	memset(&ecb, 0, sizeof(ecb));
+	ecb.priv = &cbdata;
+	xdi_diff(parent, target, &xpp, &xecfg, &ecb);
+}
+
+/*
+ * These are handy for debugging.  Removing them with #if 0 silences
+ * the "unused function" warning.
+ */
+#if 0
+static void dump_range_set(struct range_set *rs, const char *desc)
+{
+	int i;
+	printf("range set %s (%d items):\n", desc, rs->nr);
+	for (i = 0; i < rs->nr; i++)
+		printf("\t[%ld,%ld]\n", rs->ranges[i].start, rs->ranges[i].end);
+}
+
+static void dump_line_log_data(struct line_log_data *r)
+{
+	char buf[4096];
+	while (r) {
+		snprintf(buf, 4096, "file %s\n", r->spec->path);
+		dump_range_set(&r->ranges, buf);
+		r = r->next;
+	}
+}
+
+static void dump_diff_ranges(struct diff_ranges *diff, const char *desc)
+{
+	int i;
+	assert(diff->parent.nr == diff->target.nr);
+	printf("diff ranges %s (%d items):\n", desc, diff->parent.nr);
+	printf("\tparent\ttarget\n");
+	for (i = 0; i < diff->parent.nr; i++) {
+		printf("\t[%ld,%ld]\t[%ld,%ld]\n",
+		       diff->parent.ranges[i].start,
+		       diff->parent.ranges[i].end,
+		       diff->target.ranges[i].start,
+		       diff->target.ranges[i].end);
+	}
+}
+#endif
+
+
+static int ranges_overlap(struct range *a, struct range *b)
+{
+	return !(a->end <= b->start || b->end <= a->start);
+}
+
+/*
+ * Given a diff and the set of interesting ranges, determine all hunks
+ * of the diff which touch (overlap) at least one of the interesting
+ * ranges in the target.
+ */
+static void diff_ranges_filter_touched(struct diff_ranges *out,
+				       struct diff_ranges *diff,
+				       struct range_set *rs)
+{
+	int i, j = 0;
+
+	assert(out->target.nr == 0);
+
+	for (i = 0; i < diff->target.nr; i++) {
+		while (diff->target.ranges[i].start > rs->ranges[j].end) {
+			j++;
+			if (j == rs->nr)
+				return;
+		}
+		if (ranges_overlap(&diff->target.ranges[i], &rs->ranges[j])) {
+			range_set_append(&out->parent,
+					 diff->parent.ranges[i].start,
+					 diff->parent.ranges[i].end);
+			range_set_append(&out->target,
+					 diff->target.ranges[i].start,
+					 diff->target.ranges[i].end);
+		}
+	}
+}
+
+/*
+ * Adjust the line counts in 'rs' to account for the lines
+ * added/removed in the diff.
+ */
+static void range_set_shift_diff(struct range_set *out,
+				 struct range_set *rs,
+				 struct diff_ranges *diff)
+{
+	int i, j = 0;
+	long offset = 0;
+	struct range *src = rs->ranges;
+	struct range *target = diff->target.ranges;
+	struct range *parent = diff->parent.ranges;
+
+	for (i = 0; i < rs->nr; i++) {
+		while (j < diff->target.nr && src[i].start >= target[j].start) {
+			offset += (parent[j].end-parent[j].start)
+				- (target[j].end-target[j].start);
+			j++;
+		}
+		range_set_append(out, src[i].start+offset, src[i].end+offset);
+	}
+}
+
+/*
+ * Given a diff and the set of interesting ranges, map the ranges
+ * across the diff.  That is: observe that the target commit takes
+ * blame for all the + (target-side) ranges.  So for every pair of
+ * ranges in the diff that was touched, we remove the latter and add
+ * its parent side.
+ */
+static void range_set_map_across_diff(struct range_set *out,
+				      struct range_set *rs,
+				      struct diff_ranges *diff,
+				      struct diff_ranges **touched_out)
+{
+	struct diff_ranges *touched = xmalloc(sizeof(*touched));
+	struct range_set tmp1 = RANGE_SET_INIT;
+	struct range_set tmp2 = RANGE_SET_INIT;
+
+	diff_ranges_init(touched);
+	diff_ranges_filter_touched(touched, diff, rs);
+	range_set_difference(&tmp1, rs, &touched->target);
+	range_set_shift_diff(&tmp2, &tmp1, diff);
+	range_set_union(out, &tmp2, &touched->parent);
+	range_set_release(&tmp1);
+	range_set_release(&tmp2);
+
+	*touched_out = touched;
+}
+
+
+static struct commit *check_single_commit(struct rev_info *revs)
+{
+	struct object *commit = NULL;
+	int found = -1;
+	int i;
+
+	for (i = 0; i < revs->pending.nr; i++) {
+		struct object *obj = revs->pending.objects[i].item;
+		if (obj->flags & UNINTERESTING)
+			continue;
+		while (obj->type == OBJ_TAG)
+			obj = deref_tag(obj, NULL, 0);
+		if (obj->type != OBJ_COMMIT)
+			die("Non commit %s?", revs->pending.objects[i].name);
+		if (commit)
+			die("More than one commit to dig from: %s and %s?",
+			    revs->pending.objects[i].name,
+			    revs->pending.objects[found].name);
+		commit = obj;
+		found = i;
+	}
+
+	if (!commit)
+		die("No commit specified?");
+
+	return (struct commit *) commit;
+}
+
+static void fill_blob_sha1(struct commit *commit, struct diff_filespec *spec)
+{
+	unsigned mode;
+	unsigned char sha1[20];
+
+	if (get_tree_entry(commit->object.sha1, spec->path,
+			   sha1, &mode))
+		die("There is no path %s in the commit", spec->path);
+	fill_filespec(spec, sha1, 1, mode);
+
+	return;
+}
+
+static void fill_line_ends(struct diff_filespec *spec, long *lines,
+			   unsigned long **line_ends)
+{
+	int num = 0, size = 50;
+	long cur = 0;
+	unsigned long *ends = NULL;
+	char *data = NULL;
+
+	if (diff_populate_filespec(spec, 0))
+		die("Cannot read blob %s", sha1_to_hex(spec->sha1));
+
+	ends = xmalloc(size * sizeof(*ends));
+	ends[cur++] = 0;
+	data = spec->data;
+	while (num < spec->size) {
+		if (data[num] == '\n' || num == spec->size - 1) {
+			ALLOC_GROW(ends, (cur + 1), size);
+			ends[cur++] = num;
+		}
+		num++;
+	}
+
+	/* shrink the array to fit the elements */
+	ends = xrealloc(ends, cur * sizeof(*ends));
+	*lines = cur-1;
+	*line_ends = ends;
+}
+
+struct nth_line_cb {
+	struct diff_filespec *spec;
+	long lines;
+	unsigned long *line_ends;
+};
+
+static const char *nth_line(void *data, long line)
+{
+	struct nth_line_cb *d = data;
+	assert(d && line <= d->lines);
+	assert(d->spec && d->spec->data);
+
+	if (line == 0)
+		return (char *)d->spec->data;
+	else
+		return (char *)d->spec->data + d->line_ends[line] + 1;
+}
+
+static struct line_log_data *
+parse_lines(struct commit *commit, const char *prefix, struct string_list *args)
+{
+	long lines = 0;
+	unsigned long *ends = NULL;
+	struct nth_line_cb cb_data;
+	struct string_list_item *item;
+	struct line_log_data *ranges = NULL;
+
+	for_each_string_list_item(item, args) {
+		const char *name_part, *range_part;
+		const char *full_name;
+		struct diff_filespec *spec;
+		long begin = 0, end = 0;
+
+		name_part = skip_range_arg(item->string);
+		if (!name_part || *name_part != ':')
+			die("-L argument '%s' not of the form start,end:file",
+			    item->string);
+		range_part = xstrndup(item->string, name_part - item->string);
+		name_part++;
+
+		full_name = prefix_path(prefix, prefix ? strlen(prefix) : 0,
+					name_part);
+
+		spec = alloc_filespec(full_name);
+		fill_blob_sha1(commit, spec);
+		fill_line_ends(spec, &lines, &ends);
+		cb_data.spec = spec;
+		cb_data.lines = lines;
+		cb_data.line_ends = ends;
+
+		if (parse_range_arg(range_part, nth_line, &cb_data,
+				    lines, &begin, &end))
+			die("malformed -L argument '%s'", range_part);
+		if (begin < 1)
+			begin = 1;
+		if (end < 1)
+			end = lines;
+		begin--;
+		if (lines < end || lines < begin)
+			die("file %s has only %ld lines", name_part, lines);
+		line_log_data_insert(&ranges, spec, begin, end);
+
+		free(ends);
+		ends = NULL;
+	}
+
+	return ranges;
+}
+
+static struct line_log_data *line_log_data_copy_one(struct line_log_data *r)
+{
+	struct line_log_data *ret = xmalloc(sizeof(*ret));
+
+	assert(r);
+	line_log_data_init(ret);
+	range_set_copy(&ret->ranges, &r->ranges);
+
+	ret->spec = r->spec;
+	assert(ret->spec);
+	ret->spec->count++;
+
+	return ret;
+}
+
+static struct line_log_data *
+line_log_data_copy(struct line_log_data *r)
+{
+	struct line_log_data *ret = NULL;
+	struct line_log_data *tmp = NULL, *prev = NULL;
+
+	assert(r);
+	ret = tmp = prev = line_log_data_copy_one(r);
+	r = r->next;
+	while (r) {
+		tmp = line_log_data_copy_one(r);
+		prev->next = tmp;
+		prev = tmp;
+		r = r->next;
+	}
+
+	return ret;
+}
+
+/* merge two range sets across files */
+static struct line_log_data *line_log_data_merge(struct line_log_data *a,
+						 struct line_log_data *b)
+{
+	struct line_log_data *head = NULL, **pp = &head;
+
+	while (a || b) {
+		struct line_log_data *src;
+		struct line_log_data *src2 = NULL;
+		struct line_log_data *d;
+		int cmp;
+		if (!a)
+			cmp = 1;
+		else if (!b)
+			cmp = -1;
+		else
+			cmp = strcmp(a->spec->path, b->spec->path);
+		if (cmp < 0) {
+			src = a;
+			a = a->next;
+		} else if (cmp == 0) {
+			src = a;
+			a = a->next;
+			src2 = b;
+			b = b->next;
+		} else {
+			src = b;
+			b = b->next;
+		}
+		d = xmalloc(sizeof(struct line_log_data));
+		line_log_data_init(d);
+		d->spec = src->spec;
+		d->spec->count++;
+		*pp = d;
+		pp = &d->next;
+		if (src2)
+			range_set_union(&d->ranges, &src->ranges, &src2->ranges);
+		else
+			range_set_copy(&d->ranges, &src->ranges);
+	}
+
+	return head;
+}
+
+static void add_line_range(struct rev_info *revs, struct commit *commit,
+			   struct line_log_data *range)
+{
+	struct line_log_data *old = NULL;
+	struct line_log_data *new = NULL;
+
+	old = lookup_decoration(&revs->line_log_data, &commit->object);
+	if (old && range) {
+		new = line_log_data_merge(old, range);
+		free_line_log_data(old);
+	} else if (range)
+		new = line_log_data_copy(range);
+
+	if (new)
+		add_decoration(&revs->line_log_data, &commit->object, new);
+}
+
+static void clear_commit_line_range(struct rev_info *revs, struct commit *commit)
+{
+	struct line_log_data *r;
+	r = lookup_decoration(&revs->line_log_data, &commit->object);
+	if (!r)
+		return;
+	free_line_log_data(r);
+	add_decoration(&revs->line_log_data, &commit->object, NULL);
+}
+
+static struct line_log_data *lookup_line_range(struct rev_info *revs,
+					       struct commit *commit)
+{
+	struct line_log_data *ret = NULL;
+
+	ret = lookup_decoration(&revs->line_log_data, &commit->object);
+	return ret;
+}
+
+void line_log_init(struct rev_info *rev, const char *prefix, struct string_list *args)
+{
+	struct commit *commit = NULL;
+	struct line_log_data *range;
+
+	commit = check_single_commit(rev);
+	range = parse_lines(commit, prefix, args);
+	add_line_range(rev, commit, range);
+
+	if (!rev->diffopt.detect_rename) {
+		int i, count = 0;
+		struct line_log_data *r = range;
+		const char **paths;
+		while (r) {
+			count++;
+			r = r->next;
+		}
+		paths = xmalloc((count+1)*sizeof(char *));
+		r = range;
+		for (i = 0; i < count; i++) {
+			paths[i] = xstrdup(r->spec->path);
+			r = r->next;
+		}
+		paths[count] = NULL;
+		init_pathspec(&rev->diffopt.pathspec, paths);
+		free(paths);
+	}
+}
+
+static void load_tree_desc(struct tree_desc *desc, void **tree,
+			   const unsigned char *sha1)
+{
+	unsigned long size;
+	*tree = read_object_with_reference(sha1, tree_type, &size, NULL);
+	if (!*tree)
+		die("Unable to read tree (%s)", sha1_to_hex(sha1));
+	init_tree_desc(desc, *tree, size);
+}
+
+static int count_parents(struct commit *commit)
+{
+	struct commit_list *parents = commit->parents;
+	int count = 0;
+	while (parents) {
+		count++;
+		parents = parents->next;
+	}
+	return count;
+}
+
+static void move_diff_queue(struct diff_queue_struct *dst,
+			    struct diff_queue_struct *src)
+{
+	assert(src != dst);
+	memcpy(dst, src, sizeof(struct diff_queue_struct));
+	DIFF_QUEUE_CLEAR(src);
+}
+
+static void queue_diffs(struct diff_options *opt,
+			struct diff_queue_struct *queue,
+			struct commit *commit, struct commit *parent)
+{
+	void *tree1 = NULL, *tree2 = NULL;
+	struct tree_desc desc1, desc2;
+
+	assert(commit);
+	load_tree_desc(&desc2, &tree2, commit->tree->object.sha1);
+	if (parent)
+		load_tree_desc(&desc1, &tree1, parent->tree->object.sha1);
+	else
+		init_tree_desc(&desc1, "", 0);
+
+	DIFF_QUEUE_CLEAR(&diff_queued_diff);
+	diff_tree(&desc1, &desc2, "", opt);
+	diffcore_std(opt);
+	move_diff_queue(queue, &diff_queued_diff);
+
+	if (tree1)
+		free(tree1);
+	if (tree2)
+		free(tree2);
+}
+
+static char *get_nth_line(long line, unsigned long *ends, void *data)
+{
+	if (line == 0)
+		return (char *)data;
+	else
+		return (char *)data + ends[line] + 1;
+}
+
+static void print_line(const char *prefix, char first,
+		       long line, unsigned long *ends, void *data,
+		       const char *color, const char *reset)
+{
+	char *begin = get_nth_line(line, ends, data);
+	char *end = get_nth_line(line+1, ends, data);
+	int had_nl = 0;
+
+	if (end > begin && end[-1] == '\n') {
+		end--;
+		had_nl = 1;
+	}
+
+	fputs(prefix, stdout);
+	fputs(color, stdout);
+	putchar(first);
+	fwrite(begin, 1, end-begin, stdout);
+	fputs(reset, stdout);
+	putchar('\n');
+	if (!had_nl)
+		fputs("\\ No newline at end of file\n", stdout);
+}
+
+static char *output_prefix(struct diff_options *opt)
+{
+	char *prefix = "";
+
+	if (opt->output_prefix) {
+		struct strbuf *sb = opt->output_prefix(opt, opt->output_prefix_data);
+		prefix = sb->buf;
+	}
+
+	return prefix;
+}
+
+static void dump_diff_hacky_one(struct rev_info *rev, struct line_log_data *range)
+{
+	int i, j = 0;
+	long p_lines, t_lines;
+	unsigned long *p_ends = NULL, *t_ends = NULL;
+	struct diff_filepair *pair = range->pair;
+	struct diff_ranges *diff = &range->diff;
+
+	struct diff_options *opt = &rev->diffopt;
+	char *prefix = output_prefix(opt);
+	const char *c_reset = diff_get_color(opt->use_color, DIFF_RESET);
+	const char *c_frag = diff_get_color(opt->use_color, DIFF_FRAGINFO);
+	const char *c_meta = diff_get_color(opt->use_color, DIFF_METAINFO);
+	const char *c_old = diff_get_color(opt->use_color, DIFF_FILE_OLD);
+	const char *c_new = diff_get_color(opt->use_color, DIFF_FILE_NEW);
+	const char *c_plain = diff_get_color(opt->use_color, DIFF_PLAIN);
+
+	if (!pair || !diff)
+		return;
+
+	if (pair->one->sha1_valid)
+		fill_line_ends(pair->one, &p_lines, &p_ends);
+	fill_line_ends(pair->two, &t_lines, &t_ends);
+
+	printf("%s%sdiff --git a/%s b/%s%s\n", prefix, c_meta, pair->one->path, pair->two->path, c_reset);
+	printf("%s%s--- %s%s%s\n", prefix, c_meta,
+	       pair->one->sha1_valid ? "a/" : "",
+	       pair->one->sha1_valid ? pair->one->path : "/dev/null",
+	       c_reset);
+	printf("%s%s+++ b/%s%s\n", prefix, c_meta, pair->two->path, c_reset);
+	for (i = 0; i < range->ranges.nr; i++) {
+		long p_start, p_end;
+		long t_start = range->ranges.ranges[i].start;
+		long t_end = range->ranges.ranges[i].end;
+		long t_cur = t_start;
+		int j_last;
+
+		while (j < diff->target.nr && diff->target.ranges[j].end < t_start)
+			j++;
+		if (j == diff->target.nr || diff->target.ranges[j].start > t_end)
+			continue;
+
+		/* Scan ahead to determine the last diff that falls in this range */
+		j_last = j;
+		while (j_last < diff->target.nr && diff->target.ranges[j_last].start < t_end)
+			j_last++;
+		if (j_last > j)
+			j_last--;
+
+		/*
+		 * Compute parent hunk headers: we know that the diff
+		 * has the correct line numbers (but not all hunks).
+		 * So it suffices to shift the start/end according to
+		 * the line numbers of the first/last hunk(s) that
+		 * fall in this range.
+		 */
+		if (t_start < diff->target.ranges[j].start)
+			p_start = diff->parent.ranges[j].start - (diff->target.ranges[j].start-t_start);
+		else
+			p_start = diff->parent.ranges[j].start;
+		if (t_end > diff->target.ranges[j_last].end)
+			p_end = diff->parent.ranges[j_last].end + (t_end-diff->target.ranges[j_last].end);
+		else
+			p_end = diff->parent.ranges[j_last].end;
+
+		if (!p_start && !p_end) {
+			p_start = -1;
+			p_end = -1;
+		}
+
+		/* Now output a diff hunk for this range */
+		printf("%s%s@@ -%ld,%ld +%ld,%ld @@%s\n",
+		       prefix, c_frag,
+		       p_start+1, p_end-p_start, t_start+1, t_end-t_start,
+		       c_reset);
+		while (j < diff->target.nr && diff->target.ranges[j].start < t_end) {
+			int k;
+			for (; t_cur < diff->target.ranges[j].start; t_cur++)
+				print_line(prefix, ' ', t_cur, t_ends, pair->two->data,
+					   c_plain, c_reset);
+			for (k = diff->parent.ranges[j].start; k < diff->parent.ranges[j].end; k++)
+				print_line(prefix, '-', k, p_ends, pair->one->data,
+					   c_old, c_reset);
+			for (; t_cur < diff->target.ranges[j].end && t_cur < t_end; t_cur++)
+				print_line(prefix, '+', t_cur, t_ends, pair->two->data,
+					   c_new, c_reset);
+			j++;
+		}
+		for (; t_cur < t_end; t_cur++)
+			print_line(prefix, ' ', t_cur, t_ends, pair->two->data,
+				   c_plain, c_reset);
+	}
+
+	free(p_ends);
+	free(t_ends);
+}
+
+/*
+ * NEEDSWORK: manually building a diff here is not the Right
+ * Thing(tm).  log -L should be built into the diff pipeline.
+ */
+static void dump_diff_hacky(struct rev_info *rev, struct line_log_data *range)
+{
+	puts(output_prefix(&rev->diffopt));
+	while (range) {
+		dump_diff_hacky_one(rev, range);
+		range = range->next;
+	}
+}
+
+/*
+ * Unlike most other functions, this destructively operates on
+ * 'range'.
+ */
+static int process_diff_filepair(struct rev_info *rev,
+				 struct diff_filepair *pair,
+				 struct line_log_data *range,
+				 struct diff_ranges **diff_out)
+{
+	struct line_log_data *rg = range;
+	struct range_set tmp;
+	struct diff_ranges diff;
+	mmfile_t file_parent, file_target;
+
+	assert(pair->two->path);
+	while (rg) {
+		assert(rg->spec->path);
+		if (!strcmp(rg->spec->path, pair->two->path))
+			break;
+		rg = rg->next;
+	}
+
+	if (!rg)
+		return 0;
+	if (rg->ranges.nr == 0)
+		return 0;
+
+	assert(pair->two->sha1_valid);
+	diff_populate_filespec(pair->two, 0);
+	file_target.ptr = pair->two->data;
+	file_target.size = pair->two->size;
+
+	if (pair->one->sha1_valid) {
+		diff_populate_filespec(pair->one, 0);
+		file_parent.ptr = pair->one->data;
+		file_parent.size = pair->one->size;
+	} else {
+		file_parent.ptr = "";
+		file_parent.size = 0;
+	}
+
+	diff_ranges_init(&diff);
+	collect_diff(&file_parent, &file_target, &diff);
+
+	/* NEEDSWORK should apply some heuristics to prevent mismatches */
+	rg->spec->path = xstrdup(pair->one->path);
+
+	range_set_init(&tmp, 0);
+	range_set_map_across_diff(&tmp, &rg->ranges, &diff, diff_out);
+	range_set_release(&rg->ranges);
+	range_set_move(&rg->ranges, &tmp);
+
+	diff_ranges_release(&diff);
+
+	return ((*diff_out)->parent.nr > 0);
+}
+
+static struct diff_filepair *diff_filepair_dup(struct diff_filepair *pair)
+{
+	struct diff_filepair *new = xmalloc(sizeof(struct diff_filepair));
+	new->one = pair->one;
+	new->two = pair->two;
+	new->one->count++;
+	new->two->count++;
+	return new;
+}
+
+static void free_diffqueues(int n, struct diff_queue_struct *dq)
+{
+	int i, j;
+	for (i = 0; i < n; i++)
+		for (j = 0; j < dq[i].nr; j++)
+			diff_free_filepair(dq[i].queue[j]);
+	free(dq);
+}
+
+static int process_all_files(struct line_log_data **range_out,
+			     struct rev_info *rev,
+			     struct diff_queue_struct *queue,
+			     struct line_log_data *range)
+{
+	int i, changed = 0;
+
+	*range_out = line_log_data_copy(range);
+
+	for (i = 0; i < queue->nr; i++) {
+		struct diff_ranges *pairdiff = NULL;
+		if (process_diff_filepair(rev, queue->queue[i], *range_out, &pairdiff)) {
+			struct line_log_data *rg = range;
+			changed++;
+			/* NEEDSWORK tramples over data structures not owned here */
+			while (rg && strcmp(rg->spec->path, queue->queue[i]->two->path))
+				rg = rg->next;
+			assert(rg);
+			rg->pair = diff_filepair_dup(queue->queue[i]);
+			memcpy(&rg->diff, pairdiff, sizeof(struct diff_ranges));
+		}
+	}
+
+	return changed;
+}
+
+int line_log_print(struct rev_info *rev, struct commit *commit)
+{
+	struct line_log_data *range = lookup_line_range(rev, commit);
+
+	show_log(rev);
+	dump_diff_hacky(rev, range);
+	return 1;
+}
+
+static int process_ranges_ordinary_commit(struct rev_info *rev, struct commit *commit,
+					  struct line_log_data *range)
+{
+	struct commit *parent = NULL;
+	struct diff_queue_struct queue;
+	struct line_log_data *parent_range;
+	int changed;
+
+	if (commit->parents)
+		parent = commit->parents->item;
+
+	queue_diffs(&rev->diffopt, &queue, commit, parent);
+	changed = process_all_files(&parent_range, rev, &queue, range);
+	if (parent)
+		add_line_range(rev, parent, parent_range);
+	return changed;
+}
+
+static int process_ranges_merge_commit(struct rev_info *rev, struct commit *commit,
+				       struct line_log_data *range)
+{
+	struct diff_queue_struct *diffqueues;
+	struct line_log_data **cand;
+	struct commit **parents;
+	struct commit_list *p;
+	int i;
+	int nparents = count_parents(commit);
+
+	diffqueues = xmalloc(nparents * sizeof(*diffqueues));
+	cand = xmalloc(nparents * sizeof(*cand));
+	parents = xmalloc(nparents * sizeof(*parents));
+
+	p = commit->parents;
+	for (i = 0; i < nparents; i++) {
+		parents[i] = p->item;
+		p = p->next;
+		queue_diffs(&rev->diffopt, &diffqueues[i], commit, parents[i]);
+	}
+
+	for (i = 0; i < nparents; i++) {
+		int changed;
+		cand[i] = NULL;
+		changed = process_all_files(&cand[i], rev, &diffqueues[i], range);
+		if (!changed) {
+			/*
+			 * This parent can take all the blame, so we
+			 * don't follow any other path in history
+			 */
+			add_line_range(rev, parents[i], cand[i]);
+			clear_commit_line_range(rev, commit);
+			commit->parents = xmalloc(sizeof(struct commit_list));
+			commit->parents->item = parents[i];
+			commit->parents->next = NULL;
+			free(parents);
+			free(cand);
+			free_diffqueues(nparents, diffqueues);
+			/* NEEDSWORK leaking like a sieve */
+			return 0;
+		}
+	}
+
+	/*
+	 * No single parent took the blame.  We add the candidates
+	 * from the above loop to the parents.
+	 */
+	for (i = 0; i < nparents; i++) {
+		add_line_range(rev, parents[i], cand[i]);
+	}
+
+	clear_commit_line_range(rev, commit);
+	free(parents);
+	free(cand);
+	free_diffqueues(nparents, diffqueues);
+	return 1;
+
+	/* NEEDSWORK evil merge detection stuff */
+	/* NEEDSWORK leaking like a sieve */
+}
+
+static int process_ranges_arbitrary_commit(struct rev_info *rev, struct commit *commit)
+{
+	struct line_log_data *range = lookup_line_range(rev, commit);
+	int changed = 0;
+
+	if (range) {
+		if (!commit->parents || !commit->parents->next)
+			changed = process_ranges_ordinary_commit(rev, commit, range);
+		else
+			changed = process_ranges_merge_commit(rev, commit, range);
+	}
+
+	if (!changed)
+		commit->object.flags |= TREESAME;
+
+	return changed;
+}
+
+static enum rewrite_result line_log_rewrite_one(struct rev_info *rev, struct commit **pp)
+{
+	for (;;) {
+		struct commit *p = *pp;
+		if (p->parents && p->parents->next)
+			return rewrite_one_ok;
+		if (p->object.flags & UNINTERESTING)
+			return rewrite_one_ok;
+		if (!(p->object.flags & TREESAME))
+			return rewrite_one_ok;
+		if (!p->parents)
+			return rewrite_one_noparents;
+		*pp = p->parents->item;
+	}
+}
+
+int line_log_filter(struct rev_info *rev)
+{
+	struct commit *commit;
+	struct commit_list *list = rev->commits;
+	struct commit_list *out = NULL, **pp = &out;
+
+	while (list) {
+		struct commit_list *to_free = NULL;
+		commit = list->item;
+		if (process_ranges_arbitrary_commit(rev, commit)) {
+			*pp = list;
+			pp = &list->next;
+		} else
+			to_free = list;
+		list = list->next;
+		free(to_free);
+	}
+	*pp = NULL;
+
+	for (list = out; list; list = list->next)
+		rewrite_parents(rev, list->item, line_log_rewrite_one);
+
+	rev->commits = out;
+
+	return 0;
+}
diff --git a/line-log.h b/line-log.h
new file mode 100644
index 0000000..9acd123
--- /dev/null
+++ b/line-log.h
@@ -0,0 +1,49 @@
+#ifndef LINE_LOG_H
+#define LINE_LOG_H
+
+#include "diffcore.h"
+
+struct rev_info;
+struct commit;
+
+/* A range [start,end].  Lines are numbered starting at 0, and the
+ * ranges include start but exclude end. */
+struct range {
+	long start, end;
+};
+
+/* A set of ranges.  The ranges must always be disjoint and sorted. */
+struct range_set {
+	int alloc, nr;
+	struct range *ranges;
+};
+
+/* A diff, encoded as the set of pre- and post-image ranges where the
+ * files differ. A pair of ranges corresponds to a hunk. */
+struct diff_ranges {
+	struct range_set parent;
+	struct range_set target;
+};
+
+/* Linked list of interesting files and their associated ranges.  The
+ * list must be kept sorted by spec->path */
+struct line_log_data {
+	struct line_log_data *next;
+	struct diff_filespec *spec;
+	char status;
+	struct range_set ranges;
+	int arg_alloc, arg_nr;
+	const char **args;
+	struct diff_filepair *pair;
+	struct diff_ranges diff;
+};
+
+extern void line_log_data_init(struct line_log_data *r);
+
+extern void line_log_init(struct rev_info *rev, const char *prefix, struct string_list *args);
+
+extern int line_log_filter(struct rev_info *rev);
+
+extern int line_log_print(struct rev_info *rev, struct commit *commit);
+
+#endif /* LINE_LOG_H */
diff --git a/line-range.c b/line-range.c
index 0af43bf..d0c7dac 100644
--- a/line-range.c
+++ b/line-range.c
@@ -21,6 +21,8 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,
 	if (1 < begin && (spec[0] == '+' || spec[0] == '-')) {
 		num = strtol(spec + 1, &term, 10);
 		if (term != spec + 1) {
+			if (!ret)
+				return term;
 			if (spec[0] == '-')
 				num = 0 - num;
 			if (0 < num)
@@ -35,7 +37,8 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,
 	}
 	num = strtol(spec, &term, 10);
 	if (term != spec) {
-		*ret = num;
+		if (ret)
+			*ret = num;
 		return term;
 	}
 	if (spec[0] != '/')
@@ -49,6 +52,10 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,
 	if (*term != '/')
 		return spec;
 
+	/* in the scan-only case we are not interested in the regex */
+	if (!ret)
+		return term+1;
+
 	/* try [spec+1 .. term-1] as regexp */
 	*term = 0;
 	begin--; /* input is in human terms */
@@ -90,3 +97,13 @@ int parse_range_arg(const char *arg, nth_line_fn_t nth_line_cb,
 
 	return 0;
 }
+
+const char *skip_range_arg(const char *arg)
+{
+	arg = parse_loc(arg, NULL, NULL, 0, -1, NULL);
+
+	if (*arg == ',')
+		arg = parse_loc(arg+1, NULL, NULL, 0, 0, NULL);
+
+	return arg;
+}
diff --git a/line-range.h b/line-range.h
index 830f25b..88aaf08 100644
--- a/line-range.h
+++ b/line-range.h
@@ -21,4 +21,15 @@ extern int parse_range_arg(const char *arg,
 			   void *cb_data, long lines,
 			   long *begin, long *end);
 
+/*
+ * Scan past a range argument that could be parsed by
+ * 'parse_range_arg', to help the caller determine the start of the
+ * filename in '-L n,m:file' syntax.
+ *
+ * Returns a pointer to the first character after the 'n,m' part, or
+ * NULL in case the argument is obviously malformed.
+ */
+
+extern const char *skip_range_arg(const char *arg);
+
 #endif /* LINE_RANGE_H */
diff --git a/log-tree.c b/log-tree.c
index 5dc45c4..793d2c8 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -9,6 +9,7 @@
 #include "string-list.h"
 #include "color.h"
 #include "gpg-interface.h"
+#include "line-log.h"
 
 struct decoration name_decoration = { "object names" };
 
@@ -856,6 +857,9 @@ int log_tree_commit(struct rev_info *opt, struct commit *commit)
 	log.parent = NULL;
 	opt->loginfo = &log;
 
+	if (opt->line_level_traverse)
+		return line_log_print(opt, commit);
+
 	shown = log_tree_diff(opt, commit, &log);
 	if (!shown && opt->loginfo && opt->always_show_header) {
 		log.parent = NULL;
diff --git a/revision.c b/revision.c
index 46319d5..3ad9d08 100644
--- a/revision.c
+++ b/revision.c
@@ -13,6 +13,7 @@
 #include "decorate.h"
 #include "log-tree.h"
 #include "string-list.h"
+#include "line-log.h"
 #include "mailmap.h"
 
 volatile show_early_output_fn_t show_early_output;
@@ -1896,6 +1897,12 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 	if (revs->combine_merges)
 		revs->ignore_merges = 0;
 	revs->diffopt.abbrev = revs->abbrev;
+
+	if (revs->line_level_traverse) {
+		revs->limited = 1;
+		revs->topo_order = 1;
+	}
+
 	diff_setup_done(&revs->diffopt);
 
 	grep_commit_pattern_type(GREP_PATTERN_TYPE_UNSPECIFIED,
@@ -2166,6 +2173,8 @@ int prepare_revision_walk(struct rev_info *revs)
 			return -1;
 	if (revs->topo_order)
 		sort_in_topological_order(&revs->commits, revs->lifo);
+	if (revs->line_level_traverse)
+		line_log_filter(revs);
 	if (revs->simplify_merges)
 		simplify_merges(revs);
 	if (revs->children.name)
diff --git a/revision.h b/revision.h
index 640110d..6b2870c 100644
--- a/revision.h
+++ b/revision.h
@@ -96,7 +96,8 @@ struct rev_info {
 			cherry_mark:1,
 			bisect:1,
 			ancestry_path:1,
-			first_parent_only:1;
+			first_parent_only:1,
+			line_level_traverse:1;
 
 	/* Diff flags */
 	unsigned int	diff:1,
@@ -175,6 +176,9 @@ struct rev_info {
 	int count_left;
 	int count_right;
 	int count_same;
+
+	/* line level range that we are chasing */
+	struct decoration line_log_data;
 };
 
 #define REV_TREE_SAME		0
diff --git a/t/perf/p4211-line-log.sh b/t/perf/p4211-line-log.sh
new file mode 100755
index 0000000..3d074b0
--- /dev/null
+++ b/t/perf/p4211-line-log.sh
@@ -0,0 +1,34 @@
+#!/bin/sh
+
+test_description='Tests log -L performance'
+. ./perf-lib.sh
+
+test_perf_default_repo
+
+# Pick a file to log pseudo-randomly.  The sort key is the blob hash,
+# so it is stable.
+test_expect_success 'select a file' '
+	git ls-tree HEAD | grep ^100644 |
+	sort -k 3 | head -1 | cut -f 2 >filelist
+'
+
+file=$(cat filelist)
+export file
+
+test_perf 'git rev-list --topo-order (baseline)' '
+	git rev-list --topo-order HEAD >/dev/null
+'
+
+test_perf 'git log --follow (baseline for -M)' '
+	git log --oneline --follow -- "$file" >/dev/null
+'
+
+test_perf 'git log -L' '
+	git log -L 1:"$file" >/dev/null
+'
+
+test_perf 'git log -M -L' '
+	git log -M -L 1:"$file" >/dev/null
+'
+
+test_done
diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh
new file mode 100755
index 0000000..286390d
--- /dev/null
+++ b/t/t4211-line-log.sh
@@ -0,0 +1,38 @@
+#!/bin/sh
+
+test_description='test log -L'
+. ./test-lib.sh
+
+test_expect_success 'setup (import history)' '
+	git fast-import < "$TEST_DIRECTORY"/t4211/history.export &&
+	git reset --hard
+'
+
+canned_test () {
+	test_expect_success "$1" "
+		git log $1 >actual &&
+		test_cmp \"\$TEST_DIRECTORY\"/t4211/expect.$2 actual
+	"
+}
+
+canned_test "-L 4,12:a.c simple" simple-f
+canned_test "-L 4,+9:a.c simple" simple-f
+canned_test "-L '/long f/,/^}/:a.c' simple" simple-f
+
+canned_test "-L '/main/,/^}/:a.c' simple" simple-main
+
+canned_test "-L 1,+4:a.c simple" beginning-of-file
+
+canned_test "-L 20:a.c simple" end-of-file
+
+test_expect_success "-L 20,10000:a.c (bogus end)" '
+	test_must_fail git log -L 20,10000:a.c simple 2>errors &&
+	grep "has only.*lines" errors
+'
+
+canned_test "-L '/long f/',/^}/:a.c -L /main/,/^}/:a.c simple" two-ranges
+canned_test "-L 24,+1:a.c simple" vanishes-early
+
+canned_test "-L '/long f/,/^}/:b.c' move-support" move-support-f
+
+test_done
diff --git a/t/t4211/expect.beginning-of-file b/t/t4211/expect.beginning-of-file
new file mode 100644
index 0000000..91b4054
--- /dev/null
+++ b/t/t4211/expect.beginning-of-file
@@ -0,0 +1,43 @@
+commit 4a23ae5c98d59a58c6da036156959f2dc9f472ad
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:47:40 2013 +0100
+
+    change at very beginning
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -1,3 +1,4 @@
++#include <unistd.h>
+ #include <stdio.h>
+ 
+ long f(long x)
+
+commit a6eb82647d5d67f893da442f8f9375fd89a3b1e2
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:45:16 2013 +0100
+
+    touch both functions
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -1,3 +1,3 @@
+ #include <stdio.h>
+ 
+-int f(int x)
++long f(long x)
+
+commit de4c48ae814792c02a49c4c3c0c757ae69c55f6a
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:44:48 2013 +0100
+
+    initial
+
+diff --git a/a.c b/a.c
+--- /dev/null
++++ b/a.c
+@@ -0,0 +1,3 @@
++#include <stdio.h>
++
++int f(int x)
diff --git a/t/t4211/expect.end-of-file b/t/t4211/expect.end-of-file
new file mode 100644
index 0000000..bd25bb2
--- /dev/null
+++ b/t/t4211/expect.end-of-file
@@ -0,0 +1,62 @@
+commit 4659538844daa2849b1a9e7d6fadb96fcd26fc83
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:48:43 2013 +0100
+
+    change back to complete line
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -20,3 +20,5 @@
+ 	printf("%ld\n", f(15));
+ 	return 0;
+-}
+\ No newline at end of file
++}
++
++/* incomplete lines are bad! */
+
+commit 100b61a6f2f720f812620a9d10afb3a960ccb73c
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:48:10 2013 +0100
+
+    change to an incomplete line at end
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -20,3 +20,3 @@
+ 	printf("%ld\n", f(15));
+ 	return 0;
+-}
++}
+\ No newline at end of file
+
+commit a6eb82647d5d67f893da442f8f9375fd89a3b1e2
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:45:16 2013 +0100
+
+    touch both functions
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -19,3 +19,3 @@
+-	printf("%d\n", f(15));
++	printf("%ld\n", f(15));
+ 	return 0;
+ }
+
+commit de4c48ae814792c02a49c4c3c0c757ae69c55f6a
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:44:48 2013 +0100
+
+    initial
+
+diff --git a/a.c b/a.c
+--- /dev/null
++++ b/a.c
+@@ -0,0 +18,3 @@
++	printf("%d\n", f(15));
++	return 0;
++}
diff --git a/t/t4211/expect.move-support-f b/t/t4211/expect.move-support-f
new file mode 100644
index 0000000..78a8cf1
--- /dev/null
+++ b/t/t4211/expect.move-support-f
@@ -0,0 +1,40 @@
+commit 6ce3c4ff690136099bb17e1a8766b75764726ea7
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:49:50 2013 +0100
+
+    another simple change
+
+diff --git a/b.c b/b.c
+--- a/b.c
++++ b/b.c
+@@ -4,9 +4,9 @@
+ long f(long x)
+ {
+ 	int s = 0;
+ 	while (x) {
+-		x >>= 1;
++		x /= 2;
+ 		s++;
+ 	}
+ 	return s;
+ }
+
+commit e6da343666244ea9e67cbe3f3bd26da860f9fe0e
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:49:28 2013 +0100
+
+    move file
+
+diff --git a/b.c b/b.c
+--- /dev/null
++++ b/b.c
+@@ -0,0 +4,9 @@
++long f(long x)
++{
++	int s = 0;
++	while (x) {
++		x >>= 1;
++		s++;
++	}
++	return s;
++}
diff --git a/t/t4211/expect.simple-f b/t/t4211/expect.simple-f
new file mode 100644
index 0000000..a1f5bc4
--- /dev/null
+++ b/t/t4211/expect.simple-f
@@ -0,0 +1,59 @@
+commit a6eb82647d5d67f893da442f8f9375fd89a3b1e2
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:45:16 2013 +0100
+
+    touch both functions
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -3,9 +3,9 @@
+-int f(int x)
++long f(long x)
+ {
+ 	int s = 0;
+ 	while (x) {
+ 		x >>= 1;
+ 		s++;
+ 	}
+ 	return s;
+ }
+
+commit f04fb20f2c77850996cba739709acc6faecc58f7
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:44:55 2013 +0100
+
+    change f()
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -3,8 +3,9 @@
+ int f(int x)
+ {
+ 	int s = 0;
+ 	while (x) {
+ 		x >>= 1;
+ 		s++;
+ 	}
++	return s;
+ }
+
+commit de4c48ae814792c02a49c4c3c0c757ae69c55f6a
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:44:48 2013 +0100
+
+    initial
+
+diff --git a/a.c b/a.c
+--- /dev/null
++++ b/a.c
+@@ -0,0 +3,8 @@
++int f(int x)
++{
++	int s = 0;
++	while (x) {
++		x >>= 1;
++		s++;
++	}
++}
diff --git a/t/t4211/expect.simple-main b/t/t4211/expect.simple-main
new file mode 100644
index 0000000..39ce39b
--- /dev/null
+++ b/t/t4211/expect.simple-main
@@ -0,0 +1,68 @@
+commit 4659538844daa2849b1a9e7d6fadb96fcd26fc83
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:48:43 2013 +0100
+
+    change back to complete line
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -18,5 +18,5 @@
+ int main ()
+ {
+ 	printf("%ld\n", f(15));
+ 	return 0;
+-}
+\ No newline at end of file
++}
+
+commit 100b61a6f2f720f812620a9d10afb3a960ccb73c
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:48:10 2013 +0100
+
+    change to an incomplete line at end
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -18,5 +18,5 @@
+ int main ()
+ {
+ 	printf("%ld\n", f(15));
+ 	return 0;
+-}
++}
+\ No newline at end of file
+
+commit a6eb82647d5d67f893da442f8f9375fd89a3b1e2
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:45:16 2013 +0100
+
+    touch both functions
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -17,5 +17,5 @@
+ int main ()
+ {
+-	printf("%d\n", f(15));
++	printf("%ld\n", f(15));
+ 	return 0;
+ }
+
+commit de4c48ae814792c02a49c4c3c0c757ae69c55f6a
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:44:48 2013 +0100
+
+    initial
+
+diff --git a/a.c b/a.c
+--- /dev/null
++++ b/a.c
+@@ -0,0 +16,5 @@
++int main ()
++{
++	printf("%d\n", f(15));
++	return 0;
++}
diff --git a/t/t4211/expect.two-ranges b/t/t4211/expect.two-ranges
new file mode 100644
index 0000000..6109aa0
--- /dev/null
+++ b/t/t4211/expect.two-ranges
@@ -0,0 +1,102 @@
+commit 4659538844daa2849b1a9e7d6fadb96fcd26fc83
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:48:43 2013 +0100
+
+    change back to complete line
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -18,5 +18,5 @@
+ int main ()
+ {
+ 	printf("%ld\n", f(15));
+ 	return 0;
+-}
+\ No newline at end of file
++}
+
+commit 100b61a6f2f720f812620a9d10afb3a960ccb73c
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:48:10 2013 +0100
+
+    change to an incomplete line at end
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -18,5 +18,5 @@
+ int main ()
+ {
+ 	printf("%ld\n", f(15));
+ 	return 0;
+-}
++}
+\ No newline at end of file
+
+commit a6eb82647d5d67f893da442f8f9375fd89a3b1e2
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:45:16 2013 +0100
+
+    touch both functions
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -3,9 +3,9 @@
+-int f(int x)
++long f(long x)
+ {
+ 	int s = 0;
+ 	while (x) {
+ 		x >>= 1;
+ 		s++;
+ 	}
+ 	return s;
+ }
+@@ -17,5 +17,5 @@
+ int main ()
+ {
+-	printf("%d\n", f(15));
++	printf("%ld\n", f(15));
+ 	return 0;
+ }
+
+commit f04fb20f2c77850996cba739709acc6faecc58f7
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:44:55 2013 +0100
+
+    change f()
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -3,8 +3,9 @@
+ int f(int x)
+ {
+ 	int s = 0;
+ 	while (x) {
+ 		x >>= 1;
+ 		s++;
+ 	}
++	return s;
+ }
+
+commit de4c48ae814792c02a49c4c3c0c757ae69c55f6a
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:44:48 2013 +0100
+
+    initial
+
+diff --git a/a.c b/a.c
+--- /dev/null
++++ b/a.c
+@@ -0,0 +3,8 @@
++int f(int x)
++{
++	int s = 0;
++	while (x) {
++		x >>= 1;
++		s++;
++	}
++}
diff --git a/t/t4211/expect.vanishes-early b/t/t4211/expect.vanishes-early
new file mode 100644
index 0000000..1f7cd06
--- /dev/null
+++ b/t/t4211/expect.vanishes-early
@@ -0,0 +1,39 @@
+commit 4659538844daa2849b1a9e7d6fadb96fcd26fc83
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:48:43 2013 +0100
+
+    change back to complete line
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -22,1 +24,1 @@
+-}
+\ No newline at end of file
++/* incomplete lines are bad! */
+
+commit 100b61a6f2f720f812620a9d10afb3a960ccb73c
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:48:10 2013 +0100
+
+    change to an incomplete line at end
+
+diff --git a/a.c b/a.c
+--- a/a.c
++++ b/a.c
+@@ -22,1 +22,1 @@
+-}
++}
+\ No newline at end of file
+
+commit de4c48ae814792c02a49c4c3c0c757ae69c55f6a
+Author: Thomas Rast <trast@student.ethz.ch>
+Date:   Thu Feb 28 10:44:48 2013 +0100
+
+    initial
+
+diff --git a/a.c b/a.c
+--- /dev/null
++++ b/a.c
+@@ -0,0 +20,1 @@
++}
diff --git a/t/t4211/history.export b/t/t4211/history.export
new file mode 100644
index 0000000..c159794
--- /dev/null
+++ b/t/t4211/history.export
@@ -0,0 +1,330 @@
+blob
+mark :1
+data 157
+#include <stdio.h>
+
+int f(int x)
+{
+	int s = 0;
+	while (x) {
+		x >>= 1;
+		s++;
+	}
+}
+
+/*
+ * A comment.
+ */
+
+int main ()
+{
+	printf("%d\n", f(15));
+	return 0;
+}
+
+reset refs/tags/simple
+commit refs/tags/simple
+mark :2
+author Thomas Rast <trast@student.ethz.ch> 1362044688 +0100
+committer Thomas Rast <trast@student.ethz.ch> 1362044688 +0100
+data 8
+initial
+M 100644 :1 a.c
+
+blob
+mark :3
+data 168
+#include <stdio.h>
+
+int f(int x)
+{
+	int s = 0;
+	while (x) {
+		x >>= 1;
+		s++;
+	}
+	return s;
+}
+
+/*
+ * A comment.
+ */
+
+int main ()
+{
+	printf("%d\n", f(15));
+	return 0;
+}
+
+commit refs/tags/simple
+mark :4
+author Thomas Rast <trast@student.ethz.ch> 1362044695 +0100
+committer Thomas Rast <trast@student.ethz.ch> 1362044695 +0100
+data 11
+change f()
+from :2
+M 100644 :3 a.c
+
+blob
+mark :5
+data 171
+#include <stdio.h>
+
+long f(long x)
+{
+	int s = 0;
+	while (x) {
+		x >>= 1;
+		s++;
+	}
+	return s;
+}
+
+/*
+ * A comment.
+ */
+
+int main ()
+{
+	printf("%ld\n", f(15));
+	return 0;
+}
+
+commit refs/tags/simple
+mark :6
+author Thomas Rast <trast@student.ethz.ch> 1362044716 +0100
+committer Thomas Rast <trast@student.ethz.ch> 1362044716 +0100
+data 21
+touch both functions
+from :4
+M 100644 :5 a.c
+
+blob
+mark :7
+data 185
+#include <stdio.h>
+
+long f(long x)
+{
+	int s = 0;
+	while (x) {
+		x >>= 1;
+		s++;
+	}
+	return s;
+}
+
+/*
+ * This is only an example!
+ */
+
+int main ()
+{
+	printf("%ld\n", f(15));
+	return 0;
+}
+
+commit refs/tags/simple
+mark :8
+author Thomas Rast <trast@student.ethz.ch> 1362044741 +0100
+committer Thomas Rast <trast@student.ethz.ch> 1362044741 +0100
+data 14
+touch comment
+from :6
+M 100644 :7 a.c
+
+blob
+mark :9
+data 205
+#include <unistd.h>
+#include <stdio.h>
+
+long f(long x)
+{
+	int s = 0;
+	while (x) {
+		x >>= 1;
+		s++;
+	}
+	return s;
+}
+
+/*
+ * This is only an example!
+ */
+
+int main ()
+{
+	printf("%ld\n", f(15));
+	return 0;
+}
+
+commit refs/tags/simple
+mark :10
+author Thomas Rast <trast@student.ethz.ch> 1362044860 +0100
+committer Thomas Rast <trast@student.ethz.ch> 1362044860 +0100
+data 25
+change at very beginning
+from :8
+M 100644 :9 a.c
+
+blob
+mark :11
+data 204
+#include <unistd.h>
+#include <stdio.h>
+
+long f(long x)
+{
+	int s = 0;
+	while (x) {
+		x >>= 1;
+		s++;
+	}
+	return s;
+}
+
+/*
+ * This is only an example!
+ */
+
+int main ()
+{
+	printf("%ld\n", f(15));
+	return 0;
+}
+commit refs/tags/simple
+mark :12
+author Thomas Rast <trast@student.ethz.ch> 1362044890 +0100
+committer Thomas Rast <trast@student.ethz.ch> 1362044890 +0100
+data 36
+change to an incomplete line at end
+from :10
+M 100644 :11 a.c
+
+blob
+mark :13
+data 238
+#include <unistd.h>
+#include <stdio.h>
+
+long f(long x)
+{
+	int s = 0;
+	while (x) {
+		x >>= 1;
+		s++;
+	}
+	return s;
+}
+
+/*
+ * This is only an example!
+ */
+
+int main ()
+{
+	printf("%ld\n", f(15));
+	return 0;
+}
+
+/* incomplete lines are bad! */
+
+commit refs/tags/simple
+mark :14
+author Thomas Rast <trast@student.ethz.ch> 1362044923 +0100
+committer Thomas Rast <trast@student.ethz.ch> 1362044923 +0100
+data 29
+change back to complete line
+from :12
+M 100644 :13 a.c
+
+commit refs/tags/move-support
+mark :15
+author Thomas Rast <trast@student.ethz.ch> 1362044968 +0100
+committer Thomas Rast <trast@student.ethz.ch> 1362044968 +0100
+data 10
+move file
+from :14
+D a.c
+M 100644 :13 b.c
+
+blob
+mark :16
+data 237
+#include <unistd.h>
+#include <stdio.h>
+
+long f(long x)
+{
+	int s = 0;
+	while (x) {
+		x /= 2;
+		s++;
+	}
+	return s;
+}
+
+/*
+ * This is only an example!
+ */
+
+int main ()
+{
+	printf("%ld\n", f(15));
+	return 0;
+}
+
+/* incomplete lines are bad! */
+
+commit refs/tags/move-support
+mark :17
+author Thomas Rast <trast@student.ethz.ch> 1362044990 +0100
+committer Thomas Rast <trast@student.ethz.ch> 1362044990 +0100
+data 22
+another simple change
+from :15
+M 100644 :16 b.c
+
+blob
+mark :18
+data 254
+#include <unistd.h>
+#include <stdio.h>
+
+long f(long x);
+
+/*
+ * This is only an example!
+ */
+
+int main ()
+{
+	printf("%ld\n", f(15));
+	return 0;
+}
+
+/* incomplete lines are bad! */
+
+long f(long x)
+{
+	int s = 0;
+	while (x) {
+		x /= 2;
+		s++;
+	}
+	return s;
+}
+
+commit refs/heads/master
+mark :19
+author Thomas Rast <trast@student.ethz.ch> 1362045024 +0100
+committer Thomas Rast <trast@student.ethz.ch> 1362045024 +0100
+data 21
+move within the file
+from :17
+M 100644 :18 b.c
+
+reset refs/heads/master
+from :19
+
-- 
1.8.2.241.gee8bb87

^ permalink raw reply related

* [PATCH v9 5/5] Speed up log -L... -M
From: Thomas Rast @ 2013-03-21 12:52 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Bo Yang, Zbigniew Jędrzejewski-Szmek,
	Will Palmer
In-Reply-To: <cover.1363865444.git.trast@student.ethz.ch>

So far log -L only used the implicit diff filtering by pathspec.  If
the user specifies -M, we cannot do that, and so we simply handed the
whole diff queue (which is approximately 'git show --raw') to
diffcore_std().

Unfortunately this is very slow.  We can optimize a lot if we throw
out files that we know cannot possibly be interesting, in the same
spirit that the pathspec filtering reduces the number of files.

However, in this case, we have to be more careful.  Because we want to
look out for renames, we need to keep all filepairs where something
was deleted.

This is a bit hacky and should really be replaced by equivalent
support in --follow, and just using that.  However, in the meantime it
speeds up 'log -M -L' by an order of magnitude.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 line-log.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 4 deletions(-)

diff --git a/line-log.c b/line-log.c
index 81c8d74..4327327 100644
--- a/line-log.c
+++ b/line-log.c
@@ -750,7 +750,50 @@ static void move_diff_queue(struct diff_queue_struct *dst,
 	DIFF_QUEUE_CLEAR(src);
 }
 
-static void queue_diffs(struct diff_options *opt,
+static void filter_diffs_for_paths(struct line_log_data *range, int keep_deletions)
+{
+	int i;
+	struct diff_queue_struct outq;
+	DIFF_QUEUE_CLEAR(&outq);
+
+	for (i = 0; i < diff_queued_diff.nr; i++) {
+		struct diff_filepair *p = diff_queued_diff.queue[i];
+		struct line_log_data *rg = NULL;
+
+		if (!DIFF_FILE_VALID(p->two)) {
+			if (keep_deletions)
+				diff_q(&outq, p);
+			else
+				diff_free_filepair(p);
+			continue;
+		}
+		for (rg = range; rg; rg = rg->next) {
+			if (!strcmp(rg->spec->path, p->two->path))
+				break;
+		}
+		if (rg)
+			diff_q(&outq, p);
+		else
+			diff_free_filepair(p);
+	}
+	free(diff_queued_diff.queue);
+	diff_queued_diff = outq;
+}
+
+static inline int diff_might_be_rename(void)
+{
+	int i;
+	for (i = 0; i < diff_queued_diff.nr; i++)
+		if (!DIFF_FILE_VALID(diff_queued_diff.queue[i]->one)) {
+			/* fprintf(stderr, "diff_might_be_rename found creation of: %s\n", */
+			/* 	diff_queued_diff.queue[i]->two->path); */
+			return 1;
+		}
+	return 0;
+}
+
+static void queue_diffs(struct line_log_data *range,
+			struct diff_options *opt,
 			struct diff_queue_struct *queue,
 			struct commit *commit, struct commit *parent)
 {
@@ -766,7 +809,12 @@ static void queue_diffs(struct diff_options *opt,
 
 	DIFF_QUEUE_CLEAR(&diff_queued_diff);
 	diff_tree(&desc1, &desc2, "", opt);
-	diffcore_std(opt);
+	if (opt->detect_rename) {
+		filter_diffs_for_paths(range, 1);
+		if (diff_might_be_rename())
+			diffcore_std(opt);
+		filter_diffs_for_paths(range, 0);
+	}
 	move_diff_queue(queue, &diff_queued_diff);
 
 	if (tree1)
@@ -1050,7 +1098,7 @@ static int process_ranges_ordinary_commit(struct rev_info *rev, struct commit *c
 	if (commit->parents)
 		parent = commit->parents->item;
 
-	queue_diffs(&rev->diffopt, &queue, commit, parent);
+	queue_diffs(range, &rev->diffopt, &queue, commit, parent);
 	changed = process_all_files(&parent_range, rev, &queue, range);
 	if (parent)
 		add_line_range(rev, parent, parent_range);
@@ -1075,7 +1123,7 @@ static int process_ranges_merge_commit(struct rev_info *rev, struct commit *comm
 	for (i = 0; i < nparents; i++) {
 		parents[i] = p->item;
 		p = p->next;
-		queue_diffs(&rev->diffopt, &diffqueues[i], commit, parents[i]);
+		queue_diffs(range, &rev->diffopt, &diffqueues[i], commit, parents[i]);
 	}
 
 	for (i = 0; i < nparents; i++) {
-- 
1.8.2.241.gee8bb87

^ permalink raw reply related

* [PATCH v9 2/5] Export rewrite_parents() for 'log -L'
From: Thomas Rast @ 2013-03-21 12:52 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano, Bo Yang, Zbigniew Jędrzejewski-Szmek,
	Will Palmer
In-Reply-To: <cover.1363865444.git.trast@student.ethz.ch>

From: Bo Yang <struggleyb.nku@gmail.com>

The function rewrite_one is used to rewrite a single
parent of the current commit, and is used by rewrite_parents
to rewrite all the parents.

Decouple the dependence between them by making rewrite_one
a callback function that is passed to rewrite_parents. Then
export rewrite_parents for reuse by the line history browser.

We will use this function in line-log.c.

Signed-off-by: Bo Yang <struggleyb.nku@gmail.com>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 revision.c | 13 ++++---------
 revision.h | 10 ++++++++++
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/revision.c b/revision.c
index ef60205..46319d5 100644
--- a/revision.c
+++ b/revision.c
@@ -2173,12 +2173,6 @@ int prepare_revision_walk(struct rev_info *revs)
 	return 0;
 }
 
-enum rewrite_result {
-	rewrite_one_ok,
-	rewrite_one_noparents,
-	rewrite_one_error
-};
-
 static enum rewrite_result rewrite_one(struct rev_info *revs, struct commit **pp)
 {
 	struct commit_list *cache = NULL;
@@ -2200,12 +2194,13 @@ static enum rewrite_result rewrite_one(struct rev_info *revs, struct commit **pp
 	}
 }
 
-static int rewrite_parents(struct rev_info *revs, struct commit *commit)
+int rewrite_parents(struct rev_info *revs, struct commit *commit,
+	rewrite_parent_fn_t rewrite_parent)
 {
 	struct commit_list **pp = &commit->parents;
 	while (*pp) {
 		struct commit_list *parent = *pp;
-		switch (rewrite_one(revs, &parent->item)) {
+		switch (rewrite_parent(revs, &parent->item)) {
 		case rewrite_one_ok:
 			break;
 		case rewrite_one_noparents:
@@ -2371,7 +2366,7 @@ enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit)
 	if (action == commit_show &&
 	    !revs->show_all &&
 	    revs->prune && revs->dense && want_ancestry(revs)) {
-		if (rewrite_parents(revs, commit) < 0)
+		if (rewrite_parents(revs, commit, rewrite_one) < 0)
 			return commit_error;
 	}
 	return action;
diff --git a/revision.h b/revision.h
index 5da09ee..640110d 100644
--- a/revision.h
+++ b/revision.h
@@ -241,4 +241,14 @@ enum commit_action {
 extern enum commit_action get_commit_action(struct rev_info *revs, struct commit *commit);
 extern enum commit_action simplify_commit(struct rev_info *revs, struct commit *commit);
 
+enum rewrite_result {
+	rewrite_one_ok,
+	rewrite_one_noparents,
+	rewrite_one_error
+};
+
+typedef enum rewrite_result (*rewrite_parent_fn_t)(struct rev_info *revs, struct commit **pp);
+
+extern int rewrite_parents(struct rev_info *revs, struct commit *commit,
+	rewrite_parent_fn_t rewrite_parent);
 #endif
-- 
1.8.2.241.gee8bb87

^ permalink raw reply related

* Re: Which file is older in history?
From: Jeff King @ 2013-03-21 13:26 UTC (permalink / raw)
  To: Ramkumar Ramachandra; +Cc: Junio C Hamano, Git List
In-Reply-To: <CALkWK0nOzUXk0cFh8GCHc=YqLkRJ2XNFgU_tOsua81ROK99mXw@mail.gmail.com>

On Thu, Mar 21, 2013 at 06:02:32PM +0530, Ramkumar Ramachandra wrote:

> >   git log --format=%H --name-status --diff-filter=A -- path1 path2 |
> >   grep ^A |
> >   tail -1
> 
> Great!  I just learnt about --name-status now.

You can also use "--name-only", but the parsing is a little less robust.

> Nit: tail -<n> is deprecated in favor of tail -n <n>, I think.  It's
> nicer to have definite fixed command line options, as opposed to
> parsing an arbitrary -(*) and deciding if \1 is a \d+.

Yeah, POSIX has always specified "-n" for tail, as well as for head.  I
remember in the late 90's and early 00's running into systems where
POSIX "head -n" did not work, and you _had_ to use "head -1". I don't
recall now if that was the case for tail, too, nor what system that was
on (but if I had to guess, I'd say pre-Solaris SunOS).

So yeah. Using "tail -n 1" is probably a better idea in the long run.

-Peff

^ permalink raw reply

* Re: [PATCH 0/4] drop some "int x = x" hacks to silence gcc warnings
From: Joachim Schmitz @ 2013-03-21 13:44 UTC (permalink / raw)
  To: git
In-Reply-To: <514AF2E1.7020409@viscovery.net>

Johannes Sixt wrote:
> Am 3/21/2013 12:03, schrieb Jeff King:
>> I was fooling around with clang and noticed that it complains about
>> the "int x = x" construct under -Wall. That is IMHO a deficiency in
>> clang, since the idiom has a well-defined use in silencing
>> -Wuninitialized warnings.
>
> IMO, that's a myth. The construct invokes undefined behavior at least
> since C99, and the compilers are right to complain about it.

And I complained about this a couple months ago, as the compiler on 
HP-NonStop stumbles across this too (by emitting a warning)

Bye, Jojo 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox