27af01d (xdiff/xprepare: improve O(n*m) performance in
xdl_cleanup_records(), 2011-08-17) was supposed to be a performance
boost only. However, it unexpectedly changed the behaviour of diff.
Revert a part of 27af01d that removes logic that mark lines as
"multi-match" (ie. dis[i] == 2). This was preventing the multi-match
discard heuristic (performed in xdl_cleanup_records() and
xdl_clean_mmatch()) from executing.
Reported-by: Alexander Pepper <pepper@inf.fu-berlin.de>
Signed-off-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In xdl_cleanup_records(), we see O(n*m) performance, where n is the
number of records from xdf->dstart to xdf->dend, and m is the size of a
bucket in xdf->rhash (<= by mlim).
Here, we improve this to O(n) by pre-computing nm (in rcrec->len(1|2))
in xdl_classify_record().
Reported-by: Marat Radchenko <marat@slonopotamus.org>
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The ctype functions isspace(), isalnum(), et al take an integer
argument representing an unsigned character, or -1 for EOF. On
platforms with a signed char, it is unsafe to pass a char to them
without casting it to unsigned char first.
Most of git is already shielded against this by the ctype
implementation in git-compat-util.h, but xdiff, which uses libc
ctype.h, ought to be fixed.
Noticed-by: der Mouse <mouse@Rodents-Montreal.ORG>
Reported-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
For each hunk, xdl_find_func searches the preimage for a function name
until the beginning of the file. If the file does not contain any
function names, this search has complexity O(n^2) in the number of
hunks n.
Instead, inline xdl_find_func() and keep track of up to which line we
have scanned already and the contents of the last funcname line that
we have found.
Noticed and a different approach proposed by Clemens Buchacher.
This alternative solution was done by René Scharfe.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In xdl_recmatch, do the memcmp to check if the two lines are equal before
checking if whitespace flags are set. If the lines are identical, then
there is no need to check if they differ only in whitespace.
This makes the common case (there is no whitespace difference) faster.
It costs the case where lines are the same length and contain
whitespace differences, but the common case is more than 20% faster.
Signed-off-by: Dylan Reid <dgreid@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
memset() is heavily optimized, and resulting assembler code
is about 150 lines less for that file.
Signed-off-by: Alexey Mahotkin <squadette@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The labels for the three participants in a potential conflict are all
optional arguments for the xdiff merge routine; if they are NULL, then
xdl_merge() can cope by omitting the labels from its output. Move
them to the xmparam structure to allow new callers to save some
keystrokes where they are not needed.
This also has the virtue of making the xdiff merge interface more
similar to merge_trees, which might make it easier to learn.
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The ‘git checkout --conflict=diff3’ command can be used to
present conflicts hunks including text from the common ancestor:
<<<<<<< ours
ourside
|||||||
original
=======
theirside
>>>>>>> theirs
The added information is helpful for resolving merges by hand, and
merge tools can usually grok it because it is very similar to the
output from diff3 -m.
A subtle change can help more tools to understand the output. ‘diff3’
includes the name of the merge base on the ||||||| line of the output,
and some tools misparse the conflict hunks without it. Add a new
xmp->ancestor parameter to xdl_merge() for use with conflict style
XDL_MERGE_DIFF3 as a label on the ||||||| line for any conflict hunks.
If xmp->ancestor is NULL, the output format is unchanged. Thus, this
change only provides unexposed plumbing for the new feature; it does
not affect the outward behavior of git.
Requested-by: Stefan Monnier <monnier@iro.umontreal.ca>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Acked-by: Bert Wesarg <Bert.Wesarg@googlemail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Include the merge level, favor, and style flags into the xmparam_t struct.
This removes the bit twiddling with these three values into the one flags
parameter.
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The current union merge driver is implemented as an post process. But the
xdl_merge code is quite capable to produce the result by itself. Therefore
move it there.
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* jc/conflict-marker-size:
rerere: honor conflict-marker-size attribute
rerere: prepare for customizable conflict marker length
conflict-marker-size: new attribute
rerere: use ll_merge() instead of using xdl_merge()
merge-tree: use ll_merge() not xdl_merge()
xdl_merge(): allow passing down marker_size in xmparam_t
xdl_merge(): introduce xmparam_t for merge specific parameters
git_attr(): fix function signature
Conflicts:
builtin-merge-file.c
ll-merge.c
xdiff/xdiff.h
xdiff/xmerge.c
This allows the callers of xdl_merge() to pass marker_size (defaults to 7)
in xmparam_t argument, to use conflict markers of non-default length.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
So far we have only needed to be able to pass an option that is generic to
xdiff family of functions to this function. Extend the interface so that
we can give it merge specific parameters.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sometimes people want their conflicting merges autoresolved by
favouring upstream changes. The standard answer they are given is
to run "git diff --name-only | xargs git checkout MERGE_HEAD --" in
such a case. This is to accept automerge results for the paths that
are fully resolved automatically, while taking their version of the
file in full for paths that have conflicts.
This is problematic on two counts.
One is that this is not exactly what these people want. It discards
all changes they did on their branch for any paths that conflicted.
They usually want to salvage as much automerge result as possible in
a conflicted file, and want to take the upstream change only in the
conflicted part.
This patch teaches two new modes of operation to the lowest-lever
merge machinery, xdl_merge(). Instead of leaving the conflicted
lines from both sides enclosed in <<<, ===, and >>> markers, the
conflicts are resolved favouring our side or their side of changes.
A larger problem is that this tends to encourage a bad workflow by
allowing people to record such a mixed up half-merged result as a
full commit without auditing. This commit does not tackle this
issue at all. In git, we usually give long enough rope to users
with strange wishes as long as the risky features are not enabled by
default, and this is such a risky feature.
Signed-off-by: Avery Pennarun <apenwarr@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* tf/diff-whitespace-incomplete-line:
xutils: Fix xdl_recmatch() on incomplete lines
xutils: Fix hashing an incomplete line with whitespaces at the end
Thell Fowler noticed that various "ignore whitespace" options to git diff
do not work well on an incomplete line.
The loop control of the function responsible for these bugs was extremely
difficult to follow. This patch restructures the loops for three variants
of "ignore whitespace" logic.
The basic idea of the re-written logic is:
- A loop runs while the characters from both strings we are looking at
match. We declare unmatch immediately when we find something that does
not match and return false from the function. We break out of the loop
if we ran out of either side of the string.
The way we skip spaces inside this loop varies depending on the style
of ignoring whitespaces.
- After the above loop breaks, we know that the parts of the strings we
inspected so far match, ignoring the whitespaces. The lines can match
only if the remainder consists of nothing but whitespaces. This part
of the logic is shared across all three styles.
The new code is more obvious and should be much easier to follow.
Tested-by: Thell Fowler <git@tbfowler.name>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Upon seeing a whitespace, xdl_hash_record_with_whitespace() first skipped
the run of whitespaces (excluding LF) that begins there, ensuring that the
pointer points at the last whitespace character in the run, and assumed
that the next character must be LF at the end of the line. This does not
work when hashing an incomplete line, which lacks the LF at the end.
Introduce "at_eol" variable that is true when either we are at the end of
line (looking at LF) or at the end of an incomplete line, and use that
instead throughout the code.
Noticed by Thell Fowler.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
* cb/maint-1.6.0-xdl-merge-fix:
Change xdl_merge to generate output even for null merges
t6023: merge-file fails to output anything for a degenerate merge
Conflicts:
xdiff/xmerge.c
xdl_merge used to have a check to ensure that there was at least
some change in one or other side being merged but this suppressed
output for the degenerate case when base, local and remote
contents were all identical.
Removing this check enables correct output in the degenerate case
and xdl_free_script handles freeing NULL scripts so there is no
need to have the check for these calls.
Signed-off-by: Charles Bailey <charles@hashpling.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
http-push.c::finish_request():
request is initialized by the for loop
index-pack.c::free_base_data():
b is initialized by the for loop
merge-recursive.c::process_renames():
move compare to narrower scope, and remove unused assignments to it
remove unused variable renames2
xdiff/xdiffi.c::xdl_recs_cmp():
remove unused variable ec
xdiff/xemit.c::xdl_emit_diff():
xche is always overwritten
Signed-off-by: Benjamin Kramer <benny.kra@googlemail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The code used to misbehave when options to ignore certain whitespaces
(-w -b and --ignore-at-eol) were combined.
Signed-off-by: Keith Cascio <keith@cs.ucla.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The patience diff algorithm produces slightly more intuitive output
than the classic Myers algorithm, as it does not try to minimize the
number of +/- lines first, but tries to preserve the lines that are
unique.
To this end, it first determines lines that are unique in both files,
then the maximal sequence which preserves the order (relative to both
files) is extracted.
Starting from this initial set of common lines, the rest of the lines
is handled recursively, with Myers' algorithm as a fallback when
the patience algorithm fails (due to no common unique lines).
This patch includes memory leak fixes by Pierre Habouzit.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Merge two hunks if there is only the specified number of otherwise unshown
context between them. For --inter-hunk-context=1, the resulting patch has
the same number of lines but shows uninterrupted context instead of a
context header line in between.
Patches generated with this option are easier to read but are also more
likely to conflict if the file to be patched contains other changes.
This patch keeps the default for this option at 0. It is intended to just
make the feature available in order to see its advantages and downsides.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a corner case of large files whose lines do not match uniquely, the
loop to eliminate a line that matches multiple locations adjacent to a run
of lines that do not uniquely match wasted too much cycles. Fix this by
giving up early after scanning 100 lines in both direction.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
In a corner case of large files whose lines do not match uniquely, the
loop to eliminate a line that matches multiple locations adjacent to a run
of lines that do not uniquely match wasted too much cycles. Fix this by
giving up early after scanning 100 lines in both direction.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
For some users (e.g. git blame), getting textual patch output is just
extra work, as they can get all the information they need from the low-
level diff structures. Allow for an alternate low-level emit function
to be defined to allow bypassing the textual patch generation; set
xemitconf_t's emit_func member to enable this.
The (void (*)()) type is pretty ugly, but the alternative would be to
include most of the private xdiff headers in xdiff.h to get the types
required for the "proper" function prototype. Also, a (void *) won't
work, as ANSI C doesn't allow a function pointer to be cast to an
object pointer.
Signed-off-by: Brian Downing <bdowning@lavos.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When showing a conflicting merge result, and "--diff3 -m" style is asked
for, this patch makes sure that the merge reduction level does not exceed
XDL_MERGE_EAGER. This is because "diff3 -m" style output would not make
sense for anything more aggressive than XDL_MERGE_EAGER, because of the
way how the merge reduction works.
"git merge-file" no longer has to force MERGE_EAGER when "--diff3" is
asked for because of this change.
Suppose a common ancestor (shared preimage) is modified to postimage #1
and #2 (each letter represents one line):
#####
postimage#1: 1234ABCDE789
| /
| /
preimage: 123456789
| \
postimage#2: 1234AXYE789
####
XDL_MERGE_MINIMAL and XDL_MERGE_EAGER would:
(1) find the s/56/ABCDE/ done on one side and s/56/AXYE/ done on the
other side,
(2) notice that they touch an overlapping area, and
(3) mark it as a conflict, "ABCDE vs AXYE".
The difference between the two algorithms is that EAGER drops the hunk
altogether if the postimages match (i.e. both sides modified the same
way), while MINIMAL keeps it. There is no other operation performed to
the hunk. As the result, lines marked with "#" in the above picure will
be in the RCS merge style output like this (letters <, = and > represent
conflict marker lines):
output: 1234<ABCDE=AXYE>789 ; with MINIMAL/EAGER
The part from the preimage that corresponds to these conflicting changes
is "56", which is what "diff3 -m" style output adds to it:
output: 1234<ABCDE|56=AXYE>789 ; in "diff3 -m" style
Now, XDL_MERGE_ZEALOUS looks at the differences between the changes two
postimages made in order to reduce the number of lines in the conflicting
regions. It notices that both sides start their new contents with "A",
and excludes it from the output (it also excludes "E" for the same
reason). The conflict that used to be "ABCDE vs AXYE" is now "BCD vs XY":
output: 1234A<BCD=XY>E789 ; with ZEALOUS
There could even be matching parts between two postimages in the middle.
Instead of one side rewriting the shared "56" to "ABCDE" and the other
side to "AXYE", imagine the case where the postimages are "ABCDE" and
"AXCYE", in which case instead of having one conflicted hunk "BCD vs XY",
you would have two conflicting hunks "B vs X" and "D vs Y".
In either case, once you reduce "ABCDE vs AXYE" to "BCD vs XY" (or "ABCDE
vs AXCYE" to "B vs X" and "D vs Y"), there is no part from the preimage
that corresponds to the conflicting change made in both postimages
anymore. In other words, conflict reduced by ZEALOUS algorithm cannot be
expressed in "diff3 -m" style. Representing the last illustration like
this is misleading to say the least:
output: 1234A<BCD|56=XY>E789 ; broken "diff3 -m" style
because the preimage was not ...4A56E... to begin with. "A" and "E" are
common only between the postimages.
Even worse, once a single conflicting hunk is split into multiple ones
(recall the example of breaking "ABCDE vs AXCYE" to "B vs X" and "D vs
Y"), there is no sane way to distribute the preimage text across split
conflicting hunks.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This replaces hardcoded magic constants with symbolic ones for
readability, and swaps one if/else blocks to better match the
order in which 0/1/2 variables are handled to nearby codepath.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When showing conflicting merges, we traditionally followed RCS's merge
output format. The output shows:
<<<<<<<
postimage from one side;
=======
postimage of the other side; and
>>>>>>>
Some poeple find it easier to be able to understand what is going on when
they can view the common ancestor's version, which is used by "diff3 -m",
which shows:
<<<<<<<
postimage from one side;
|||||||
shared preimage;
=======
postimage of the other side; and
>>>>>>>
This is an initial step to bring that as an optional feature to git.
Only "git merge-file" has been converted, with "--diff3" option.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This simply moves code around to make a separate function that prepares
a single conflicted hunk with markers into the buffer.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a merge conflicts, there are often common lines that are not really
common, such as empty lines or lines containing a single curly bracket.
With XDL_MERGE_ZEALOUS_ALNUM, we use the following heuristics: when a
hunk does not contain any letters or digits, it is treated as conflicting.
In other words, a conflict which used to look like this:
<<<<<<<
a = 1;
=======
output();
>>>>>>>
}
}
}
<<<<<<<
output();
=======
b = 1;
>>>>>>>
will look like this with ZEALOUS_ALNUM:
<<<<<<<
a = 1;
}
}
}
output();
=======
output();
}
}
}
b = 1;
>>>>>>>
To demonstrate this, git-merge-file has been switched from
XDL_MERGE_ZEALOUS to XDL_MERGE_ZEALOUS_ALNUM.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a merge conflicts, there are often less than three common lines
between two conflicting regions.
Since a conflict takes up as many lines as are conflicting, plus three
lines for the commit markers, the output will be shorter (and thus,
simpler) in this case, if the common lines will be merged into the
conflicting regions.
This patch merges up to three common lines into the conflicts.
For example, what looked like this before this patch:
<<<<<<<
if (a == 1)
=======
if (a != 0)
>>>>>>>
{
int i;
<<<<<<<
a = 0;
=======
a = !a;
>>>>>>>
will now look like this:
<<<<<<<
if (a == 1)
{
int i;
a = 0;
=======
if (a != 0)
{
int i;
a = !a;
>>>>>>>
Suggested Linus (based on ideas by "Voltage Spike" -- if that name is
real, it is mighty cool).
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Solaris Workshop Compiler found a few unreachable statements.
Signed-off-by: Guido Ostkamp <git@ostkamp.fastmail.fm>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This makes"diff -p" hunk headers customizable via gitattributes mechanism.
It is based on Johannes's earlier patch that allowed to define a single
regexp to be used for everything.
The mechanism to arrive at the regexp that is used to define hunk header
is the same as other use of gitattributes. You assign an attribute, funcname
(because "diff -p" typically uses the name of the function the patch is about
as the hunk header), a simple string value. This can be one of the names of
built-in pattern (currently, "java" is defined) or a custom pattern name, to
be looked up from the configuration file.
(in .gitattributes)
*.java funcname=java
*.perl funcname=perl
(in .git/config)
[funcname]
java = ... # ugly and complicated regexp to override the built-in one.
perl = ... # another ugly and complicated regexp to define a new one.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This uses "git-apply --whitespace=strip" to fix whitespace errors that have
crept in to our source files over time. There are a few files that need
to have trailing whitespaces (most notably, test vectors). The results
still passes the test, and build result in Documentation/ area is unchanged.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since in at least one use case, xdl_hash_record() takes over 15% of the
CPU time, it makes sense to even micro-optimize it. For many cases, no
whitespace special handling is needed, and in these cases we should not
even bother to check for whitespace in _every_ iteration of the loop.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
`git diff --ignore-space-at-eol` will ignore whitespace at the
line ends.
Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
In very obscure cases, a merge can hit an unexpected code path (where the
original code went as far as saying that this was a bug). This failing
merge was noticed by Alexandre Juillard.
The problem is that the original file contains something like this:
-- snip --
two non-empty lines
before two empty lines
after two empty lines
-- snap --
and this snippet is reduced to _one_ empty line in _both_ new files.
However, it is ambiguous as to which hunk takes the empty line: the first
or the second one?
Indeed in Alexandre's example files, the xdiff algorithm attributes the
empty line to the first hunk in one case, and to the second hunk in the
other case.
(Trimming down the example files _changes_ that behaviour!)
Thus, the call to xdl_merge_cmp_lines() has no chance to realize that the
change is actually identical in both new files. Therefore,
xdl_refine_conflicts() finds an empty diff script, which was not expected
there, because (the original author of xdl_merge() thought)
xdl_merge_cmp_lines() would catch that case earlier.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <junkio@cox.net>
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>
* 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
...
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>
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>