Commit graph

26 commits

Author SHA1 Message Date
Johannes Schindelin 5d6b151fdd xdl_merge(): fix a segmentation fault when refining conflicts
The function xdl_refine_conflicts() tries to break down huge
conflicts by doing a diff on the conflicting regions. However,
this does not make sense when one side is empty.

Worse, when one side is not only empty, but after EOF, the code
accessed unmapped memory.

Noticed by Luben Tuikov, Shawn Pearce and Alexandre Julliard, the
latter providing a test case.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-12-28 13:59:39 -08:00
Junio C Hamano 8042ed1ceb Merge branch 'master' into js/merge
* master: (42 commits)
  git-svn: correctly handle packed-refs in refs/remotes/
  add test case for recursive merge
  git-svn: correctly display fatal() error messages
  git-svn: allow dcommit to take an alternate head
  git-svn: enable logging of information not supported by git
  Clarify fetch error for missing objects.
  Move Fink and Ports check to after config file
  shortlog: fix segfault on empty authorname
  shortlog: remove "[PATCH]" prefix from shortlog output
  Make sure the empty tree exists when needed in merge-recursive.
  Don't use memcpy when source and dest. buffers may overlap
  no need to install manpages as executable
  Documentation: simpler shared repository creation
  shortlog: fix segfault on empty authorname
  Add branch.*.merge warning and documentation update
  Fix perl/ build.
  git-svn: use do_switch for --follow-parent if the SVN library supports it
  Fix documentation copy&paste typo
  git-svn: extra error check to ensure we open a file correctly
  Documentation: update git-clone man page with new behavior
  ...
2006-12-12 21:52:19 -08:00
Johannes Schindelin 98e6da8a36 xdl_merge(): fix and simplify conflict handling
Suppose you have changes in new1 to the original lines 10-20,
and changes in new2 to the original lines 15-25, then the
changes to 10-25 conflict. But it is possible that the next
changes in new1 still overlap with this change to new2.

So, in the next iteration we have to look at the same change
to new2 again.

The old code tried to be a bit too clever. The new code is
shorter and more to the point: do not fiddle with the ranges
at all.

Also, xdl_append_merge() tries harder to combine conflicts.
This is necessary, because with the above simplification,
some conflicts would not be recognized as conflicts otherwise:

In the above scenario, it is possible that there is no other
change to new1. Absent the combine logic, the change in new2
would be recorded _again_, but as a non-conflict.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
2006-12-05 13:30:22 -08:00
Johannes Schindelin c7c24889bb diff -b: ignore whitespace at end of line
This is _not_ the same as "treat eol as whitespace", since that would mean
that multiple empty lines would be treated as equal to e.g. a space.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-12-04 16:30:54 -08:00
Johannes Schindelin 710daa83fc xdl_merge(): fix thinko
If one side's block (of changed lines) ends later than the other
side's block, the former should be tested against the next block
of the other side, not vice versa.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-12-02 17:28:19 -08:00
Johannes Schindelin 875b8ce476 xdl_merge(): fix an off-by-one bug
The line range is i1 .. (i1 + chg1 - 1), not i1 .. (i1 + chg1).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-12-02 17:28:19 -08:00
Junio C Hamano 53a7085a10 xmerge: make return value from xdl_merge() more usable.
The callers would want to know if the resulting merge is clean;
do not discard that information away after calling xdl_do_merge().

Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-12-02 17:28:19 -08:00
Johannes Schindelin 857b933e04 xdiff: add xdl_merge()
This new function implements the functionality of RCS merge, but
in-memory. It returns < 0 on error, otherwise the number of conflicts.

Finding the conflicting lines can be a very expensive task. You can
control the eagerness of this algorithm:

- a level value of 0 means that all overlapping changes are treated
  as conflicts,
- a value of 1 means that if the overlapping changes are identical,
  it is not treated as a conflict.
- If you set level to 2, overlapping changes will be analyzed, so that
  almost identical changes will not result in huge conflicts. Rather,
  only the conflicting lines will be shown inside conflict markers.

With each increasing level, the algorithm gets slower, but more accurate.
Note that the code for level 2 depends on the simple definition of
mmfile_t specific to git, and therefore it will be harder to port that
to LibXDiff.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-12-02 17:28:19 -08:00
Andy Parkins f73da29fa2 Increase length of function name buffer
In xemit.c:xdl_emit_diff() a buffer for showing the function name as
commentary is allocated; this buffer was 40 characters.  This is a bit
small; particularly for C++ function names where there is often an
identical prefix (like void LongNamespace::LongClassName) on multiple
functions, which makes the context the same everywhere.  In other words
the context is useless.  This patch increases that buffer to 80
characters - which may still not be enough, but is better

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-11-23 22:51:41 -08:00
Petr Baudis 70da769a46 xdiff: Match GNU diff behaviour when deciding hunk comment worthiness of lines
This removes the '#' and '(' tests and adds a '$' test instead although I have
no idea what it is actually good for - but hey, if that's what GNU diff does...

Pasky only went and did as Junio sayeth.

