D1313: dirstate: gate access to self._map.dirs

[pulkit (Pulkit Goyal)]

pulkit added a comment.


  Do we need this anymore or can this be abandoned? (trying to close some 
not-required-any-more differentials)

REPOSITORY
  rHG Mercurial

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

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

D1313: dirstate: gate access to self._map.dirs

[mbthomas (Mark Thomas)]

mbthomas requested changes to this revision.
mbthomas added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> dirstate.py:405
>  scmutil.checkfilename(f)
> -if f in self._map.dirs:
> -raise error.Abort(_('directory %r already in dirstate') % f)
> -# shadows
> -for d in util.finddirs(f):
> -if d in self._map.dirs:
> -break
> -entry = self._map.get(d)
> -if entry is not None and entry[0] != 'r':
> -raise error.Abort(
> -_('file %r in dirstate clashes with %r') % (d, f))
> +if "dirs" in self._map.__dict__:
> +if f in self._map.dirs:

I don't think it's safe to do this.  We're looking for clashes between files 
and directories, so we do need to look in self.dirs, even if that means 
generating it because we're accessing it for the first time.

REPOSITORY
  rHG Mercurial

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

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

D1313: dirstate: gate access to self._map.dirs

[mbolin (Michael Bolin)]

mbolin created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  `_droppath()` already checks `"dirs" in self._map.__dict__` before accessing
  `self._map.dirs`. This updates `_addpath()` to be equally cautious.

REPOSITORY
  rHG Mercurial

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

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
@@ -388,7 +388,7 @@
 return self._map.copymap
 
 def _droppath(self, f):
-if self[f] not in "?r" and "dirs" in self._map.__dict__:
+if "dirs" in self._map.__dict__ and self[f] not in "?r":
 self._map.dirs.delpath(f)
 
 if "filefoldmap" in self._map.__dict__:
@@ -402,16 +402,17 @@
 oldstate = self[f]
 if state == 'a' or oldstate == 'r':
 scmutil.checkfilename(f)
-if f in self._map.dirs:
-raise error.Abort(_('directory %r already in dirstate') % f)
-# shadows
-for d in util.finddirs(f):
-if d in self._map.dirs:
-break
-entry = self._map.get(d)
-if entry is not None and entry[0] != 'r':
-raise error.Abort(
-_('file %r in dirstate clashes with %r') % (d, f))
+if "dirs" in self._map.__dict__:
+if f in self._map.dirs:
+raise error.Abort(_('directory %r already in dirstate') % 
f)
+# shadows
+for d in util.finddirs(f):
+if d in self._map.dirs:
+break
+entry = self._map.get(d)
+if entry is not None and entry[0] != 'r':
+raise error.Abort(
+_('file %r in dirstate clashes with %r') % (d, f))
 if oldstate in "?r" and "dirs" in self._map.__dict__:
 self._map.dirs.addpath(f)
 self._dirty = True



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