D1340: dirstate: add explicit methods for modifying dirstate

2017-11-17 Thread mbthomas (Mark Thomas) 
This revision was automatically updated to reflect the committed changes.
Closed by commit rHGd85f598b24e0: dirstate: add explicit methods for modifying 
dirstate (authored by mbthomas, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D1340?vs=3514=3613

REVISION DETAIL
  https://phab.mercurial-scm.org/D1340

AFFECTED FILES
  mercurial/dirstate.py

CHANGE DETAILS

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -415,11 +415,11 @@
 self._map.dirs.addpath(f)
 self._dirty = True
 self._updatedfiles.add(f)
-self._map[f] = dirstatetuple(state, mode, size, mtime)
 if state != 'n' or mtime == -1:
 self._map.nonnormalset.add(f)
 if size == -2:
 self._map.otherparentset.add(f)
+self._map.addfile(f, state, mode, size, mtime)
 
 def normal(self, f):
 '''Mark a file normal and clean.'''
@@ -490,8 +490,8 @@
 elif entry[0] == 'n' and entry[2] == -2: # other parent
 size = -2
 self._map.otherparentset.add(f)
-self._map[f] = dirstatetuple('r', 0, size, 0)
 self._map.nonnormalset.add(f)
+self._map.removefile(f, size)
 if size == 0:
 self._map.copymap.pop(f, None)
 
@@ -503,10 +503,9 @@
 
 def drop(self, f):
 '''Drop a file from the dirstate'''
-if f in self._map:
+if self._map.dropfile(f):
 self._dirty = True
 self._droppath(f)
-del self._map[f]
 if f in self._map.nonnormalset:
 self._map.nonnormalset.remove(f)
 self._map.copymap.pop(f, None)
@@ -636,7 +635,7 @@
 for f in self._updatedfiles:
 e = dmap.get(f)
 if e is not None and e[0] == 'n' and e[3] == now:
-dmap[f] = dirstatetuple(e[0], e[1], e[2], -1)
+dmap.addfile(f, e[0], e[1], e[2], -1)
 self._map.nonnormalset.add(f)
 
 # emulate that all 'dirstate.normal' results are written out
@@ -1207,8 +1206,9 @@
 
 - the state map maps filenames to tuples of (state, mode, size, mtime),
   where state is a single character representing 'normal', 'added',
-  'removed', or 'merged'. It is accessed by treating the dirstate as a
-  dict.
+  'removed', or 'merged'. It is read by treating the dirstate as a
+  dict.  File state is updated by calling the `addfile`, `removefile` and
+  `dropfile` methods.
 
 - `copymap` maps destination filenames to their source filename.
 
@@ -1282,22 +1282,37 @@
 def __contains__(self, key):
 return key in self._map
 
-def __setitem__(self, key, value):
-self._map[key] = value
-
 def __getitem__(self, key):
 return self._map[key]
 
-def __delitem__(self, key):
-del self._map[key]
-
 def keys(self):
 return self._map.keys()
 
 def preload(self):
 """Loads the underlying data, if it's not already loaded"""
 self._map
 
+def addfile(self, f, state, mode, size, mtime):
+"""Add a tracked file to the dirstate."""
+self._map[f] = dirstatetuple(state, mode, size, mtime)
+
+def removefile(self, f, size):
+"""
+Mark a file as removed in the dirstate.
+
+The `size` parameter is used to store sentinel values that indicate
+the file's previous state.  In the future, we should refactor this
+to be more explicit about what that state is.
+"""
+self._map[f] = dirstatetuple('r', 0, size, 0)
+
+def dropfile(self, f):
+"""
+Remove a file from the dirstate.  Returns True if the file was
+previously recorded.
+"""
+return self._map.pop(f, None) is not None
+
 def nonnormalentries(self):
 '''Compute the nonnormal dirstate entries from the dmap'''
 try:
@@ -1427,8 +1442,6 @@
 # Avoid excess attribute lookups by fast pathing certain checks
 self.__contains__ = self._map.__contains__
 self.__getitem__ = self._map.__getitem__
-self.__setitem__ = self._map.__setitem__
-self.__delitem__ = self._map.__delitem__
 self.get = self._map.get
 
 def write(self, st, now):



To: mbthomas, #hg-reviewers, durin42
Cc: durin42, mbolin, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1340: dirstate: add explicit methods for modifying dirstate

2017-11-15 Thread mbthomas (Mark Thomas) 
mbthomas updated this revision to Diff 3514.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D1340?vs=3437=3514

REVISION DETAIL
  https://phab.mercurial-scm.org/D1340

AFFECTED FILES
  mercurial/dirstate.py

CHANGE DETAILS

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -415,11 +415,11 @@
 self._map.dirs.addpath(f)
 self._dirty = True
 self._updatedfiles.add(f)
-self._map[f] = dirstatetuple(state, mode, size, mtime)
 if state != 'n' or mtime == -1:
 self._map.nonnormalset.add(f)
 if size == -2:
 self._map.otherparentset.add(f)
+self._map.addfile(f, state, mode, size, mtime)
 
 def normal(self, f):
 '''Mark a file normal and clean.'''
@@ -490,8 +490,8 @@
 elif entry[0] == 'n' and entry[2] == -2: # other parent
 size = -2
 self._map.otherparentset.add(f)
-self._map[f] = dirstatetuple('r', 0, size, 0)
 self._map.nonnormalset.add(f)
+self._map.removefile(f, size)
 if size == 0:
 self._map.copymap.pop(f, None)
 
@@ -503,10 +503,9 @@
 
 def drop(self, f):
 '''Drop a file from the dirstate'''
-if f in self._map:
+if self._map.dropfile(f):
 self._dirty = True
 self._droppath(f)
-del self._map[f]
 if f in self._map.nonnormalset:
 self._map.nonnormalset.remove(f)
 self._map.copymap.pop(f, None)
@@ -636,7 +635,7 @@
 for f in self._updatedfiles:
 e = dmap.get(f)
 if e is not None and e[0] == 'n' and e[3] == now:
-dmap[f] = dirstatetuple(e[0], e[1], e[2], -1)
+dmap.addfile(f, e[0], e[1], e[2], -1)
 self._map.nonnormalset.add(f)
 
 # emulate that all 'dirstate.normal' results are written out
@@ -1207,8 +1206,9 @@
 
 - the state map maps filenames to tuples of (state, mode, size, mtime),
   where state is a single character representing 'normal', 'added',
-  'removed', or 'merged'. It is accessed by treating the dirstate as a
-  dict.
+  'removed', or 'merged'. It is read by treating the dirstate as a
+  dict.  File state is updated by calling the `addfile`, `removefile` and
+  `dropfile` methods.
 
 - `copymap` maps destination filenames to their source filename.
 
@@ -1282,22 +1282,37 @@
 def __contains__(self, key):
 return key in self._map
 
-def __setitem__(self, key, value):
-self._map[key] = value
-
 def __getitem__(self, key):
 return self._map[key]
 
-def __delitem__(self, key):
-del self._map[key]
-
 def keys(self):
 return self._map.keys()
 
 def preload(self):
 """Loads the underlying data, if it's not already loaded"""
 self._map
 
+def addfile(self, f, state, mode, size, mtime):
+"""Add a tracked file to the dirstate."""
+self._map[f] = dirstatetuple(state, mode, size, mtime)
+
+def removefile(self, f, size):
+"""
+Mark a file as removed in the dirstate.
+
+The `size` parameter is used to store sentinel values that indicate
+the file's previous state.  In the future, we should refactor this
+to be more explicit about what that state is.
+"""
+self._map[f] = dirstatetuple('r', 0, size, 0)
+
+def dropfile(self, f):
+"""
+Remove a file from the dirstate.  Returns True if the file was
+previously recorded.
+"""
+return self._map.pop(f, None) is not None
+
 def nonnormalentries(self):
 '''Compute the nonnormal dirstate entries from the dmap'''
 try:
@@ -1427,8 +1442,6 @@
 # Avoid excess attribute lookups by fast pathing certain checks
 self.__contains__ = self._map.__contains__
 self.__getitem__ = self._map.__getitem__
-self.__setitem__ = self._map.__setitem__
-self.__delitem__ = self._map.__delitem__
 self.get = self._map.get
 
 def write(self, st, now):



To: mbthomas, #hg-reviewers, durin42
Cc: durin42, mbolin, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1340: dirstate: add explicit methods for modifying dirstate

2017-11-13 Thread mbthomas (Mark Thomas) 
mbthomas updated this revision to Diff 3437.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D1340?vs=3340=3437

REVISION DETAIL
  https://phab.mercurial-scm.org/D1340

AFFECTED FILES
  mercurial/dirstate.py

CHANGE DETAILS

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -415,11 +415,11 @@
 self._map.dirs.addpath(f)
 self._dirty = True
 self._updatedfiles.add(f)
-self._map[f] = dirstatetuple(state, mode, size, mtime)
 if state != 'n' or mtime == -1:
 self._map.nonnormalset.add(f)
 if size == -2:
 self._map.otherparentset.add(f)
+self._map.addfile(f, state, mode, size, mtime)
 
 def normal(self, f):
 '''Mark a file normal and clean.'''
@@ -490,8 +490,8 @@
 elif entry[0] == 'n' and entry[2] == -2: # other parent
 size = -2
 self._map.otherparentset.add(f)
-self._map[f] = dirstatetuple('r', 0, size, 0)
 self._map.nonnormalset.add(f)
+self._map.removefile(f, size)
 if size == 0:
 self._map.copymap.pop(f, None)
 
@@ -503,10 +503,9 @@
 
 def drop(self, f):
 '''Drop a file from the dirstate'''
-if f in self._map:
+if self._map.dropfile(f):
 self._dirty = True
 self._droppath(f)
-del self._map[f]
 if f in self._map.nonnormalset:
 self._map.nonnormalset.remove(f)
 self._map.copymap.pop(f, None)
@@ -636,7 +635,7 @@
 for f in self._updatedfiles:
 e = dmap.get(f)
 if e is not None and e[0] == 'n' and e[3] == now:
-dmap[f] = dirstatetuple(e[0], e[1], e[2], -1)
+dmap.addfile(f, e[0], e[1], e[2], -1)
 self._map.nonnormalset.add(f)
 
 # emulate that all 'dirstate.normal' results are written out
@@ -1208,8 +1207,9 @@
 
 - the state map maps filenames to tuples of (state, mode, size, mtime),
   where state is a single character representing 'normal', 'added',
-  'removed', or 'merged'. It is accessed by treating the dirstate as a
-  dict.
+  'removed', or 'merged'. It is read by treating the dirstate as a
+  dict.  File state is updated by calling the `addfile`, `removefile` and
+  `dropfile` methods.
 
 - `copymap` is a dict mapping filenames to the filename they were copied
   from.
@@ -1284,22 +1284,40 @@
 def __contains__(self, key):
 return key in self._map
 
-def __setitem__(self, key, value):
-self._map[key] = value
-
 def __getitem__(self, key):
 return self._map[key]
 
-def __delitem__(self, key):
-del self._map[key]
-
 def keys(self):
 return self._map.keys()
 
 def preload(self):
 """Loads the underlying data, if it's not already loaded"""
 self._map
 
+def addfile(self, f, state, mode, size, mtime):
+"""Add a tracked file to the dirstate."""
+self._map[f] = dirstatetuple(state, mode, size, mtime)
+
+def removefile(self, f, size):
+"""
+Mark a file as removed in the dirstate.
+
+The `size` parameter is used to store sentinel values that indicate
+the file's previous state.  In the future, we should refactor this
+to be more explicit about what that state is.
+"""
+self._map[f] = dirstatetuple('r', 0, size, 0)
+
+def dropfile(self, f):
+"""
+Remove a file from the dirstate.  Returns True if the file was
+previously recorded.
+"""
+exists = f in self._map
+if exists:
+del self._map[f]
+return exists
+
 def nonnormalentries(self):
 '''Compute the nonnormal dirstate entries from the dmap'''
 try:
@@ -1429,8 +1447,6 @@
 # Avoid excess attribute lookups by fast pathing certain checks
 self.__contains__ = self._map.__contains__
 self.__getitem__ = self._map.__getitem__
-self.__setitem__ = self._map.__setitem__
-self.__delitem__ = self._map.__delitem__
 self.get = self._map.get
 
 def write(self, st, now):



To: mbthomas, #hg-reviewers, durin42
Cc: durin42, mbolin, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1340: dirstate: add explicit methods for modifying dirstate

2017-11-11 Thread mbthomas (Mark Thomas) 
mbthomas added inline comments.

INLINE COMMENTS

> mbolin wrote in dirstate.py:1275
> To avoid doing two lookups in `self._map`:
> 
>   try:
>   self._map.pop(f)
>   return True
>   except KeyError:
>   return False

Good idea.  I think `return self._map.pop(f, None) is not None` would avoid two 
lookups and also the exception.  This function's going to be getting more 
complex in later diffs, so I'd like to avoid an exception handler if possible.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1340

To: mbthomas, #hg-reviewers, durin42
Cc: durin42, mbolin, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1340: dirstate: add explicit methods for modifying dirstate

2017-11-11 Thread mbthomas (Mark Thomas) 
mbthomas added inline comments.

INLINE COMMENTS

> durin42 wrote in dirstate.py:1196
> Per my comment above, let's insert a patch before this one that documents in 
> more detail the API that dirstatemap is supposed to be providing, probably in 
> a docstring on this class. Sound reasonable?
> 
> (Then the above docstring I complained about could point to this docstring.)

Agreed.  I removed the docstring above because it became a lie (we no longer 
implement `__setitem__` or `__delitem__`, so it's not really a dict any more).  
I will add a docstring here on Monday.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1340

To: mbthomas, #hg-reviewers, durin42
Cc: durin42, mbolin, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1340: dirstate: add explicit methods for modifying dirstate

2017-11-10 Thread durin42 (Augie Fackler) 
durin42 requested changes to this revision.
durin42 added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> dirstate.py:130
>  def _map(self):
> -'''Return the dirstate contents as a map from filename to
> -(state, mode, size, time).'''
> +'''Returns the dirstate map.'''
>  self._map = dirstatemap(self._ui, self._opener, self._root)

Why is ~all the interesting content of this docstring removed? Where is the 
meaning of the dirstate map documented other than this docstring?

> dirstate.py:1196
>  
>  class dirstatemap(object):
>  def __init__(self, ui, opener, root):

Per my comment above, let's insert a patch before this one that documents in 
more detail the API that dirstatemap is supposed to be providing, probably in a 
docstring on this class. Sound reasonable?

(Then the above docstring I complained about could point to this docstring.)

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1340

To: mbthomas, #hg-reviewers, durin42
Cc: durin42, mbolin, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1340: dirstate: add explicit methods for modifying dirstate

2017-11-09 Thread mbolin (Michael Bolin) 
mbolin added inline comments.

INLINE COMMENTS

> dirstate.py:1275
> +"""
> +exists = f in self._map
> +if exists:

To avoid doing two lookups in `self._map`:

  try:
  self._map.pop(f)
  return True
  except KeyError:
  return False

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1340

To: mbthomas, #hg-reviewers
Cc: mbolin, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D1340: dirstate: add explicit methods for modifying dirstate

2017-11-08 Thread mbthomas (Mark Thomas) 
mbthomas created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Instead of assigning dirstatetuple objects to entries in the dirstate, move
  responsibility for creating tuples into the dirstatemap.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1340

AFFECTED FILES
  mercurial/dirstate.py

CHANGE DETAILS

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -127,8 +127,7 @@
 
 @propertycache
 def _map(self):
-'''Return the dirstate contents as a map from filename to
-(state, mode, size, time).'''
+'''Returns the dirstate map.'''
 self._map = dirstatemap(self._ui, self._opener, self._root)
 return self._map
 
@@ -416,11 +415,11 @@
 self._map.dirs.addpath(f)
 self._dirty = True
 self._updatedfiles.add(f)
-self._map[f] = dirstatetuple(state, mode, size, mtime)
 if state != 'n' or mtime == -1:
 self._map.nonnormalset.add(f)
 if size == -2:
 self._map.otherparentset.add(f)
+self._map.addfile(f, state, mode, size, mtime)
 
 def normal(self, f):
 '''Mark a file normal and clean.'''
@@ -491,8 +490,8 @@
 elif entry[0] == 'n' and entry[2] == -2: # other parent
 size = -2
 self._map.otherparentset.add(f)
-self._map[f] = dirstatetuple('r', 0, size, 0)
 self._map.nonnormalset.add(f)
+self._map.removefile(f, size)
 if size == 0:
 self._map.copymap.pop(f, None)
 
@@ -504,10 +503,9 @@
 
 def drop(self, f):
 '''Drop a file from the dirstate'''
-if f in self._map:
+if self._map.dropfile(f):
 self._dirty = True
 self._droppath(f)
-del self._map[f]
 if f in self._map.nonnormalset:
 self._map.nonnormalset.remove(f)
 self._map.copymap.pop(f, None)
@@ -637,7 +635,7 @@
 for f in self._updatedfiles:
 e = dmap.get(f)
 if e is not None and e[0] == 'n' and e[3] == now:
-dmap[f] = dirstatetuple(e[0], e[1], e[2], -1)
+dmap.addfile(f, e[0], e[1], e[2], -1)
 self._map.nonnormalset.add(f)
 
 # emulate that all 'dirstate.normal' results are written out
@@ -1245,22 +1243,40 @@
 def __contains__(self, key):
 return key in self._map
 
-def __setitem__(self, key, value):
-self._map[key] = value
-
 def __getitem__(self, key):
 return self._map[key]
 
-def __delitem__(self, key):
-del self._map[key]
-
 def keys(self):
 return self._map.keys()
 
 def preload(self):
 """Loads the underlying data, if it's not already loaded"""
 self._map
 
+def addfile(self, f, state, mode, size, mtime):
+"""Add a tracked file to the dirstate."""
+self._map[f] = dirstatetuple(state, mode, size, mtime)
+
+def removefile(self, f, size):
+"""
+Mark a file as removed in the dirstate.
+
+The `size` parameter is used to store sentinel values that indicate
+the file's previous state.  In the future, we should refactor this
+to be more explicit about what that state is.
+"""
+self._map[f] = dirstatetuple('r', 0, size, 0)
+
+def dropfile(self, f):
+"""
+Remove a file from the dirstate.  Returns True if the file was
+previously recorded.
+"""
+exists = f in self._map
+if exists:
+del self._map[f]
+return exists
+
 def nonnormalentries(self):
 '''Compute the nonnormal dirstate entries from the dmap'''
 try:
@@ -1390,8 +1406,6 @@
 # Avoid excess attribute lookups by fast pathing certain checks
 self.__contains__ = self._map.__contains__
 self.__getitem__ = self._map.__getitem__
-self.__setitem__ = self._map.__setitem__
-self.__delitem__ = self._map.__delitem__
 self.get = self._map.get
 
 def write(self, st, now):



To: mbthomas, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel