We introduce a new record type, 'C', to indicate change/delete conflicts. This
is a separate record type because older versions of Mercurial will not be able
to handle these conflicts.
We aren't actually storing any change/delete conflicts yet -- that will come in
future patches.
This works around a bug in older Mercurial versions' handling of the v2 merge
state.
We also add a bunch of tests that make sure that
(1) we correctly abort when the merge state has an unsupported record type
(2) aborting the merge, rebase or histedit continues to work and clears out the
merge state.
This is too low-level to be the top-level documentation for mergestate. We're
restricting the top-level documentation to only be about what consumers of the
mergestate and anyone extending it need to care about.
With this patch, mergestate.clean() will no longer abort when it encounters an
unsupported merge type. However we hold off on testing it until backwards
compatibility is in place.
Eventually, we'll move the read call out of the constructor. This will:
- avoid unnecessary reads when we're going to nuke the merge state anyway
- avoid raising an exception if there's an unsupported merge record
'clean' seems like a good name for it because I wanted to avoid anything with
the word 'new' in it, and 'reset' is more an action performed on a merge state
than a way to get a new merge state.
Thanks to Martin von Zweigbergk for feedback about naming this.
We're going to catch this exception in 'hg summary' to print a better error
message.
This code is pretty untested, so there are no changes to test output. In
upcoming patches we're going to test the output more thoroughly.
We're going to treat these conflicts similarly to merge conflicts, and this
change to the way we store things in memory makes future code a lot simpler.
(1) These aren't currently read from anywhere, so emptying this out is
pointless.
(2) These *will* be read from later in upcoming patches, and not emptying
them out will be important then.
I actually wanted to reduce the amount of code around the call to
applyupdates(), so I tried moving these warnings a little earlier, and
I think it makes the output make a little more sense (see changes to
test cases).
This only makes a difference when a merge driver is active -- in that case we
don't want to try and merge all the files, just the ones still unresolved after
the merge driver's preprocess step is over.
This will be a chance for the merge driver to finish resolving or generating
any driver-resolved files.
As before, having a separate error state from 'unresolved' is too big a
refactoring for now, so we hack around it by setting unresolved to a positive
value when necessary.
We also need to update our internal state to whatever state driverpreprocess
leaves it in.
Adding an error state separate from the unresolved count is too big a
refactoring for now, so we hack around it by setting it to a positive value to
indicate an error state.
The exact semantics for what should happen (particularly with respect to error
handling) are still a bit hard to pin down, so I think it's better to
experiment with it as an extension for now. For now this stub will act as a
convenient point for extensions to hook on.
c67339617276 (while 3.4 code-freeze) made all 'update' hooks run after
releasing wlock for visibility of in-memory dirstate changes. But this
breaks paired invocation of 'preupdate' and 'update' hooks.
For example, 'hg backout --merge' for TARGET revision, which isn't
parent of CURRENT, consists of steps below:
1. update from CURRENT to TARGET
2. commit BACKOUT revision, which backs TARGET out
3. update from BACKOUT to CURRENT
4. merge TARGET into CURRENT
Then, we expects hooks to run in the order below:
- 'preupdate' on CURRENT for (1)
- 'update' on TARGET for (1)
- 'preupdate' on BACKOUT for (3)
- 'update' on CURRENT for (3)
- 'preupdate' on TARGET for (4)
- 'update' on CURRENT/TARGET for (4)
But hooks actually run in the order below:
- 'preupdate' on CURRENT for (1)
- 'preupdate' on BACKOUT for (3)
- 'preupdate' on TARGET for (4)
- 'update' on TARGET for (1), but actually on CURRENT/TARGET
- 'update' on CURRENT for (3), but actually on CURRENT/TARGET
- 'update' on CURRENT for (4), but actually on CURRENT/TARGET
Root cause of the issue focused by c67339617276 is that external
'update' hook process can't view in-memory changes (especially, of
dirstate), because they aren't written out until the end of
transaction (or wlock).
Now, hooks can be invoked just after updating, because previous
patches made in-memory changes visible to external process.
This patch may break backward compatibility from the point of view of
"scheduling hook execution", but should be reasonable because 'update'
hooks had been executed in this order before 3.4.
This patch tests "hg backout" and "hg unshelve", because the former
activates the transaction before 'update' hook invocation, but the
former doesn't.
Now, 'dirstate.write(tr)' delays writing in-memory changes out, if a
transaction is running.
This may cause treating this revision as "the first bad one" at
bisecting in some cases using external hook process inside transaction
scope, because some external hooks and editor process are still
invoked without HG_PENDING and pending changes aren't visible to them.
'dirstate.write()' callers below in localrepo.py explicitly use 'None'
as 'tr', because they can assume that no transaction is running:
- just before starting transaction
- at closing transaction, or
- at unlocking wlock
Now that all internal callers pre-compute and set a destination at a higher level
it feels like we can kill this API. This will allow us to simplify this
function. However I feel like this is a bit too central and critical to break
now. I'm adding a devel warning to let extension make catch this in the next
cycle.
File/directory case folding collisions cannot be represented on case folding
systems and have to fail.
To detect this and abort early, utilize that for file/directory collisions, a
sorted list of case folded manifest names will have the colliding directory
right after the file.
(This could perhaps be optimized, but this way of doing it also has
directory/directory case folding in mind ... which however not is handled yet.)
A driver-resolved file is a file that's handled specially by the driver. A
common use case for this state would be autogenerated files, the generation of
which should happen only after all source conflicts are resolved.
This is done with an uppercase letter because older versions of Mercurial will
not know how to treat such files at all.
A 'merge driver' is a coordinator for the overall merge process. It will be
able to control:
- tools for individual files, much like the merge-patterns configuration does
today
- tools that can work across groups of files
- the ordering of file resolution
- resolution of automatically generated files
- adding and removing additional files to and from the dirstate
Since it is a critical part of the merge process, it really is part of the
merge state.
This is a lowercase character (i.e. optional) because ignoring this is fine for
older versions of Mercurial -- however, if there are any files that are
specially treated by the driver, we should abort. That will happen in upcoming
patches.
There is a potential security issue with storing the merge driver in the merge
state. See the inline comments for more details.
For the same reason, we move the bookmark related update logic into the
'destupdate' function. This requires to extend the returns of the function to
include the bookmark that needs to move (more or less) and the bookmark to
activate at the end of the function. See function documentation for details on
this returns.
We perform all that we can non-interactively before prompting the user for input
via their merge tool. This allows for a maximally consistent state when the user
is first prompted.
The test output changes indicate the actual behavior change happening.
The section of code that writes out the version of the file cached in the merge
state should only be run at preresolve time. This is so that if the premerge
keeps around conflict markers, those don't get overwritten before the main
merge.
This means that in ms.resolve we must call merge after calling premerge. This
doesn't yet mean that all premerges happen before any merges -- however, this
does get us closer to our goal.
The output differences are because we recompute the merge tool. The only
user-visible difference caused by this patch is that if the tool is missing
we'll print the warning twice. Not a huge deal, though.
The home of 'Abort' is 'error' not 'util' however, a lot of code seems to be
confused about that and gives all the credit to 'util' instead of the
hardworking 'error'. In a spirit of equity, we break the cycle of injustice and
give back to 'error' the respect it deserves. And screw that 'util' poser.
For great justice.
We can safely drop this because the very same assignment is enforcement later in
the function. Dropping it will make it simpler to extract the default
destination logic in its own function.
This is another step toward having "default" destination more clear and unified.
Not all the logic is there because some bookmark related computation happened
elsewhere. It will be moved later.
The function is private because as for the other ones, cleanup is needed before
we can proceed.
Resolving other conflicts before merge ones is better because the state before
the merge is as consistent as possible. It will also help with future work
involving automatic resolution of merge conflicts with an external merge
driver.
There are no ordering issues here because it is easy to verify that the same
file is never in both the dg/dm and the m sets.