mercurial-scm/hg: comparison mercurial/cmdutil.py

equal deleted inserted replaced

-:a46dfc2b58a3
+:28dfb2df4ab9
 from . import (
 bookmarks,
 changelog,
 copies,
 crecord as crecordmod,
-dirstateguard,
 encoding,
 error,
 formatter,
 logcmdutil,
 match as matchmod,
 )
 return err
+class _AddRemoveContext:
+"""a small (hacky) context to deal with lazy opening of context
+This is to be used in the `commit` function right below. This deals with
+lazily open a `changing_files` context inside a `transaction` that span the
+full commit operation.
+We need :
+- a `changing_files` context to wrap the dirstate change within the
+"addremove" operation,
+- a transaction to make sure these change are not written right after the
+addremove, but when the commit operation succeed.
+However it get complicated because:
+- opening a transaction "this early" shuffle hooks order, especially the
+`precommit` one happening after the `pretxtopen` one which I am not too
+enthusiastic about.
+- the `mq` extensions + the `record` extension stacks many layers of call
+to implement `qrefresh --interactive` and this result with `mq` calling a
+`strip` in the middle of this function. Which prevent the existence of
+transaction wrapping all of its function code. (however, `qrefresh` never
+call the `addremove` bits.
+- the largefile extensions (and maybe other extensions?) wraps `addremove`
+so slicing `addremove` in smaller bits is a complex endeavour.
+So I eventually took a this shortcut that open the transaction if we
+actually needs it, not disturbing much of the rest of the code.
+It will result in some hooks order change for `hg commit --addremove`,
+however it seems a corner case enough to ignore that for now (hopefully).
+Notes that None of the above problems seems insurmountable, however I have
+been fighting with this specific piece of code for a couple of day already
+and I need a solution to keep moving forward on the bigger work around
+`changing_files` context that is being introduced at the same time as this
+hack.
+Each problem seems to have a solution:
+- the hook order issue could be solved by refactoring the many-layer stack
+that currently composes a commit and calling them earlier,
+- the mq issue could be solved by refactoring `mq` so that the final strip
+is done after transaction closure. Be warned that the mq code is quite
+antic however.
+- large-file could be reworked in parallel of the `addremove` to be
+friendlier to this.
+However each of these tasks are too much a diversion right now. In addition
+they will be much easier to undertake when the `changing_files` dust has
+settled."""
+def __init__(self, repo):
+self._repo = repo
+self._transaction = None
+self._dirstate_context = None
+self._state = None
+def __enter__(self):
+assert self._state is None
+self._state = True
+return self
+def open_transaction(self):
+"""open a `transaction` and `changing_files` context
+Call this when you know that change to the dirstate will be needed and
+we need to open the transaction early
+This will also open the dirstate `changing_files` context, so you should
+call `close_dirstate_context` when the distate changes are done.
+"""
+assert self._state is not None
+if self._transaction is None:
+self._transaction = self._repo.transaction(b'commit')
+self._transaction.__enter__()
+if self._dirstate_context is None:
+self._dirstate_context = self._repo.dirstate.changing_files(
+self._repo
+)
+self._dirstate_context.__enter__()
+def close_dirstate_context(self):
+"""close the change_files if any
+Call this after the (potential) `open_transaction` call to close the
+(potential) changing_files context.
+"""
+if self._dirstate_context is not None:
+self._dirstate_context.__exit__(None, None, None)
+self._dirstate_context = None
+def __exit__(self, *args):
+if self._dirstate_context is not None:
+self._dirstate_context.__exit__(*args)
+if self._transaction is not None:
+self._transaction.__exit__(*args)
 def commit(ui, repo, commitfunc, pats, opts):
 '''commit the specified files or all outstanding changes'''
 date = opts.get(b'date')
 if date:
 opts[b'date'] = dateutil.parsedate(date)
-dsguard = None
+with repo.wlock(), repo.lock():
-# extract addremove carefully -- this function can be called from a command
-# that doesn't support addremove
-if opts.get(b'addremove'):
-dsguard = dirstateguard.dirstateguard(repo, b'commit')
-with dsguard or util.nullcontextmanager():
 message = logmessage(ui, opts)
 matcher = scmutil.match(repo[None], pats, opts)
-if True:
+with _AddRemoveContext(repo) as c:
 # extract addremove carefully -- this function can be called from a
 # command that doesn't support addremove
 if opts.get(b'addremove'):
 relative = scmutil.anypats(pats, opts)
 uipathfn = scmutil.getuipathfn(
 repo,
 matcher,
 b"",
 uipathfn,
 opts,
+open_tr=c.open_transaction,
 )
 m = _(b"failed to mark all new/missing files as added/removed")
 if r != 0:
 raise error.Abort(m)
+c.close_dirstate_context()
 return commitfunc(ui, repo, message, matcher, opts)
 def samefile(f, ctx1, ctx2):
 if f in ctx1.manifest():

changeset 50029	28dfb2df4ab9
parent 50028	a46dfc2b58a3
child 50038	6cdcab3ae3fa