Signed-off-by: Petr Baudis <pasky@suse.cz>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-10-25 12:57:33 -07:00
Jim Meyering 67aef03455 xdiff/xemit.c (xdl_find_func): Elide trailing white space in a context header.
This removes trailing blanks from git-generated diff headers
the same way a similar patch did that for GNU diff:

  http://article.gmane.org/gmane.comp.gnu.utils.bugs/13839

That is, it removes trailing blanks on the hunk header line that
shows the function name.

Signed-off-by: Jim Meyering <jim@meyering.net>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-10-23 14:33:54 -07:00
Junio C Hamano 3453f862e1 Merge branch 'maint'
* maint:
  Fix hash function in xdiff library
2006-10-16 21:58:54 -07:00
Linus Torvalds 9de0834663 Fix hash function in xdiff library
Jim Mayering noticed that xdiff library took insanely long time
when comparing files with many identical lines.

This was because the hash function used in the library is broken
on 64-bit architectures and caused too many collisions.

http://thread.gmane.org/gmane.comp.version-control.git/28962/focus=28994

Acked-by: Davide Libenzi <davidel@xmaliserver.org>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-10-16 21:27:44 -07:00
Johannes Schindelin 2344d47fba diff: fix 2 whitespace issues
When whitespace or whitespace change was ignored, the function
xdl_recmatch() returned memcmp() style differences, which is wrong,
since it should return 0 on non-match.

Also, there were three horrible off-by-one bugs, even leading to wrong
hashes in the whitespace special handling.

The issue was noticed by Ray Lehtiniemi.

For good measure, this commit adds a test.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-10-12 09:30:14 -07:00
Junio C Hamano 096b173234 Merge branch 'lt/merge-tree'
* lt/merge-tree:
  Improved three-way blob merging code
  Prepare "git-merge-tree" for future work
  xdiff: generate "anti-diffs" aka what is common to two files
2006-07-12 22:31:22 -07:00
Pavel Roskin 82e5a82fd7 Fix more typos, primarily in the code
The only visible change is that git-blame doesn't understand
"--compability" anymore, but it does accept "--compatibility" instead,
which is already documented.

Signed-off-by: Pavel Roskin <proski@gnu.org>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-07-10 00:36:44 -07:00
Linus Torvalds a9ed376b15 xdiff: generate "anti-diffs" aka what is common to two files
This fairly trivial patch adds a new XDL_EMIT_xxx flag to tell libxdiff
that we don't want to generate the _diff_ between two files, we want to
see the lines that are _common_ to two files.

So when you set XDL_EMIT_COMMON, xdl_diff() will do everything exactly
like it used to do, but the output records it generates just contain the
lines that aren't part of the diff.

This is for doing things like generating the common base case for a file
that was added in both branches.

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-06-28 22:24:32 -07:00
Johannes Schindelin 0d21efa51c Teach diff about -b and -w flags
This adds -b (--ignore-space-change) and -w (--ignore-all-space) flags to
diff. The main part of the patch is teaching libxdiff about it.

[jc: renamed xdl_line_match() to xdl_recmatch() since the former is used
 for different purposes in xpatchi.c which is in the parts of the upstream
 source we do not use.]

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-06-23 17:35:27 -07:00
Junio C Hamano d281786fcd xdiff: minor changes to match libxdiff-0.21
This reformats the change 621c53cc08
introduced to match what upstream author implemented in libxdiff-0.21
without changing any logic (hopefully ;-).  This is to help keep
us in sync with the upstream.

Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-06-19 18:43:49 -07:00
Davide Libenzi 295ba2fb89 xdiff: post-process hunks to make them consistent. 2006-04-13 16:48:45 -07:00
Marco Roeland 0ed49a3ed9 xdiff/xdiffi.c: fix warnings about possibly uninitialized variables
Compiling this module gave the following warnings (some double dutch!):

xdiff/xdiffi.c: In functie 'xdl_recs_cmp':
xdiff/xdiffi.c:298: let op: 'spl.i1' may be used uninitialized in this function
xdiff/xdiffi.c:298: let op: 'spl.i2' may be used uninitialized in this function
xdiff/xdiffi.c:219: let op: 'fbest1' may be used uninitialized in this function
xdiff/xdiffi.c:219: let op: 'bbest1' may be used uninitialized in this function

A superficial tracking of their usage, without deeper knowledge about the
algorithm, indeed confirms that there are code paths on which these
variables will be used uninitialized. In practice these code paths might never
be reached, but then these fixes will not change the algorithm. If these
code paths are ever reached we now at least have a predictable outcome. And
should the very small performance impact of these initializations be
noticeable, then they should at least be replaced by comments why certain
code paths will never be reached.

