| Submitter | Durham Goode |
|---|---|
| Date | April 7, 2016, 9:11 p.m. |
| Message ID | <5e49537579979f112688.1460063501@dev8486.prn1.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/14424/ |
| State | Accepted |
| Headers | show |
Comments
On Thu, Apr 07, 2016 at 02:11:41PM -0700, Durham Goode wrote: > # HG changeset patch > # User Durham Goode <durham@fb.com> > # Date 1460063449 25200 > # Thu Apr 07 14:10:49 2016 -0700 > # Node ID 5e49537579979f11268847526f83bb9dc2baf8d8 > # Parent ea86cdcd9b50bf38c6b9dd7bbaa04b9c8cc0aefb > transaction: allow running file generators after finalizers > queued, thanks > Previously, transaction.close would run the file generators before running the > finalizers (see the list below for what is in each). Since file generators > contain the bookmarks and the dirstate, this meant we made the dirstate and > bookmarks visible to external readers before we actually wrote the commits into > the changelog, which could result in missing bookmarks and missing working copy > parents (especially on servers with high commit throughput, since pulls might > fail to see certain bookmarks in this situation). > > By moving the changelog writing to be before the bookmark/dirstate writing, we > ensure the commits are present before they are referenced. > > This implementation allows certain file generators to be after the finalizers. > We didn't want to move all of the generators, since it's important that things > like phases actually run before the finalizers (otherwise you could expose > commits as public when they really shouldn't be). > > For reference, file generators currently consist of: bookmarks, dirstate, and > phases. Finalizers currently consist of: changelog, revbranchcache, and fncache. > > diff --git a/mercurial/transaction.py b/mercurial/transaction.py > --- a/mercurial/transaction.py > +++ b/mercurial/transaction.py > @@ -23,6 +23,19 @@ from . import ( > > version = 2 > > +# These are the file generators that should only be executed after the > +# finalizers are done, since they rely on the output of the finalizers (like > +# the changelog having been written). > +postfinalizegenerators = set([ > + 'bookmarks', > + 'dirstate' > +]) > + > +class GenerationGroup(object): > + ALL='all' > + PREFINALIZE='prefinalize' > + POSTFINALIZE='postfinalize' > + > def active(func): > def _active(self, *args, **kwds): > if self.count == 0: > @@ -276,12 +289,19 @@ class transaction(object): > # but for bookmarks that are handled outside this mechanism. > self._filegenerators[genid] = (order, filenames, genfunc, location) > > - def _generatefiles(self, suffix=''): > + def _generatefiles(self, suffix='', group=GenerationGroup.ALL): > # write files registered for generation > any = False > - for entry in sorted(self._filegenerators.values()): > + for id, entry in sorted(self._filegenerators.iteritems()): > any = True > order, filenames, genfunc, location = entry > + > + # for generation at closing, check if it's before or after finalize > + postfinalize = group == GenerationGroup.POSTFINALIZE > + if (group != GenerationGroup.ALL and > + (id in postfinalizegenerators) != (postfinalize)): > + continue > + > vfs = self._vfsmap[location] > files = [] > try: > @@ -407,10 +427,11 @@ class transaction(object): > '''commit the transaction''' > if self.count == 1: > self.validator(self) # will raise exception if needed > - self._generatefiles() > + self._generatefiles(group=GenerationGroup.PREFINALIZE) > categories = sorted(self._finalizecallback) > for cat in categories: > self._finalizecallback[cat](self) > + self._generatefiles(group=GenerationGroup.POSTFINALIZE) > > self.count -= 1 > if self.count != 0: > diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t > --- a/tests/test-bookmarks.t > +++ b/tests/test-bookmarks.t > @@ -846,3 +846,52 @@ updates the working directory and curren > 6:81dcce76aa0b > $ hg -R ../cloned-bookmarks-update bookmarks | grep ' Y ' > * Y 6:81dcce76aa0b > + > + $ cd .. > + > +ensure changelog is written before bookmarks > + $ hg init orderrepo > + $ cd orderrepo > + $ touch a > + $ hg commit -Aqm one > + $ hg book mybook > + $ echo a > a > + > + $ cat > $TESTTMP/pausefinalize.py <<EOF > + > from mercurial import extensions, localrepo > + > import os, time > + > def transaction(orig, self, desc, report=None): > + > tr = orig(self, desc, report) > + > def sleep(*args, **kwargs): > + > retry = 20 > + > while retry > 0 and not os.path.exists("$TESTTMP/unpause"): > + > retry -= 1 > + > time.sleep(0.5) > + > if os.path.exists("$TESTTMP/unpause"): > + > os.remove("$TESTTMP/unpause") > + > # It is important that this finalizer start with 'a', so it runs before > + > # the changelog finalizer appends to the changelog. > + > tr.addfinalize('a-sleep', sleep) > + > return tr > + > > + > def extsetup(ui): > + > # This extension inserts an artifical pause during the transaction > + > # finalizer, so we can run commands mid-transaction-close. > + > extensions.wrapfunction(localrepo.localrepository, 'transaction', > + > transaction) > + > EOF > + $ hg commit -qm two --config extensions.pausefinalize=$TESTTMP/pausefinalize.py & > + $ sleep 2 > + $ hg log -r . > + changeset: 0:867bc5792c8c > + bookmark: mybook > + tag: tip > + user: test > + date: Thu Jan 01 00:00:00 1970 +0000 > + summary: one > + > + $ hg bookmarks > + * mybook 0:867bc5792c8c > + $ touch $TESTTMP/unpause > + > + $ cd .. > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Patch
diff --git a/mercurial/transaction.py b/mercurial/transaction.py --- a/mercurial/transaction.py +++ b/mercurial/transaction.py @@ -23,6 +23,19 @@ from . import ( version = 2 +# These are the file generators that should only be executed after the +# finalizers are done, since they rely on the output of the finalizers (like +# the changelog having been written). +postfinalizegenerators = set([ + 'bookmarks', + 'dirstate' +]) + +class GenerationGroup(object): + ALL='all' + PREFINALIZE='prefinalize' + POSTFINALIZE='postfinalize' + def active(func): def _active(self, *args, **kwds): if self.count == 0: @@ -276,12 +289,19 @@ class transaction(object): # but for bookmarks that are handled outside this mechanism. self._filegenerators[genid] = (order, filenames, genfunc, location) - def _generatefiles(self, suffix=''): + def _generatefiles(self, suffix='', group=GenerationGroup.ALL): # write files registered for generation any = False - for entry in sorted(self._filegenerators.values()): + for id, entry in sorted(self._filegenerators.iteritems()): any = True order, filenames, genfunc, location = entry + + # for generation at closing, check if it's before or after finalize + postfinalize = group == GenerationGroup.POSTFINALIZE + if (group != GenerationGroup.ALL and + (id in postfinalizegenerators) != (postfinalize)): + continue + vfs = self._vfsmap[location] files = [] try: @@ -407,10 +427,11 @@ class transaction(object): '''commit the transaction''' if self.count == 1: self.validator(self) # will raise exception if needed - self._generatefiles() + self._generatefiles(group=GenerationGroup.PREFINALIZE) categories = sorted(self._finalizecallback) for cat in categories: self._finalizecallback[cat](self) + self._generatefiles(group=GenerationGroup.POSTFINALIZE) self.count -= 1 if self.count != 0: diff --git a/tests/test-bookmarks.t b/tests/test-bookmarks.t --- a/tests/test-bookmarks.t +++ b/tests/test-bookmarks.t @@ -846,3 +846,52 @@ updates the working directory and curren 6:81dcce76aa0b $ hg -R ../cloned-bookmarks-update bookmarks | grep ' Y ' * Y 6:81dcce76aa0b + + $ cd .. + +ensure changelog is written before bookmarks + $ hg init orderrepo + $ cd orderrepo + $ touch a + $ hg commit -Aqm one + $ hg book mybook + $ echo a > a + + $ cat > $TESTTMP/pausefinalize.py <<EOF + > from mercurial import extensions, localrepo + > import os, time + > def transaction(orig, self, desc, report=None): + > tr = orig(self, desc, report) + > def sleep(*args, **kwargs): + > retry = 20 + > while retry > 0 and not os.path.exists("$TESTTMP/unpause"): + > retry -= 1 + > time.sleep(0.5) + > if os.path.exists("$TESTTMP/unpause"): + > os.remove("$TESTTMP/unpause") + > # It is important that this finalizer start with 'a', so it runs before + > # the changelog finalizer appends to the changelog. + > tr.addfinalize('a-sleep', sleep) + > return tr + > + > def extsetup(ui): + > # This extension inserts an artifical pause during the transaction + > # finalizer, so we can run commands mid-transaction-close. + > extensions.wrapfunction(localrepo.localrepository, 'transaction', + > transaction) + > EOF + $ hg commit -qm two --config extensions.pausefinalize=$TESTTMP/pausefinalize.py & + $ sleep 2 + $ hg log -r . + changeset: 0:867bc5792c8c + bookmark: mybook + tag: tip + user: test + date: Thu Jan 01 00:00:00 1970 +0000 + summary: one + + $ hg bookmarks + * mybook 0:867bc5792c8c + $ touch $TESTTMP/unpause + + $ cd ..