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 <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
t4211 attempts to test multiple git-log -L ranges where one range is a
superset of the other, and falsely succeeds because its "expected"
output is incorrect.
Overlapping -L ranges handed to git-log are coalesced by
line-log.c:sort_and_merge_range_set() into a set of non-overlapping,
disjoint ranges. When one range is a subset of another,
sort_and_merge_range_set() should coalesce both ranges to the superset
range, but instead the coalesced range often is incorrectly truncated to
the end of the subset range. For example, ranges 2-8 and 3-4 are
coalesced incorrectly to 2-4.
One can observe this incorrect behavior with git-log -L using the test
repository created by t4211. The superset/subset ranges t4211 employs
are 4-$ and 8-12 (where $ represents end-of-file). The coalesced range
should be 4-$. Manually invoking git-log with the same ranges the test
employs, we see:
% git log -L 4:a.c simple |
awk '/^commit [0-9a-f]{40}/ { print substr($2,1,7) }'
4659538
100b61a
39b6eb2
a6eb826
f04fb20
de4c48a
% git log -L 8,12:a.c simple | awk ...
f04fb20
de4c48a
% git log -L 4:a.c -L 8,12:a.c simple | awk ...
a6eb826
f04fb20
de4c48a
This last output is incorrect. 8-12 is a subset of 4-$, hence the output
of the coalesced range should be the same as the 4-$ output shown first.
In fact, the above incorrect output is the truncated bogus range 4-12:
% git log -L 4,12:a.c simple | awk ...
a6eb826
f04fb20
de4c48a
Fix the test to correctly fail in the presence of the
sort_and_merge_range_set() coalescing bug. Do so by changing the
"expected" output to the commits mentioned in the 4-$ output above.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <trast@inf.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 ->spec->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->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->spec->path = xstrdup(pair->one->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/->spec->path/->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 <trast@inf.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This tests a toy example of a history like
* Merge
| \
| * Modify foo
| |
* | Rename foo->bar
| /
* Create foo
Current log -L fails on this; we'll fix it in the next commit.
Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Embarrassingly, the -M test did not actually invoke -M, and thus not
really test the feature.
Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <trast@inf.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <trast@inf.ethz.ch>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
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>
Signed-off-by: Junio C Hamano <gitster@pobox.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>
Signed-off-by: Junio C Hamano <gitster@pobox.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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
An earlier workaround designed to help people who list logical
directories that will not match what getcwd(3) returns in the
GIT_CEILING_DIRECTORIES had an adverse effect when it is slow to
stat and readlink a directory component of an element listed on it.
* mh/maint-ceil-absolute:
Provide a mechanism to turn off symlink resolution in ceiling paths
In commit 9db31bdf (submodule: Add --force option for git submodule
update, 2011-04-01) we added the option to the implementation's usage
synopsis but forgot to add it to the synopsis in the command
documentation. Add the option to the synopsis in the same location it
is reported in usage and re-wrap the options to avoid long lines.
Signed-off-by: Brad King <brad.king@kitware.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* 'for-junio' of git://github.com/kusma/git:
wincred: improve compatibility with windows versions
wincred: accept CRLF on stdin to simplify console usage
This reverts commit 78457bc0cc.
commit 28c5d9e ("vcs-svn: drop string_pool") previously removed
the only call-site for strtok_r. So let's get rid of the compat
implementation as well.
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
On WinXP, the windows credential helper doesn't work at all (due to missing
Cred[Un]PackAuthenticationBuffer APIs). On Win7, the credential format used
by wincred is incompatible with native Windows tools (such as the control
panel applet or 'cmdkey.exe /generic'). These Windows tools only set the
TargetName, UserName and CredentialBlob members of the CREDENTIAL
structure (where CredentialBlob is the UTF-16-encoded password).
Remove the unnecessary packing / unpacking of the password, along with the
related API definitions, for compatibility with Windows XP.
Don't use CREDENTIAL_ATTRIBUTEs to identify credentials for compatibility
with Windows credential manager tools. Parse the protocol, username, host
and path fields from the credential's target name instead.
Credentials created with an old wincred version will have mangled or empty
passwords after this change.
Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
The windows credential helper currently only accepts LF on stdin, but bash
and cmd.exe both send CRLF. This prevents interactive use in the console.
Change the stdin parser to optionally accept CRLF.
Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
Further updates to the user manual.
* wk/user-manual:
user-manual: Flesh out uncommitted changes and submodule updates
user-manual: Use request-pull to generate "please pull" text
user-manual: Reorganize the reroll sections, adding 'git rebase -i'
A change made on v1.8.1.x maintenance track had a nasty regression
to break the build when autoconf is used.
* jn/less-reconfigure:
Makefile: avoid infinite loop on configure.ac change
"git check-ignore ." segfaulted, as a function it calls deep in its
callchain took a string in the <ptr, length> form but did not stop
when given an empty string.
* as/check-ignore:
name-hash: allow hashing an empty string
t0008: document test_expect_success_multi
An earlier change to config.mak.autogen broke a build driven by the
./configure script when --htmldir is not specified on the command
line of ./configure.
* ct/autoconf-htmldir:
Bugfix: undefined htmldir in config.mak.autogen
* bw/get-tz-offset-perl:
cvsimport: format commit timestamp ourselves without using strftime
perl/Git.pm: fix get_tz_offset to properly handle DST boundary cases
Move Git::SVN::get_tz to Git::get_tz_offset
"Advice" is a mass noun, not a count noun; it's not ordinarily
pluralized.
Signed-off-by: Greg Price <price@mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Commit 1b77d83cab 'setup_git_directory_gently_1(): resolve symlinks
in ceiling paths' changed the setup code to resolve symlinks in the
entries in GIT_CEILING_DIRECTORIES. Because those entries are
compared textually to the symlink-resolved current directory, an
entry in GIT_CEILING_DIRECTORIES that contained a symlink would have
no effect. It was known that this could cause performance problems
if the symlink resolution *itself* touched slow filesystems, but it
was thought that such use cases would be unlikely. The intention of
the earlier change was to deal with a case when the user has this:
GIT_CEILING_DIRECTORIES=/home/gitster
but in reality, /home/gitster is a symbolic link to somewhere else,
e.g. /net/machine/home4/gitster. A textual comparison between the
specified value /home/gitster and the location getcwd(3) returns
would not help us, but readlink("/home/gitster") would still be
fast.
After this change was released, Anders Kaseorg <andersk@mit.edu>
reported:
> [...] my computer has been acting so slow when I’m not connected to
> the network. I put various network filesystem paths in
> $GIT_CEILING_DIRECTORIES, such as
> /afs/athena.mit.edu/user/a/n/andersk (to avoid hitting its parents
> /afs/athena.mit.edu, /afs/athena.mit.edu/user/a, and
> /afs/athena.mit.edu/user/a/n which all live in different AFS
> volumes). Now when I’m not connected to the network, every
> invocation of Git, including the __git_ps1 in my shell prompt, waits
> for AFS to timeout.
To allow users to work around this problem, give them a mechanism to
turn off symlink resolution in GIT_CEILING_DIRECTORIES entries. All
the entries that follow an empty entry will not be checked for symbolic
links and used literally in comparison. E.g. with these:
GIT_CEILING_DIRECTORIES=:/foo/bar:/xyzzy or
GIT_CEILING_DIRECTORIES=/foo/bar::/xyzzy
we will not readlink("/xyzzy") because it comes after an empty entry.
With the former (but not with the latter), "/foo/bar" comes after an
empty entry, and we will not readlink it, either.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
