[PATCH 1 of 4] pull: extract transaction logic into separate object

[Augie Fackler]

On Tue, Nov 25, 2014 at 10:26:26AM -0800, Eric Sumner wrote:
> # HG changeset patch
> # User Eric Sumner <ericsumner at fb.com>
> # Date 1416609177 28800
> #      Fri Nov 21 14:32:57 2014 -0800
> # Node ID 0b5ec0d98ac188df2afa03457e3f551e28e1eb28
> # Parent  a81c76106d9036060b3371f36b00ceefa5d60898
> pull: extract transaction logic into separate object
>
> This patch series is intended to allow bundle2 push reply part handlers to
> make changes to the local repository; it has been developed in parallel with
> an extension that allows the server to rebase incoming changesets while applying
> them.

I'd very much like a look at that extension. Any plans to release it as OSS?

(series so far looks fine to me, was just curious about details)

>
> Aside from the transaction logic, the pulloperation class is used primarily as
> a logic-free data structure for storing state information.  This diff extracts
> the transaction logic into its own class that can be shared with push
> operations.
>
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -771,10 +771,8 @@
>          self.explicitbookmarks = bookmarks
>          # do we force pull?
>          self.force = force
> -        # the name the pull transaction
> -        self._trname = 'pull\n' + util.hidepassword(remote.url())
> -        # hold the transaction once created
> -        self._tr = None
> +        # transaction manager
> +        self.trmanager = None
>          # set of common changeset between local and remote before pull
>          self.common = None
>          # set of pulled head
> @@ -807,14 +805,30 @@
>              return self.heads
>
>      def gettransaction(self):
> -        """get appropriate pull transaction, creating it if needed"""
> -        if self._tr is None:
> -            self._tr = self.repo.transaction(self._trname)
> -            self._tr.hookargs['source'] = 'pull'
> -            self._tr.hookargs['url'] = self.remote.url()
> +        # deprecated; talk to trmanager directly
> +        return self.trmanager.transaction()
> +
> +class transactionmanager(object):
> +    """An object to manages the lifecycle of a transaction
> +
> +    It creates the transaction on demand and calls the appropriate hooks when
> +    closing the transaction."""
> +    def __init__(self, repo, source, url):
> +        self.repo = repo
> +        self.source = source
> +        self.url = url
> +        self._tr = None
> +
> +    def transaction(self):
> +        """Return an open transaction object, constructing if necessary"""
> +        if not self._tr:
> +            trname = '%s\n%s' % (self.source, util.hidepassword(self.url))
> +            self._tr = self.repo.transaction(trname)
> +            self._tr.hookargs['source'] = self.source
> +            self._tr.hookargs['url'] = self.url
>          return self._tr
>
> -    def closetransaction(self):
> +    def close(self):
>          """close transaction if created"""
>          if self._tr is not None:
>              repo = self.repo
> @@ -828,7 +842,7 @@
>                                    lambda tr: repo._afterlock(runhooks))
>              self._tr.close()
>
> -    def releasetransaction(self):
> +    def release(self):
>          """release transaction if created"""
>          if self._tr is not None:
>              self._tr.release()
> @@ -846,6 +860,7 @@
>      pullop.remotebookmarks = remote.listkeys('bookmarks')
>      lock = pullop.repo.lock()
>      try:
> +        pullop.trmanager = transactionmanager(repo, 'pull', remote.url())
>          _pulldiscovery(pullop)
>          if (pullop.repo.ui.configbool('experimental', 'bundle2-exp', False)
>              and pullop.remote.capable('bundle2-exp')):
> @@ -854,9 +869,9 @@
>          _pullphase(pullop)
>          _pullbookmarks(pullop)
>          _pullobsolete(pullop)
> -        pullop.closetransaction()
> +        pullop.trmanager.close()
>      finally:
> -        pullop.releasetransaction()
> +        pullop.trmanager.release()
>          lock.release()
>
>      return pullop
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel