[PATCH 02 of 10] subrepo: use "vfs.tryread()" instead of explicit invocations of file APIs
FUJIWARA Katsunori
foozy at lares.dti.ne.jp
Fri Jun 6 08:24:26 CDT 2014
At Thu, 05 Jun 2014 17:20:03 -0700,
Pierre-Yves David wrote:
>
> On 05/28/2014 08:00 AM, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> > # Date 1401288802 -32400
> > # Wed May 28 23:53:22 2014 +0900
> > # Node ID 59f48202458c5236a65631c9a43425e8176245de
> > # Parent 25cb27f9fa274069475fc8d1a129a272d9d7e147
> > subrepo: use "vfs.tryread()" instead of explicit invocations of file APIs
> >
> > This patch removes "_calcfilehash", because there is no code path
> > referring it.
> >
> > This patch also avoids building absolute path "absname" up by
> > "self._repo.join" for files in "filelist", because "vfs.tryread"
> > doesn't need it.
>
> use of "and" uin a commit description. Can this be two changesets?
'building absolute path "absname" up' becomes fully meaningless by
using 'vfs.tryread()' instread of '_calcfilehash()'. So, I think that
avoiding 'building absolute path "absname" up' and using
'vfs.tryread()' should be applied at the same time.
But separating removal of "_calcfilehash" into another patch may make
this one easier to be read.
I'll try to do so (and write more detailed description) in V2 series
> > diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> > --- a/mercurial/subrepo.py
> > +++ b/mercurial/subrepo.py
> > @@ -31,14 +31,6 @@ def _getstorehashcachename(remotepath):
> > '''get a unique filename for the store hash cache of a remote repository'''
> > return util.sha1(_expandedabspath(remotepath)).hexdigest()[0:12]
> >
> > -def _calcfilehash(filename):
> > - data = ''
> > - if os.path.exists(filename):
> > - fd = open(filename, 'rb')
> > - data = fd.read()
> > - fd.close()
> > - return util.sha1(data).hexdigest()
> > -
> > class SubrepoAbort(error.Abort):
> > """Exception class used to avoid handling a subrepo error more than once"""
> > def __init__(self, *args, **kw):
> > @@ -554,9 +546,10 @@ class hgsubrepo(abstractsubrepo):
> > # sort the files that will be hashed in increasing (likely) file size
> > filelist = ('bookmarks', 'store/phaseroots', 'store/00changelog.i')
> > yield '# %s\n' % _expandedabspath(remotepath)
> > + vfs = self._repo.vfs
> > for relname in filelist:
> > - absname = os.path.normpath(self._repo.join(relname))
> > - yield '%s = %s\n' % (relname, _calcfilehash(absname))
> > + yield '%s = %s\n' % (relname,
> > + util.sha1(vfs.tryread(relname)).hexdigest())
>
> You can maybe use an intermediate variable to avoid the need to spawn
> this on two lines.
I thought that an intermediate variable in this case might be
redundant at working for this series.
I'll introduce an intermediate variable for readability.
>
> --
> Pierre-Yves David
>
----------------------------------------------------------------------
[FUJIWARA Katsunori] foozy at lares.dti.ne.jp
More information about the Mercurial-devel
mailing list