Some extra initializations in this patch now fix the warnings.
2006-04-08 23:35:22 -07:00
Davide Libenzi ca557afff9 Clean-up trivially redundant diff.
Also corrects the line numbers in unified output when using
zero lines context.
2006-04-04 00:11:09 -07:00
Mark Wooding acb7257729 xdiff: Show function names in hunk headers.
The speed of the built-in diff generator is nice; but the function names
shown by `diff -p' are /really/ nice.  And I hate having to choose.  So,
we hack xdiff to find the function names and print them.

xdiff has grown a flag to say whether to dig up the function names.  The
builtin_diff function passes this flag unconditionally.  I suppose it
could parse GIT_DIFF_OPTS, but it doesn't at the moment.  I've also
reintroduced the `function name' into the test suite, from which it was
removed in commit 3ce8f089.

The function names are parsed by a particularly stupid algorithm at the
moment: it just tries to find a line in the `old' file, from before the
start of the hunk, whose first character looks plausible.  Still, it's
most definitely a start.

Signed-off-by: Mark Wooding <mdw@distorted.org.uk>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-03-27 18:43:51 -08:00
Junio C Hamano 3ce8f08944 built-in diff: minimum tweaks
This fixes up a couple of minor issues with the real built-in
diff to be more usable:

 - Omit ---/+++ header unless we emit diff output;

 - Detect and punt binary diff like GNU does;

 - Honor GIT_DIFF_OPTS minimally (only -u<number> and
   --unified=<number> are currently supported);

 - Omit line count of 1 from "@@ -l,k +m,n @@" hunk header
   (i.e. when k == 1 or n == 1)

 - Adjust testsuite for the lack of -p support.

Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-03-25 16:50:00 -08:00
Linus Torvalds 621c53cc08 builtin-diff: \No newline at end of file.
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-03-25 16:49:59 -08:00
Linus Torvalds 3443546f6e Use a *real* built-in diff generator
This uses a simplified libxdiff setup to generate unified diffs _without_
doing  fork/execve of GNU "diff".

This has several huge advantages, for example:

Before:

	[torvalds@g5 linux]$ time git diff v2.6.16.. > /dev/null

	real    0m24.818s
	user    0m13.332s
	sys     0m8.664s

After:

	[torvalds@g5 linux]$ time git diff v2.6.16.. > /dev/null

	real    0m4.563s
	user    0m2.944s
	sys     0m1.580s

and the fact that this should be a lot more portable (ie we can ignore all
the issues with doing fork/execve under Windows).

Perhaps even more importantly, this allows us to do diffs without actually
ever writing out the git file contents to a temporary file (and without
any of the shell quoting issues on filenames etc etc).

NOTE! THIS PATCH DOES NOT DO THAT OPTIMIZATION YET! I was lazy, and the
current "diff-core" code actually will always write the temp-files,
because it used to be something that you simply had to do. So this current
one actually writes a temp-file like before, and then reads it into memory
again just to do the diff. Stupid.

But if this basic infrastructure is accepted, we can start switching over
diff-core to not write temp-files, which should speed things up even
further, especially when doing big tree-to-tree diffs.

Now, in the interest of full disclosure, I should also point out a few
downsides:

 - the libxdiff algorithm is different, and I bet GNU diff has gotten a
   lot more testing. And the thing is, generating a diff is not an exact
   science - you can get two different diffs (and you will), and they can
   both be perfectly valid. So it's not possible to "validate" the
   libxdiff output by just comparing it against GNU diff.

 - GNU diff does some nice eye-candy, like trying to figure out what the
   last function was, and adding that information to the "@@ .." line.
   libxdiff doesn't do that.

 - The libxdiff thing has some known deficiencies. In particular, it gets
   the "\No newline at end of file" case wrong. So this is currently for
   the experimental branch only. I hope Davide will help fix it.

That said, I think the huge performance advantage, and the fact that it
integrates better is definitely worth it. But it should go into a
development branch at least due to the missing newline issue.

Technical note: this is based on libxdiff-0.17, but I did some surgery to
get rid of the extraneous fat - stuff that git doesn't need, and seriously
cutting down on mmfile_t, which had much more capabilities than the diff
algorithm either needed or used. In this version, "mmfile_t" is just a
trivial <pointer,length> tuple.

That said, I tried to keep the differences to simple removals, so that you
can do a diff between this and the libxdiff origin, and you'll basically
see just things getting deleted. Even the mmfile_t simplifications are
left in a state where the diffs should be readable.

Apologies to Davide, whom I'd love to get feedback on this all from (I
wrote my own "fill_mmfile()" for the new simpler mmfile_t format: the old
complex format had a helper function for that, but I did my surgery with
the goal in mind that eventually we _should_ just do

	mmfile_t mf;

	buf = read_sha1_file(sha1, type, &size);
	mf->ptr = buf;
	mf->size = size;
	.. use "mf" directly ..

which was really a nightmare with the old "helpful" mmfile_t, and really
is that easy with the new cut-down interfaces).

[ Btw, as any hawk-eye can see from the diff, this was actually generated
  with itself, so it is "self-hosting". That's about all the testing it
  has gotten, along with the above kernel diff, which eye-balls correctly,
  but shows the newline issue when you double-check it with "git-apply" ]

Signed-off-by: Linus Torvalds <torvalds@osdl.org>
Signed-off-by: Junio C Hamano <junkio@cox.net>
2006-03-25 16:49:58 -08:00