[PATCH] transaction: reorder unlinking .hg/journal and .hg/journal.backupfiles

[Yuya Nishihara]

On Fri, 16 Oct 2015 03:52:40 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> # Date 1444933791 -32400
> #      Fri Oct 16 03:29:51 2015 +0900
> # Node ID 6fa58be5278941c147384bff82cda8de96cc2cd5
> # Parent  07db7e95c464537aeb2dd7aba39de0813eaffd04
> transaction: reorder unlinking .hg/journal and .hg/journal.backupfiles
> 
> After this reordering, absence of '.hg/journal' just before starting
> new transaction means also absence of '.hg/journal.backupfiles'.
> 
> In this case, all temporary files for preceding transaction should be
> completely unlinked, and HG_PENDING doesn't cause unintentional
> reading stalled temporary files in.
> 
> Otherwise, 'repo.transaction()' raises exception with "run 'hg
> recover' to clean up transaction" hint.
> 
> diff --git a/mercurial/transaction.py b/mercurial/transaction.py
> --- a/mercurial/transaction.py
> +++ b/mercurial/transaction.py
> @@ -75,10 +75,10 @@
>              if not c:
>                  raise
>  
> -    opener.unlink(journal)
>      backuppath = "%s.backupfiles" % journal
>      if opener.exists(backuppath):
>          opener.unlink(backuppath)
> +    opener.unlink(journal)
>      try:
>          for f in backupfiles:
>              if opener.exists(f):
> @@ -427,10 +427,11 @@
>          self._writeundo()
>          if self.after:
>              self.after()
> +        if self.opener.isfile(self._backupjournal):
> +            self.opener.unlink(self._backupjournal)
>          if self.opener.isfile(self.journal):
>              self.opener.unlink(self.journal)
> -        if self.opener.isfile(self._backupjournal):
> -            self.opener.unlink(self._backupjournal)
> +        if True:
>              for l, _f, b, c in self._backupentries:
>                  if l not in self._vfsmap and c:
>                      self.report("couldn't remote %s: unknown cache location"

I'm afraid that this "if True" does more than the reordering. But I know little
about the transaction, maybe I'm wrong.

OT: "couldn't remote" is a typo of "couldn't remove" ?

> @@ -497,10 +498,10 @@
>  
>          try:
>              if not self.entries and not self._backupentries:
> +                if self._backupjournal:
> +                    self.opener.unlink(self._backupjournal)
>                  if self.journal:
>                      self.opener.unlink(self.journal)
> -                if self._backupjournal:
> -                    self.opener.unlink(self._backupjournal)
>                  return
>  
>              self.report(_("transaction abort!\n"))
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel