<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/git.git/line-log.c, branch jk/robustify-parse-commit</title>
<subtitle>github.com: git/git.git
</subtitle>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/'/>
<entry>
<title>line-log: fix "log -LN" crash when N is last line of file</title>
<updated>2013-07-23T19:09:48+00:00</updated>
<author>
<name>Eric Sunshine</name>
<email>sunshine@sunshineco.com</email>
</author>
<published>2013-07-23T14:28:08+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=df6308eb82bc9d2074b35387583548b8b153433d'/>
<id>df6308eb82bc9d2074b35387583548b8b153433d</id>
<content type='text'>
range-set invariants are: ranges must be (1) non-empty, (2) disjoint,
(3) sorted in ascending order.

line_log_data_insert() breaks the non-empty invariant under the
following conditions: the incoming range is empty and the pathname
attached to the range has not yet been encountered. In this case,
line_log_data_insert() assigns the empty range to a new line_log_data
record without taking any action to ensure that the empty range is
eventually folded out.  Subsequent range-set functions crash or throw an
assertion failure upon encountering such an anomaly.  Fix this bug.

Signed-off-by: Eric Sunshine &lt;sunshine@sunshineco.com&gt;
Acked-by: Thomas Rast &lt;trast@inf.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
range-set invariants are: ranges must be (1) non-empty, (2) disjoint,
(3) sorted in ascending order.

line_log_data_insert() breaks the non-empty invariant under the
following conditions: the incoming range is empty and the pathname
attached to the range has not yet been encountered. In this case,
line_log_data_insert() assigns the empty range to a new line_log_data
record without taking any action to ensure that the empty range is
eventually folded out.  Subsequent range-set functions crash or throw an
assertion failure upon encountering such an anomaly.  Fix this bug.

Signed-off-by: Eric Sunshine &lt;sunshine@sunshineco.com&gt;
Acked-by: Thomas Rast &lt;trast@inf.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>range-set: satisfy non-empty ranges invariant</title>
<updated>2013-07-23T19:09:14+00:00</updated>
<author>
<name>Eric Sunshine</name>
<email>sunshine@sunshineco.com</email>
</author>
<published>2013-07-23T14:28:06+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=f8395edc6f5c59a92adcf42ea20a01872ec22700'/>
<id>f8395edc6f5c59a92adcf42ea20a01872ec22700</id>
<content type='text'>
range-set invariants are: ranges must be (1) non-empty, (2) disjoint,
(3) sorted in ascending order.

During processing, various range-set utility functions break the
invariants (for instance, by adding empty ranges), with the
expectation that a finalizing sort_and_merge_range_set() will restore
sanity.

sort_and_merge_range_set(), however, neglects to fold out empty
ranges, thus it fails to satisfy the non-empty constraint. Subsequent
range-set functions crash or throw an assertion failure upon
encountering such an anomaly. Rectify the situation by having
sort_and_merge_range_set() fold out empty ranges.

Signed-off-by: Eric Sunshine &lt;sunshine@sunshineco.com&gt;
Acked-by: Thomas Rast &lt;trast@inf.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
range-set invariants are: ranges must be (1) non-empty, (2) disjoint,
(3) sorted in ascending order.

During processing, various range-set utility functions break the
invariants (for instance, by adding empty ranges), with the
expectation that a finalizing sort_and_merge_range_set() will restore
sanity.

sort_and_merge_range_set(), however, neglects to fold out empty
ranges, thus it fails to satisfy the non-empty constraint. Subsequent
range-set functions crash or throw an assertion failure upon
encountering such an anomaly. Rectify the situation by having
sort_and_merge_range_set() fold out empty ranges.

Signed-off-by: Eric Sunshine &lt;sunshine@sunshineco.com&gt;
Acked-by: Thomas Rast &lt;trast@inf.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>range-set: fix sort_and_merge_range_set() corner case bug</title>
<updated>2013-07-23T18:37:46+00:00</updated>
<author>
<name>Eric Sunshine</name>
<email>sunshine@sunshineco.com</email>
</author>
<published>2013-07-23T14:28:04+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=b6679e768f39f718b7f2983d1958f5fb1121f356'/>
<id>b6679e768f39f718b7f2983d1958f5fb1121f356</id>
<content type='text'>
When handed an empty range_set (range_set.nr == 0),
sort_and_merge_range_set() incorrectly sets range_set.nr to 1 at exit.
Subsequent range_set functions then access the bogus range at element
zero and crash or throw an assertion failure. Fix this bug.

Signed-off-by: Eric Sunshine &lt;sunshine@sunshineco.com&gt;
Acked-by: Thomas Rast &lt;trast@inf.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When handed an empty range_set (range_set.nr == 0),
sort_and_merge_range_set() incorrectly sets range_set.nr to 1 at exit.
Subsequent range_set functions then access the bogus range at element
zero and crash or throw an assertion failure. Fix this bug.

