This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5504

narrow: make dirstateguard back up and restore working copy narrowspec instead
ClosedPublic

Authored by martinvonz on Jan 7 2019, 3:45 AM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHGb74481038438: narrow: make dirstateguard back up and restore working copy narrowspec instead
Summary

We used to have only one narrowspec for the store and the working
copy, but now that we have one narrowspec for each, it seems clear
that the dirstateguard was supposed to back up and restore the
narrowspec associated with the working copy, not the one associated
with the store.

clearbackup() (for the store narrowspec) is not needed because the
presence of the file in localrepository._journalfiles() takes care of
that.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Jan 7 2019, 3:45 AM
martinvonz updated this revision to Diff 13053.Jan 7 2019, 2:02 PM
pulkit added a subscriber: pulkit.Jan 10 2019, 1:52 PM

This one removes clearbackup() while restorebackup() and savebackup() are still there. Does that means that we are sure that we won't need to manually clear the backup from store?

martinvonz added a comment.Jan 10 2019, 2:24 PM
In D5504#82068, @pulkit wrote:

This one removes clearbackup() while restorebackup() and savebackup() are still there. Does that means that we are sure that we won't need to manually clear the backup from store?

Good question. It just meant that clearbackup() is no longer called and I didn't think further :P I think this patch is still correct, but I'll insert a patch that adds journal.narrowspec to localrepository._journalfiles().

martinvonz edited the summary of this revision. (Show Details)Jan 10 2019, 4:50 PM
martinvonz updated this revision to Diff 13144.
pulkit accepted this revision.Jan 16 2019, 2:18 AM
Closed by commit rHGb74481038438: narrow: make dirstateguard back up and restore working copy narrowspec instead (authored by martinvonz). · Explain WhyJan 16 2019, 2:49 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/dirstateguard.py (6 lines)
Mmercurial/narrowspec.py (19 lines)

Diff 13237

mercurial/dirstateguard.py

def __init__(self, repo, name): def __init__(self, repo, name):
self._repo = repo self._repo = repo
self._active = False self._active = False
self._closed = False self._closed = False
self._backupname = 'dirstate.backup.%s.%d' % (name, id(self)) self._backupname = 'dirstate.backup.%s.%d' % (name, id(self))
self._narrowspecbackupname = ('narrowspec.backup.%s.%d' % self._narrowspecbackupname = ('narrowspec.backup.%s.%d' %
(name, id(self))) (name, id(self)))
repo.dirstate.savebackup(repo.currenttransaction(), self._backupname) repo.dirstate.savebackup(repo.currenttransaction(), self._backupname)
narrowspec.savebackup(repo, self._narrowspecbackupname) narrowspec.savewcbackup(repo, self._narrowspecbackupname)
self._active = True self._active = True
def __del__(self): def __del__(self):
if self._active: # still active if self._active: # still active
# this may occur, even if this class is used correctly: # this may occur, even if this class is used correctly:
# for example, releasing other resources like transaction # for example, releasing other resources like transaction
# may raise exception before ``dirstateguard.release`` in # may raise exception before ``dirstateguard.release`` in
# ``release(tr, ....)``. # ``release(tr, ....)``.
self._abort() self._abort()
def close(self): def close(self):
if not self._active: # already inactivated if not self._active: # already inactivated
msg = (_("can't close already inactivated backup: %s") msg = (_("can't close already inactivated backup: %s")
% self._backupname) % self._backupname)
raise error.Abort(msg) raise error.Abort(msg)
self._repo.dirstate.clearbackup(self._repo.currenttransaction(), self._repo.dirstate.clearbackup(self._repo.currenttransaction(),
self._backupname) self._backupname)
narrowspec.clearbackup(self._repo, self._narrowspecbackupname) narrowspec.clearwcbackup(self._repo, self._narrowspecbackupname)
self._active = False self._active = False
self._closed = True self._closed = True
def _abort(self): def _abort(self):
narrowspec.restorebackup(self._repo, self._narrowspecbackupname) narrowspec.restorewcbackup(self._repo, self._narrowspecbackupname)
self._repo.dirstate.restorebackup(self._repo.currenttransaction(), self._repo.dirstate.restorebackup(self._repo.currenttransaction(),
self._backupname) self._backupname)
self._active = False self._active = False
def release(self): def release(self):
if not self._closed: if not self._closed:
if not self._active: # already inactivated if not self._active: # already inactivated
msg = (_("can't release already inactivated backup: %s") msg = (_("can't release already inactivated backup: %s")
% self._backupname) % self._backupname)
raise error.Abort(msg) raise error.Abort(msg)
self._abort() self._abort()

mercurial/narrowspec.py

svfs.tryunlink(backupname) svfs.tryunlink(backupname)
util.copyfile(svfs.join(FILENAME), svfs.join(backupname), hardlink=True) util.copyfile(svfs.join(FILENAME), svfs.join(backupname), hardlink=True)
def restorebackup(repo, backupname): def restorebackup(repo, backupname):
if repository.NARROW_REQUIREMENT not in repo.requirements: if repository.NARROW_REQUIREMENT not in repo.requirements:
return return
util.rename(repo.svfs.join(backupname), repo.svfs.join(FILENAME)) util.rename(repo.svfs.join(backupname), repo.svfs.join(FILENAME))
def clearbackup(repo, backupname): def savewcbackup(repo, backupname):
if repository.NARROW_REQUIREMENT not in repo.requirements: if repository.NARROW_REQUIREMENT not in repo.requirements:
return return
repo.svfs.unlink(backupname) vfs = repo.vfs
vfs.tryunlink(backupname)
# It may not exist in old repos
if vfs.exists(DIRSTATE_FILENAME):
util.copyfile(vfs.join(DIRSTATE_FILENAME), vfs.join(backupname),
hardlink=True)
def restorewcbackup(repo, backupname):
if repository.NARROW_REQUIREMENT not in repo.requirements:
return
util.rename(repo.vfs.join(backupname), repo.vfs.join(DIRSTATE_FILENAME))
def clearwcbackup(repo, backupname):
if repository.NARROW_REQUIREMENT not in repo.requirements:
return
repo.vfs.unlink(backupname)
def restrictpatterns(req_includes, req_excludes, repo_includes, repo_excludes): def restrictpatterns(req_includes, req_excludes, repo_includes, repo_excludes):
r""" Restricts the patterns according to repo settings, r""" Restricts the patterns according to repo settings,
results in a logical AND operation results in a logical AND operation
:param req_includes: requested includes :param req_includes: requested includes
:param req_excludes: requested excludes :param req_excludes: requested excludes
:param repo_includes: repo includes :param repo_includes: repo includes