[PATCH 1 of 2] dirstate: don't use actualfilename to name the backup file

[FUJIWARA Katsunori]

At Tue, 24 May 2016 14:15:51 -0700,
Mateusz Kwapich wrote:
> 
> # HG changeset patch
> # User Mateusz Kwapich <mitrandir at fb.com>
> # Date 1464124006 25200
> #      Tue May 24 14:06:46 2016 -0700
> # Node ID 0b235895f8304569f6b12722fe2c080a83261ce3
> # Parent  ceca932c080d0de52ece0e8dd0226f006c7b9cb1
> dirstate: don't use actualfilename to name the backup file
> 
> The issue with using actualfilename is that dirstate saved during transaction
> with "pending" in filename will be impossible to recover from outside of the
> transaction because the recover method will be looking for the name without
> "pending".
> 
> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
> --- a/mercurial/dirstate.py
> +++ b/mercurial/dirstate.py
> @@ -1228,7 +1228,8 @@ class dirstate(object):
>              # end of this transaction
>              tr.registertmp(filename, location='plain')
>  
> -        self._opener.write(prefix + filename + suffix,
> +        filename = self._actualfilename(tr)

This 'filename' initialization is redundant, because it is already
initialized at the beginning of savebackup() by result of
_actualfilename(tr).


Maybe, '.pending' in name of backup file is useful only for manual
debugging or whitebox test. Therefore, omitting it in this patch seems
reasonable.


BTW (this is not related to this series), to avoid accidental writing
changes into original file, is "assert prefix or suffix" needed at the
beginning of savebackup(), because both prefix and suffix can be
omitted after 930d4ee4647e ?


> +        self._opener.write(prefix + self._filename + suffix,
>                             self._opener.tryread(filename))
>  
>      def restorebackup(self, tr, suffix='', prefix=''):
> @@ -1237,9 +1238,10 @@ class dirstate(object):
>          # changes of dirstate out after restoring from backup file
>          self.invalidate()
>          filename = self._actualfilename(tr)
> -        self._opener.rename(prefix + filename + suffix, filename)
> +        # using self._filename to avoid having "pending" in the backup filename
> +        self._opener.rename(prefix + self._filename + suffix, filename)
>  
>      def clearbackup(self, tr, suffix='', prefix=''):
>          '''Clear backup file with suffix'''
> -        filename = self._actualfilename(tr)
> -        self._opener.unlink(prefix + filename + suffix)
> +        # using self._filename to avoid having "pending" in the backup filename
> +        self._opener.unlink(prefix + self._filename + suffix)
> _______________________________________________
> 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