Signed-off-by: Eric Sunshine &lt;sunshine@sunshineco.com&gt;
Acked-by: Thomas Rast &lt;trast@inf.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>range_set: fix coalescing bug when range is a subset of another</title>
<updated>2013-07-09T16:25:04+00:00</updated>
<author>
<name>Eric Sunshine</name>
<email>sunshine@sunshineco.com</email>
</author>
<published>2013-07-09T05:55:05+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=3755b53af779ce75fa3ea4581a0e6525bc67278d'/>
<id>3755b53af779ce75fa3ea4581a0e6525bc67278d</id>
<content type='text'>
When coalescing ranges, sort_and_merge_range_set() unconditionally
assumes that the end of a range being folded into a preceding range
should become the end of the coalesced range. This assumption, however,
is invalid when one range is a subset of another.  For example, given
ranges 1-5 and 2-3 added via range_set_append_unsafe(),
sort_and_merge_range_set() incorrectly coalesces them to range 1-3
rather than the correct union range 1-5. Fix this bug.

Signed-off-by: Eric Sunshine &lt;sunshine@sunshineco.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When coalescing ranges, sort_and_merge_range_set() unconditionally
assumes that the end of a range being folded into a preceding range
should become the end of the coalesced range. This assumption, however,
is invalid when one range is a subset of another.  For example, given
ranges 1-5 and 2-3 added via range_set_append_unsafe(),
sort_and_merge_range_set() incorrectly coalesces them to range 1-3
rather than the correct union range 1-5. Fix this bug.

Signed-off-by: Eric Sunshine &lt;sunshine@sunshineco.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>log -L: improve comments in process_all_files()</title>
<updated>2013-04-12T18:37:09+00:00</updated>
<author>
<name>Thomas Rast</name>
<email>trast@inf.ethz.ch</email>
</author>
<published>2013-04-12T16:05:12+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=1ddac3ff9aa29d1103aa05f59772812289b4cefa'/>
<id>1ddac3ff9aa29d1103aa05f59772812289b4cefa</id>
<content type='text'>
The funny range assignment in process_all_files() had me sidetracked
while investigating what led to the previous commit.  Let's improve
the comments.

Signed-off-by: Thomas Rast &lt;trast@inf.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The funny range assignment in process_all_files() had me sidetracked
while investigating what led to the previous commit.  Let's improve
the comments.

Signed-off-by: Thomas Rast &lt;trast@inf.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>log -L: store the path instead of a diff_filespec</title>
<updated>2013-04-12T18:37:03+00:00</updated>
<author>
<name>Thomas Rast</name>
<email>trast@inf.ethz.ch</email>
</author>
<published>2013-04-12T16:05:11+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=31c61918313c7057a48ac939d8699860e8bfff99'/>
<id>31c61918313c7057a48ac939d8699860e8bfff99</id>
<content type='text'>
line_log_data has held a diff_filespec* since the very early versions
of the code.  However, the only place in the code where we actually
need the full filespec is parse_range_arg(); in all other cases, we
are only interested in the path, so there is hardly a reason to store
a filespec.  Even worse, it causes a lot of redundant -&gt;spec-&gt;path
pointer dereferencing.

And *even* worse, it caused the following bug.  If you merge a rename
with a modification to the old filename, like so:

  * Merge
  | \
  |  * Modify foo
  |  |
  *  | Rename foo-&gt;bar
  | /
  * Create foo

we internally -- in process_ranges_merge_commit() -- scan all parents.
We are mainly looking for one that doesn't have any modifications, so
that we can assign all the blame to it and simplify away the merge.
In doing so, we run the normal machinery on all parents in a loop.
For each parent, we prepare a "working set" line_log_data by making a
copy with line_log_data_copy(), which does *not* make a copy of the
spec.

Now suppose the rename is the first parent.  The diff machinery tells
us that the filepair is ('foo', 'bar').  We duly update the path we
are interested in:

  rg-&gt;spec-&gt;path = xstrdup(pair-&gt;one-&gt;path);

But that 'struct spec' is shared between the output line_log_data and
the original input line_log_data.  So we just wrecked the state of
process_ranges_merge_commit().  When we get around to the second
parent, the ranges tell us we are interested in a file 'foo' while the
commits touch 'bar'.

So most of this patch is just s/-&gt;spec-&gt;path/-&gt;path/ and associated
management changes.  This implicitly fixes the bug because we removed
the shared parts between input and output of line_log_data_copy(); it
is now safe to overwrite the path in the copy.

There's one only somewhat related change: the comment in
process_all_files() explains the reasoning behind using 'range' there.
That bit of half-correct code had me sidetracked for a while.

Signed-off-by: Thomas Rast &lt;trast@inf.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
line_log_data has held a diff_filespec* since the very early versions
of the code.  However, the only place in the code where we actually
need the full filespec is parse_range_arg(); in all other cases, we
are only interested in the path, so there is hardly a reason to store
a filespec.  Even worse, it causes a lot of redundant -&gt;spec-&gt;path
pointer dereferencing.

And *even* worse, it caused the following bug.  If you merge a rename
with a modification to the old filename, like so:

  * Merge
  | \
  |  * Modify foo
  |  |
  *  | Rename foo-&gt;bar
  | /
  * Create foo

we internally -- in process_ranges_merge_commit() -- scan all parents.
We are mainly looking for one that doesn't have any modifications, so
that we can assign all the blame to it and simplify away the merge.
In doing so, we run the normal machinery on all parents in a loop.
For each parent, we prepare a "working set" line_log_data by making a
copy with line_log_data_copy(), which does *not* make a copy of the
spec.

Now suppose the rename is the first parent.  The diff machinery tells
us that the filepair is ('foo', 'bar').  We duly update the path we
are interested in:

  rg-&gt;spec-&gt;path = xstrdup(pair-&gt;one-&gt;path);

But that 'struct spec' is shared between the output line_log_data and
the original input line_log_data.  So we just wrecked the state of
process_ranges_merge_commit().  When we get around to the second
parent, the ranges tell us we are interested in a file 'foo' while the
commits touch 'bar'.

So most of this patch is just s/-&gt;spec-&gt;path/-&gt;path/ and associated
management changes.  This implicitly fixes the bug because we removed
the shared parts between input and output of line_log_data_copy(); it
is now safe to overwrite the path in the copy.

There's one only somewhat related change: the comment in
process_all_files() explains the reasoning behind using 'range' there.
That bit of half-correct code had me sidetracked for a while.

Signed-off-by: Thomas Rast &lt;trast@inf.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>log -L: fix overlapping input ranges</title>
<updated>2013-04-05T17:39:09+00:00</updated>
<author>
<name>Thomas Rast</name>
<email>trast@inf.ethz.ch</email>
</author>
<published>2013-04-05T14:34:48+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=209618860c2627a4e134a15472587c574b328b40'/>
<id>209618860c2627a4e134a15472587c574b328b40</id>
<content type='text'>
The existing code was too defensive, and would trigger the assert in
range_set_append() if the user gave overlapping ranges.

The intent was always to define overlapping ranges as just the union
of all of them, as evidenced by the call to sort_and_merge_range_set().
(Which was already used, unlike what the comment said.)

Fix by splitting out the meat of range_set_append() to a new _unsafe()
function that lacks the paranoia.  sort_and_merge_range_set will fix
up the ranges, so we don't need the checks there.

Signed-off-by: Thomas Rast &lt;trast@inf.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
The existing code was too defensive, and would trigger the assert in
range_set_append() if the user gave overlapping ranges.

The intent was always to define overlapping ranges as just the union
of all of them, as evidenced by the call to sort_and_merge_range_set().
(Which was already used, unlike what the comment said.)

Fix by splitting out the meat of range_set_append() to a new _unsafe()
function that lacks the paranoia.  sort_and_merge_range_set will fix
up the ranges, so we don't need the checks there.

Signed-off-by: Thomas Rast &lt;trast@inf.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>log -L: check range set invariants when we look it up</title>
<updated>2013-04-05T17:39:08+00:00</updated>
<author>
<name>Thomas Rast</name>
<email>trast@inf.ethz.ch</email>
</author>
<published>2013-04-05T14:34:47+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=4596f190d384872305e502ebece2e38358ba463e'/>
<id>4596f190d384872305e502ebece2e38358ba463e</id>
<content type='text'>
lookup_line_range() is a good place to check that the range sets
satisfy the invariants: they have been computed and set in earlier
iterations, and we now start working with them.

Signed-off-by: Thomas Rast &lt;trast@inf.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
lookup_line_range() is a good place to check that the range sets
satisfy the invariants: they have been computed and set in earlier
iterations, and we now start working with them.

Signed-off-by: Thomas Rast &lt;trast@inf.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Speed up log -L... -M</title>
<updated>2013-03-28T17:30:28+00:00</updated>
<author>
<name>Thomas Rast</name>
<email>trast@student.ethz.ch</email>
</author>
<published>2013-03-28T16:47:34+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=39410bf0bf81cb3f76fb9bfa8308cd672bc1442c'/>
<id>39410bf0bf81cb3f76fb9bfa8308cd672bc1442c</id>
<content type='text'>
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 &lt;trast@student.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
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 &lt;trast@student.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>log -L: :pattern:file syntax to find by funcname</title>
<updated>2013-03-28T17:30:04+00:00</updated>
<author>
<name>Thomas Rast</name>
<email>trast@student.ethz.ch</email>
</author>
<published>2013-03-28T16:47:33+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/git.git/commit/?id=13b8f68c1f0b6d4ff5a7bed5a834e7e5e34a9fd0'/>
<id>13b8f68c1f0b6d4ff5a7bed5a834e7e5e34a9fd0</id>
<content type='text'>
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 &lt;trast@student.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
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 &lt;trast@student.ethz.ch&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
</pre>
</div>
</content>
</entry>
</feed>
