[PATCH STABLE?] commit: avoid a dirstate race with multiple commits in the same process

Greg Ward greg at gerg.ca
Tue Mar 15 21:19:36 CDT 2011


On 16 March 2011, Adrian Buehlmann said:
> > +    def getfstime(self, dir):
> > +        '''Return the current time to filesystem resolution.'''
> 
> I somehow have a hard time with the grammar and semantic in this doc
> sentence. Maybe:
> 
>            '''Return the filesystem's notion of "now", rounded down
>            to the dirstate's one second resolution'''

That's true on filesystems with one second resolution (e.g. most POSIX
filesystems).  But on legacy MS-DOS filesystems, the resolution is 2
seconds.  Or at least I *assume* that is why dirstate.write() uses
stat() + mtime to get the current time instead of just time.time().

> > +
> > +    def maybelookup(self, f, now):
> 
> I'm still pondering about a better name for this function. Proposals for
> bikeshedding follow below:
> 
>        def validatemtime(self, f, now):

Meh.  maybelookup() isn't perfect, but I prefer it to validatemtime().
Keep trying... ;-)

> > +        '''Examine f to determine if it needs more work to determine its
> > +        true status, or whether it can be considered normal.  If more
> > +        work needed, set the in-memory state to lookup; otherwise, leave
> > +        it alone. Thus, maybelookup() can affect the next call to
> > +        status(). now must be the current time to filesystem resolution
> > +        (see getfstime()).'''
> 
> I think something like
> 
>            '''Examine the entry for file f and invalidate it, if its
>            mtime is too close to "now" to be useful.'''
> 
> should be enough together with (<cough>) my preexisting fat comment inside.

Ahh yes, now you are hitting the nail on the head.  Much better
docstring.  Let's see what Matt thinks of my split-and-resend; if I
need to send it again, I'll incorporate this docstring.

And, BTW, your fat comment is still extremely useful.  This stuff is
not even remotely obvious; every little bit helps.

Thanks for the review!

        Greg


More information about the Mercurial-devel mailing list