[PATCH 1 of 5] transaction: introduce scope

[Martin von Zweigbergk]

On Mon, Feb 27, 2017 at 10:16 AM, Jun Wu <quark at fb.com> wrote:
> Excerpts from Gregory Szorc's message of 2017-02-27 10:05:52 -0800:
>>
>> > On Feb 27, 2017, at 09:35, Jun Wu <quark at fb.com> wrote:
>> >
>> > # HG changeset patch
>> > # User Jun Wu <quark at fb.com>
>> > # Date 1488185788 28800
>> > #      Mon Feb 27 00:56:28 2017 -0800
>> > # Node ID 5dac612ec9a87553fcd329100846bcb01bae9d80
>> > # Parent  b4cb86ab4c719eb615a4308eafd8b1386a511eeb
>> > # Available At https://bitbucket.org/quark-zju/hg-draft
>> > #              hg pull https://bitbucket.org/quark-zju/hg-draft  -r 5dac612ec9a8
>> > transaction: introduce scope
>> >
>> > Previously, transaction tries to cover everything: bookmarks, dirstate,
>> > phaseroots, and branch. It backs them up unconditionally.
>> >
>> > That could be inefficient if we still back up the dirstate in a giant repo,
>> > where the transaction is only intended to change bookmarks.
>> >
>> > This patch introduces "scope" to transactions, which is a set of things that
>> > the transaction should cover. For bookmark-only update, it could avoid
>> > specifying the "dirstate" scope to be more efficient.
>> >
>> > diff --git a/mercurial/transaction.py b/mercurial/transaction.py
>> > --- a/mercurial/transaction.py
>> > +++ b/mercurial/transaction.py
>> > @@ -102,5 +102,6 @@ def _playback(journal, report, opener, v
>> > class transaction(object):
>> >     def __init__(self, report, opener, vfsmap, journalname, undoname=None,
>> > -                 after=None, createmode=None, validator=None, releasefn=None):
>> > +                 after=None, createmode=None, validator=None, releasefn=None,
>> > +                 scope=None):
>> >         """Begin a new transaction
>> >
>> > @@ -111,4 +112,5 @@ class transaction(object):
>> >         * `createmode`: the mode of the journal file that will be created
>> >         * `releasefn`: called after releasing (with transaction and result)
>> > +        * `scope`: a set-like, nested transaction's scope must be a subset
>> >         """
>> >         self.count = 1
>> > @@ -171,4 +173,7 @@ class transaction(object):
>> >         self._abortcallback = {}
>> >
>> > +        # a set indicating what's covered
>> > +        self._scope = scope or frozenset()
>> > +
>> >     def __del__(self):
>> >         if self.journal:
>> > @@ -342,5 +347,10 @@ class transaction(object):
>> >
>> >     @active
>> > -    def nest(self):
>> > +    def nest(self, scope=None):
>> > +        scope = scope or frozenset()
>> > +        if not scope.issubset(self._scope):
>> > +            raise error.ProgrammingError(
>> > +                'nested transaction has a superset scope (%s > %s)'
>> > +                % (scope, self._scope))
>>
>> Sets are unordered, so this message isn't deterministic. If you think this could interfere with testing, you should throw some sorted() in here.
>
> Good advice!
>
>> >         self.count += 1
>> >         self.usages += 1
>> > @@ -555,4 +565,7 @@ class transaction(object):
>> >             self.releasefn(self, False) # notify failure of transaction
>> >
>> > +    def scope(self):
>> > +        return self._scope
>> > +
>>
>> This seems like an anti-pattern. Can the attribute not be exposed directly?
>
> Maybe a "hasscope(scopename) -> bool" method.

I'm with Greg about exposing it directly.

>
> I'll wait for more comments before sending a V2 (with some tests).
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel