This matches 'hook failed' warnings.
We're also going to add hints to some of the hook load errors. Without this
change we'd have two pairs of parens for a single error message, which looks
really cluttered.
The new transaction context did not handle the case where an exception during
close should still call release. This cause pretxnclose hooks that failed to
cause the transaction to fail without aborting, thus requiring a hg recover.
I've added a test.
Sometimes a txnclose or changegroup hook wants to iterate through all
the changesets in transaction: in that situation usually the revset
`$HG_NODE:` is used to select the revisions. Unfortunately this revset
sometimes may contain too many changesets because we don't have the
write lock while the hook runs newer changes may be added to
repository in the meantime.
That's why there is a need for extra variable carrying the information about
the last change in the transaction.
is None (issue4983)
Some hooks, such as post-init and post-clone, do not get a repo parameter in
their environment. If there is no repo, there is no repo.currenttransaction();
attempting to retrieve it anyway was causing crashes. Now currenttransaction is
only retrieved and written if the repo is not None.
Changeset e7b51de6e8eb alters the 'HG_PENDING' mechanism to be "always" there.
This change is made under the assumption than we previously did it only when
"writepending() actually wrote something". This assumption was wrong,
'writepending()' informs of pending changes the first time something is written
and for all following calls. We back this change out to restore the former
behavior, which was already correct.
We need to call delayupdate again after writing to the changelog.
Otherwise the prechangegroup hook consumes the delayupdate subscription and
future hooks don't see the pending changes (see issue 4934 for more details).
Adds a test that triggers the prechangegroup hook before the pretxnchangegroup
hook and verifies that the output of pretxnchangegroup doesn't change.
Previously we would only include HG_PENDING in the hook args if the
transaction's writepending() actually wrote something. This is a bad criteria,
since it's possible that a previous call to writepending() wrote stuff and the
hooks want to still see that.
The solution is to always have hooks execute within the scope of the pending
changes by always putting HG_PENDING in the environment.
Before this patch, "hg commit" (process A) executes steps below:
1. get current branch heads via 'repo.branchheads()'
- cache 'repo.changelog'
2. invoke 'repo.commit()'
3. acquire wlock
- invalidate 'repo.dirstate'
4. access 'repo.dirstate'
- re-read '.hg/dirstate'
- check validity of parent revisions with 'repo.changelog'
5. invoke 'repo.commitctx()'
6. acquire store lock (slock)
- invalidate 'repo.changelog'
7. do committing
8. release slock
9. release wlock
10. check new branch head (via 'cmdutil.commitstatus()')
If acquisition of wlock at (3) above waits for another "hg commit"
(process B) or so running parallelly to release wlock, process A
causes creating orphan revision, because:
- '.hg/dirstate' refers the revision, which is newly added by
process B, as its parent
- but already cached 'repo.changelog' doesn't contain such revision
- therefore, validating parents of '.hg/dirstate' at (4) above
replaces such revision with 'nullid'
Then, process A creates "orphan" revision, of which parent is "null"
revision.
In addition to it, "created new head" may be shown at the end of
process A unintentionally, if store is updated parallelly, because
both getting branch heads (1) and checking new branch head (10) are
executed outside slock scope.
To avoid this issue, this patch makes "hg commit" acquire wlock and
slock before processing.
This patch resolves the issue between "hg commit" processes, but not
one between "hg commit" and other commands. Subsequent patches resolve
the latter.
Even after this patch, there are still corner case problems below:
- filecache may overlook changes of '.hg/dirstate', and it causes
similar issue (see below for detail)
https://bz.mercurial-scm.org/show_bug.cgi?id=4368#c10
- 3rd party extension may cause similar issue, if it directly uses
'repo.commit()' without acquisition of wlock and slock
This can be fixed by acquisition of slock at the beginning of
'repo.commit()', but it seems suitable for "default" branch
In fact, acquisition of slock itself is already introduced at
"default" branch by ec227b188932, but acquisition is not at the
beginning of 'repo.commit()'.
This patch also changes some tests:
- test-fncache.t needs this tricky wrapping, to release (= forced
failure of) wlock certainly
- order of "hg commit" output is changed by widening scope of locks,
because some hooks are fired after releasing wlock
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.
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.
The update command is touching the repository and should lock it for
the length of its operations. Equally importantly, it should lock the
repository when it is writing bookmarks. It wasn't doing so until now,
leaving doors open for all kinds of drunk beaver parties.
This results in some minor tests changes, and the fixing of a couple
of bugs from race conditions.
Code does not receive any changes beside extra indentation.
For some time, bookmark can and should be moved in the transaction. This
changeset migrates the 'hg bookmarks' commands to use a transaction.
Tests regarding rollback and transaction hooks are impacted for
obvious reasons. Some have to be slightly updated to keep testing the
same things. Some can just be dropped because they do not make sense
anymore.
When using bundle2, the bookmark's pushkey parts are now made mandatory. As a
result failure to update the bookmark server side will result in the transaction
being aborted.
We are already fetching remote bookmarks to honor the -B option, we
now pass that data to the pull process so it can reuse it. This
prevents a race condition between the initial looking and the actual
pulling of changesets and bookmarks. Tests are updated to handle this
fact.
Using bundle2 has an effect on which hooks are run when. We turn it on for
test-hooks early to reduce the noise of switching the default exchange to
bundle2.
Before this patch, hook argument `txnid` isn't passed to `pretxnopen`
hooks, even though `hooks` section of `hg help config` describes so.
``pretxnopen``
Run before any new repository transaction is open. The reason for the
transaction will be in ``$HG_TXNNAME`` and a unique identifier for the
transaction will be in ``HG_TXNID``. A non-zero status will prevent the
transaction from being opened.
There were 2 test failures in 3.4-rc when running test-hook.t with the
largefiles extension enabled. For context, the first is a commit hook:
@@ -618,9 +621,9 @@
$ echo 'update = hg id' >> .hg/hgrc
$ echo bb > a
$ hg ci -ma
- 223eafe2750c tip
+ d3354c4310ed+
$ hg up 0
- cb9a9f314b8b
+ 223eafe2750c+ tip
1 files updated, 0 files merged, 0 files removed, 0 files unresolved
make sure --verbose (and --quiet/--debug etc.) are propagated to the local ui
In both cases, largefiles acquires the wlock before calling into core, which
also acquires the wlock. The first case was fixed in 4100e338a886 by ensuring
the hook only runs after the lock has been fully released. The full release is
important, because that is what writes dirstate to the disk, allowing external
hooks to see the result of the update. This simply changes how the update hook
is called, so that it too is deferred until the lock is finally released.
There are many uses of mergemod.update(), but in terms of commands, it looks
like the following commands take wlock while calling mergemod.update(), and
therefore will now have their hook fired at a later time:
backout, fetch, histedit, qpush, rebase, shelve, transplant
Unlike the others, fetch immediately unlocks after calling update(), so for all
intents and purposes, its hook invocation is not deferred (but the external hook
still sees the proper state).
If 'wlock' is taken, we should add 'afterlock' callback to the 'wlock' instead.
Otherwise, running post transaction hook after 'lock' is release but 'wlock' is
still taken lead to a deadlock (eg: 'hg update' during a hook).
This situation is much more common since: b7067abc16c0
push: acquire local 'wlock' if "pushback" is expected (BC) (issue4596)
This hook will be called whenever a transaction is aborted. This will make it
easy for people to clean up temporary content they may have created during a
transaction.
We are adding a 'txnclose' hook that will be run right before a transaction is
closed. Hooks running at that time will have access to the full transaction
content through both 'hookargs' content and on-disk reading. They will be able
to abort the transaction.
We are adding generic hooking for all transactions. We may have useful
information about what happened during the transaction, user of the transaction
should have filled the 'hookargs' dictionnary of the transaction. This hook is
simple because it has no power to rollback the transaction.
We are adding generic hooking for all transactions. We do not really have any
useful information to include when opening the transaction but this is a
useful time to allow a hook anyway. We better let people abort transaction before
they happen than after multiple seconds/minutes of processing.
Once the transaction is closed, we now write transaction related data for
possible future undo. For now, we only do it for full file "backup" because
their were not handle at all in that case. In the future, we could move all the
current logic to set undo up (that currently exists in localrepository) inside
transaction itself, but it is not strictly requires to solve the current
situation.
commitctx already showed notes with filenames but didn't provide any context.
It is just as relevant to know when manifest or changelog is committed.
So, in addition to filenames, also show headlines 'committing files:',
'committing manifest' and 'committing changelog'.
This allow to gracefully report the failure of the bookmark push and carry on.
Before this change set. Local push would plain quit and wireprotocol would
failed in various ungraceful way.
addchangegroup creates a runhook function that is used to invoke the
changegroup and incoming hooks, but at the time the function is called,
the contents of hookargs associated with the transaction may have been
modified externally. For instance, bundle2 code affects it with
obsolescence markers and bookmarks info.
It also creates problems when a single transaction is used with multiple
changegroups added (as per an upcoming change), whereby the contents
of hookargs are that of after adding a latter changegroup when invoking
the hook for the first changegroup.
We do all the things in one go now, updating existing bookmark, adding new ones,
and overwriting the ones explicitly specified for --bookmark. This impacts the
tests by removing some duplicated or unnecessary output.
The discovery phases for bookmarks now use the list of explicitly pushed bookmarks
to do addition, removal and overwriting.
Tests are impacted because this reduces the amount of listkeys calls issued, removes
some duplicated messages and improves the accuracy of some messages.
There is no reason for bookmarks to get a special treatment. As a first step we
move the code as is in the `exchange.pull` function. Integration with the rest
of the flow will come later.
Adding bookmarks to pull means that most clone paths are now pulling bookmarks
through pull. We ensure that bookmark-update messages are properly suppressed in
that case.
In test-pull-http.t the 'requesting all changes' message disappear because we
now get the authentication error on the `listkeys`command before such message
is printed.
The discovery of necessary bookmark updates is now done within the "discovery
phase". This opens the door to the inclusion of bookmarks in a unified bundle2
push.
There is no reason to expose unnecessary Python implementation details and
memory locations, also not in debug mode.
readablefunc was already creating a nice name - we move that functionality
up and reuse it.
We consider having a __call__ and being types.FunctionType sufficiently
similar and unify these two to just using the existing check for __call__.
Consider a bookmark B that exists both locally and remotely. If B is updated
remotely, and then a pull is performed where the pull set contains the new
location of B, the bookmark is updated locally. However, if remote B is
updated in the middle of a pull to a location not in the pull set, the
bookmark won't be updated locally at all.
To fix this, list bookmarks before pulling in changesets, not after. This
still leaves a race open if B gets moved in between listing bookmarks and
pulling in changesets, but the race window is much smaller. Fixing the race
properly would require a bundle format upgrade.
test-hook.t's output changes because we no longer do two listkeys calls during
pull, just one.
test-pull-http.t's output changes because we now search for bookmarks before
searching for changes.
An upcoming patch will change the order of operations and perform a listkeys
before a changegroup fetch. This will cause a few tests to print out the wrong
error message.
