[PATCH 2 of 2] shelve: use backup functions instead of manually copying dirstate

FUJIWARA Katsunori foozy at lares.dti.ne.jp
Wed May 25 06:09:20 EDT 2016


At Tue, 24 May 2016 14:15:52 -0700,
Mateusz Kwapich wrote:
> 
> # HG changeset patch
> # User Mateusz Kwapich <mitrandir at fb.com>
> # Date 1464121793 25200
> #      Tue May 24 13:29:53 2016 -0700
> # Node ID 5f509e6681089b79eccf7890a4632ef45006eb6d
> # Parent  0b235895f8304569f6b12722fe2c080a83261ce3
> shelve: use backup functions instead of manually copying dirstate
> 
> This increases encapsulation of dirstate: the dirstate file is private
> to the dirstate module and shouldn't be touched by extensions directly.

Cleanup for this code path itself seems good (and thank you for taking
care of my hacky work :-)).

But I'm not sure whether this "crossing transaction scope boundary" of
save/restore scope is reasonable.

So, +0 for this patch (I don't' have any better idea)

> diff --git a/hgext/shelve.py b/hgext/shelve.py
> --- a/hgext/shelve.py
> +++ b/hgext/shelve.py
> @@ -226,28 +226,10 @@ def cleanupoldbackups(repo):
>  def _aborttransaction(repo):
>      '''Abort current transaction for shelve/unshelve, but keep dirstate
>      '''
> -    backupname = 'dirstate.shelve'
> -    dirstatebackup = None
> -    try:
> -        # create backup of (un)shelved dirstate, because aborting transaction
> -        # should restore dirstate to one at the beginning of the
> -        # transaction, which doesn't include the result of (un)shelving
> -        fp = repo.vfs.open(backupname, "w")
> -        dirstatebackup = backupname
> -        # clearing _dirty/_dirtypl of dirstate by _writedirstate below
> -        # is unintentional. but it doesn't cause problem in this case,
> -        # because no code path refers them until transaction is aborted.
> -        repo.dirstate._writedirstate(fp) # write in-memory changes forcibly
> -
> -        tr = repo.currenttransaction()
> -        tr.abort()
> -
> -        # restore to backuped dirstate
> -        repo.vfs.rename(dirstatebackup, 'dirstate')
> -        dirstatebackup = None
> -    finally:
> -        if dirstatebackup:
> -            repo.vfs.unlink(dirstatebackup)
> +    tr = repo.currenttransaction()
> +    repo.dirstate.savebackup(tr, suffix='.shelve')
> +    tr.abort()
> +    repo.dirstate.restorebackup(None, suffix='.shelve')
>  
>  def createcmd(ui, repo, pats, opts):
>      """subcommand that creates a new shelve"""
> _______________________________________________
> 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


More information about the Mercurial-devel mailing list