Patchwork [2,of,3,RFC] context: start the move of wlock from repo to workingctx

login
register
mail settings
Submitter Sean Farley
Date June 1, 2017, 12:25 a.m.
Message ID <11f1b897d1475648a88f.1496276721@1.0.0.127.in-addr.arpa>
Download mbox | patch
Permalink /patch/21110/
State Accepted, archived
Headers show

Comments

Sean Farley - June 1, 2017, 12:25 a.m.
# HG changeset patch
# User Sean Farley <sean@farley.io>
# Date 1494537056 25200
#      Thu May 11 14:10:56 2017 -0700
# Branch wctxds
# Node ID 11f1b897d1475648a88ff0115954413f46bf4137
# Parent  498dae194ccf1e82caed51a02e6ce0b77f8d92e8
context: start the move of wlock from repo to workingctx
Yuya Nishihara - June 1, 2017, 2:31 p.m.
On Wed, 31 May 2017 17:25:21 -0700, Sean Farley wrote:
> # HG changeset patch
> # User Sean Farley <sean@farley.io>
> # Date 1494537056 25200
> #      Thu May 11 14:10:56 2017 -0700
> # Branch wctxds
> # Node ID 11f1b897d1475648a88ff0115954413f46bf4137
> # Parent  498dae194ccf1e82caed51a02e6ce0b77f8d92e8
> context: start the move of wlock from repo to workingctx

Does it mean eventually workingctx will never be created more than once
per repository?

Current context objects aren't persistent, but dirstate and wlock should
be single source of truth.
Sean Farley - June 1, 2017, 6:28 p.m.
Yuya Nishihara <yuya@tcha.org> writes:

> On Wed, 31 May 2017 17:25:21 -0700, Sean Farley wrote:
>> # HG changeset patch
>> # User Sean Farley <sean@farley.io>
>> # Date 1494537056 25200
>> #      Thu May 11 14:10:56 2017 -0700
>> # Branch wctxds
>> # Node ID 11f1b897d1475648a88ff0115954413f46bf4137
>> # Parent  498dae194ccf1e82caed51a02e6ce0b77f8d92e8
>> context: start the move of wlock from repo to workingctx
>
> Does it mean eventually workingctx will never be created more than once
> per repository?
>
> Current context objects aren't persistent, but dirstate and wlock should
> be single source of truth.

I haven't crossed that bridge yet, but yes, I think something like that
would need to happen. If you have ideas about how that could happen (or
are against it), please do tell.
Yuya Nishihara - June 2, 2017, 1:09 p.m.
On Thu, 01 Jun 2017 11:28:56 -0700, Sean Farley wrote:
> Yuya Nishihara <yuya@tcha.org> writes:
> > On Wed, 31 May 2017 17:25:21 -0700, Sean Farley wrote:
> >> # HG changeset patch
> >> # User Sean Farley <sean@farley.io>
> >> # Date 1494537056 25200
> >> #      Thu May 11 14:10:56 2017 -0700
> >> # Branch wctxds
> >> # Node ID 11f1b897d1475648a88ff0115954413f46bf4137
> >> # Parent  498dae194ccf1e82caed51a02e6ce0b77f8d92e8
> >> context: start the move of wlock from repo to workingctx
> >
> > Does it mean eventually workingctx will never be created more than once
> > per repository?
> >
> > Current context objects aren't persistent, but dirstate and wlock should
> > be single source of truth.
> 
> I haven't crossed that bridge yet, but yes, I think something like that
> would need to happen. If you have ideas about how that could happen (or
> are against it), please do tell.

My gut feeling is we'll need a storage object behind workingctx anyway, which
is currently served by localrepository. Maybe it's good idea to detach
dirstate, wlock, and the data guarded by wlock from localrepo, but the object
holding them wouldn't be a workingctx.
Sean Farley - June 2, 2017, 5:32 p.m.
Yuya Nishihara <yuya@tcha.org> writes:

> On Thu, 01 Jun 2017 11:28:56 -0700, Sean Farley wrote:
>> Yuya Nishihara <yuya@tcha.org> writes:
>> > On Wed, 31 May 2017 17:25:21 -0700, Sean Farley wrote:
>> >> # HG changeset patch
>> >> # User Sean Farley <sean@farley.io>
>> >> # Date 1494537056 25200
>> >> #      Thu May 11 14:10:56 2017 -0700
>> >> # Branch wctxds
>> >> # Node ID 11f1b897d1475648a88ff0115954413f46bf4137
>> >> # Parent  498dae194ccf1e82caed51a02e6ce0b77f8d92e8
>> >> context: start the move of wlock from repo to workingctx
>> >
>> > Does it mean eventually workingctx will never be created more than once
>> > per repository?
>> >
>> > Current context objects aren't persistent, but dirstate and wlock should
>> > be single source of truth.
>> 
>> I haven't crossed that bridge yet, but yes, I think something like that
>> would need to happen. If you have ideas about how that could happen (or
>> are against it), please do tell.
>
> My gut feeling is we'll need a storage object behind workingctx anyway, which
> is currently served by localrepository. Maybe it's good idea to detach
> dirstate, wlock, and the data guarded by wlock from localrepo, but the object
> holding them wouldn't be a workingctx.

Not a bad idea. I'll look into that this weekend.

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
index a3dc7c5..c42c5e6 100644
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -1564,10 +1564,11 @@  class workingctx(committablectx):
                  changes=None):
         super(workingctx, self).__init__(repo, text, user, date, extra, changes)
 
         # This is where working directory references should live. For now we
         # just borrow references to stuff in localrepo.
+        self.wlock = self._repo.wlock
 
     @property
     def dirstate(self):
         return self._repo.dirstate
 
@@ -1607,11 +1608,11 @@  class workingctx(committablectx):
                 self.modified() or self.added() or self.removed() or
                 (missing and self.deleted()))
 
     def add(self, list, prefix=""):
         join = lambda f: os.path.join(prefix, f)
-        with self._repo.wlock():
+        with self.wlock():
             ui, ds = self._repo.ui, self.dirstate
             rejected = []
             lstat = self._repo.wvfs.lstat
             for f in list:
                 scmutil.checkportable(ui, join(f))
@@ -1639,11 +1640,11 @@  class workingctx(committablectx):
                     ds.add(f)
             return rejected
 
     def forget(self, files, prefix=""):
         join = lambda f: os.path.join(prefix, f)
-        with self._repo.wlock():
+        with self.wlock():
             rejected = []
             for f in files:
                 if f not in self.dirstate:
                     self._repo.ui.warn(_("%s not tracked!\n") % join(f))
                     rejected.append(f)
@@ -1653,11 +1654,11 @@  class workingctx(committablectx):
                     self.dirstate.drop(f)
             return rejected
 
     def undelete(self, list):
         pctxs = self.parents()
-        with self._repo.wlock():
+        with self.wlock():
             for f in list:
                 if self.dirstate[f] != 'r':
                     self._repo.ui.warn(_("%s not removed!\n") % f)
                 else:
                     fctx = f in pctxs[0] and pctxs[0][f] or pctxs[1][f]
@@ -1675,11 +1676,11 @@  class workingctx(committablectx):
             return
         if not (stat.S_ISREG(st.st_mode) or stat.S_ISLNK(st.st_mode)):
             self._repo.ui.warn(_("copy failed: %s is not a file or a "
                                  "symbolic link\n") % dest)
         else:
-            with self._repo.wlock():
+            with self.wlock():
                 if self.dirstate[dest] in '?':
                     self.dirstate.add(dest)
                 elif self.dirstate[dest] in 'r':
                     self.dirstate.normallookup(dest)
                 self.dirstate.copy(source, dest)
@@ -1738,11 +1739,11 @@  class workingctx(committablectx):
             try:
                 # updating the dirstate is optional
                 # so we don't wait on the lock
                 # wlock can invalidate the dirstate, so cache normal _after_
                 # taking the lock
-                with self._repo.wlock(False):
+                with self.wlock(False):
                     normal = self.dirstate.normal
                     for f in fixup:
                         normal(f)
                     # write changes out explicitly, because nesting
                     # wlock at runtime may prevent 'wlock.release()'