[PATCH V2] transaction: allow running file generators after finalizers

[FUJIWARA Katsunori]

At Tue, 29 Mar 2016 18:14:42 +0900,
FUJIWARA Katsunori wrote:
> 
> At Sat, 26 Mar 2016 22:15:50 -0700,
> Pierre-Yves David wrote:
> > 
> > On 03/20/2016 10:49 AM, Durham Goode wrote:
> > > # HG changeset patch
> > > # User Durham Goode <durham at fb.com>
> > > # Date 1458495989 25200
> > > #      Sun Mar 20 10:46:29 2016 -0700
> > > # Node ID 1431f0447a6b0be851f609b386fabf83b2f54666
> > > # Parent  ed75909c4c670a7d9db4a2bef9817a0d5f0b4d9c
> > > transaction: allow running file generators after finalizers
> 
> [...]
> 
> > > […]
> > > @@ -276,12 +284,17 @@ 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, postfinalize=False, suffix=''):
> > 
> > The postfinalize=False as the default is an issue here,
> > 
> > Before this patch tr._generatefiles() would get all file generated. 
> > After that patch, only part of them are. Third parties and future 
> > callers are going to get this wrong and introduce various transaction 
> > bug. This is highlighted by the fact they had to be a special case when 
> > 'suffix' is passed.
> > 
> > I think we need three values here: 'all', 'precl', 'postcl':
> > 
> >    def _generatefiles(self, suffix='', group='all')
> 
> If possible, wording without abbreviation is friendly to
> non-English-native developer, like me :-)
> 
> If "preclose"/"postclose" is long, how about "closing"/"closed" ?

I remembered that "cl" is often used as an abbreviation of "changelog"
in Mercurial source. Would you mean pre/post-changelog(-finalization) ?


> > This is a small change but I think this is a critical enough code that 
> > it deserved to be expurged from surprise.
> > 
> > 
> > Also, we should try to avoid changing the position of arguments, this 
> > gratuitously break other callers.
> > 
> > >           # 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
> > > +            if not suffix and (id in postfinalizegenerators) != (postfinalize):
> > > +                continue
> > > +
> > 
> > [above, the suffix "hack" I was referring too]
> > 
> > > […]
> > 
> > Cheers,
> > 
> > -- 
> > Pierre-Yves David
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel at mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> 
> ----------------------------------------------------------------------
> [FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp