merge.calculateupdates() now filters the update actions through sparse
by default.
The filtering no-ops if sparse isn't enabled or no sparse config
is defined.
The function has been refactored to behave more like a filter
instead of a wrapper of merge.calculateupdates().
We should arguably take sparse into account earlier in
merge.calculateupdates(). This patch preserves the old behavior
of applying sparse at the end of update calculation, which is the
simplest and safest approach.
This was our last method on the custom repo type, meaning we could
remove that custom type and inline the 2 lines of code into
reposetup().
As part of the move, instead of wrapping merge.update() from
the sparse extension, we inline the function call. The ported
function now no-ops if sparse isn't enabled, making it safe to
always call.
The call site in update() may not be the most appropriate. But
it matches the previous behavior, which is the safest thing
to do. It can be improved later.
This simplifies the method slightly. It does create a full list of
paths while doing so, but it's not a lot of data anyway (besides, I
would think references to strings are no larger than (references to?)
True).
mergestate.unresolved() is a generator, so it seems better for it to
rely on iteritems() than items(), although it also seems unlikely for
it to make a noticeable difference.
Switch the lone call in merge.py to use it.
As with past refactors, the goal is to make wctx hot-swappable with an
in-memory context in the future. This change should be a no-op today.
This is necessary because some callers in merge.py pass backgroundclose=True
when writing.
As with previous changes in this series, this should be a no-op.
We would like to migrate direct calls of repo.wvfs/wwrite/wread/etc to a
call on the relevant workingfilectx, both as a cleanup (to reduce the number of
working copy functions on `repo`), and also to facilitate an in-memory merge
that doesn't write to the working copy.
In order to do that, the first step is to ensure we pass the target wctx around
and perform our writes and reads on it. Later, this object might become a
memctx.
In Python 2:
>>> ls = [4, 2, None]
>>> sorted(ls)
[None, 2, 4]
In Python 3:
>>> ls = [4, 2, None]
>>> sorted(ls)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: unorderable types: NoneType() < int()
Therefore we replaced None with -1, and the safe part is that, None and -1 are
only the keys which are used for sorting so we don't need to convert the -1's
back to None.
The old merge code would call manifestmerge and calculate the complete diff
between the source to the destination. In many cases, like rebase, the vast
majority of differences between the source and destination are irrelevant
because they are differences between the destination and the common ancestor
only, and therefore don't affect the merge. Since most actions are 'keep', all
the effort to compute them is wasted.
Instead, let's compute the difference between the source and the common ancestor
and only perform the diff of those files against the merge destination. When
using treemanifest, this lets us avoid loading almost the entire tree when
rebasing from a very old ancestor. This speeds up rebase of an old stack of 27
commits by 20x.
In mozilla-central, without treemanifest, when rebasing a commit from
default~100000 to default, this speeds up the manifestmerge step from 2.6s to
1.2s. However, the additional diff adds an overhead to all manifestmerge calls,
especially for flat manifests. When rebasing a commit from default~1 to default
it appears to add 100ms in mozilla-central. While we could put this optimization
behind a flag, I think the fact that it makes merge O(number of changes being
applied) instead of O(number of changes between X and Y) justifies it.
As part of changing manifest.diff to accept a matcher, a previous patch added
matcher calls to each location in merge.manifestmerge that tested if 'x in mf'
to maintain the same behavior as before. After analyzing it further, this
matcher call isn't needed, and in fact hurts future patches ability to use the
matcher here.
Basically, all these 'if x in mf' checks were checking if a matched file's copy
source was in the matcher as well. This meant if you passed a matcher for just
file foo, it would not return file bar even if foo was a copy of bar. Since
manifestmerge cares about copy information, let's allow all lookups of copy
sources.
We also update one spot with a 'is not None' check, since it wasn't obvious that
the value could sometimes be None before, which broke when we called
matcher(None).
A future patch adds matcher optimizations to manifestmerge which causes this
code path to get covered by existing tests.
With experimental.updatecheck=noconflict set, if one checks out
e1870a31325e (tests: add execute bit and fix shbang line, 2015-12-22)
and then try to check out its parent, hg will complain about
conflicting changes, even though the working directory is clean. We
need to also allow the 'e' action in merge.py. The 'e' action is used
when moving to a commit where the only change to the file is to its
executable flag, so it's just an optimized 'g' action.
Doesn't seem to be worth writing a test for, since the existing setup
in test-update-branches.t does not set any flags.
The working directory will usually be clean or very clean, and wc will usually
have the same branch as its parent. This change will thus usually not make any
difference and is done as a separate change to show that. It will be used in a
later change.
This gets rid of the manifest.matches calls in merge.py in favor of the new api.
This is part of getting rid of manifest.matches since it is O(manifest).
With experimental.updatecheck=noconflict, if the update is aborted
because of conlicts, "uncommitted changes" is not quite
accurate. Let's use "conflicting changes" instead. Also fix the hint
to recomment --clean, not --merge, since that's what we do for other
failed updates.
Since 5a430b57b16e (update: change default destination to tipmost
descendant (issue4673) (BC), 2016-02-02), non-linear updates can no
longer happen if the user doesn't specify a destination, so we can
drop these case from the table in the docstring of merge.update().
The check is done in merge.update() already and the next few patches
will add more checks there. Some of the additional checks will need
information about the merge that will not be available in destutil.
Since commands.postincoming() catches UpdateAbort(), we need to change
merge.update() to raise that more specific exception.
This goes directly again c6bcc960108f (destupdate: move the check
related to the "clean" logic in the function, 2015-10-05), but it will
simplify the next few patches, and we can always move it out again
(preferably move, not copy) after if we still think it's better that
way.
The comment about "obsolete.background" seems to have been about
obsolete.foreground ever since it was introduced in fbdfa64ceced
(update: allow dirty update to foreground (successors), 2013-04-16),
so let's change it.
As far as I can tell, all the callers of merge.update() have been
migrated over to use destutil to find the default destination when the
revision was not specified. So it's time to delete the code for
handling a node value of None. Let's add an assertion that node is not
None, so any extensions relying on it will not silently misbehave.
The merge code works by applying the changes between an ancestor and
the working copy onto the destination. To achieve an update, it sets
the ancestor to be the parent of the working copy. To achieve a clean
update (update -C), it sets the ancestor to be the working copy itself
(so there are no changes to carry over). The logic for this was spread
out a bit. Let's move it all to one place so it's easier to follow the
reason for it. Also add some documentation.
0b5f1f2efc77 introduced handling of a crash in this case. A review comment
suggested that it was not entirely obvious that a 'dm' always would have a 'r'
for the source file.
To mitigate that risk, make the code more conservative and make less
assumptions.
Work around that 'dm' in the data model only can have one operation for the
target file, but still can have multiple and conflicting operations on the
source file where the other operation is a 'rm'. The move would thus fail with
'abort: No such file or directory'.
In this case it is "obvious" that the file should be removed, either before or
after moving it. We thus keep the 'rm' of the source file but drop the 'dm'.
This is not a pretty fix but quite "obviously" safe (famous last words...) as
it only touches a rare code path that used to crash. It is possible that it
would be better to swap the files for 'dm' as suggested on
https://bz.mercurial-scm.org/show_bug.cgi?id=5020#c13 but it is not entirely
obvious that it not just would create conflicts on the other file. That can be
revisited later.
Previously we indicated that the .hgsubstate file was dirty by adding a '+' to
the end of its hash in the wctx manifest. This made is complicated to have new
manifest implementations that rely on the node length being fixed.
In previous patches we added added and modified node placeholders, so let's use
those to indicate dirty here as well. It doesn't look like anything ever
depended on this '+' (aside from it being different to the parent), so nothing
else needed to change here.
