Patchwork [2,of,2] manifest: disallow setting the node id of an entry to None

login
register
mail settings
Submitter Augie Fackler
Date Dec. 15, 2014, 3:22 p.m.
Message ID <2e273b5a679e41066119.1418656965@arthedain.pit.corp.google.com>
Download mbox | patch
Permalink /patch/7114/
State Superseded
Headers show

Comments

Augie Fackler - Dec. 15, 2014, 3:22 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1418409644 18000
#      Fri Dec 12 13:40:44 2014 -0500
# Node ID 2e273b5a679e41066119727405963fe72baa9081
# Parent  58e93de9c73de47de64c45a1eb18d60de9604198
manifest: disallow setting the node id of an entry to None

manifest.diff() uses None as a special value to denote the absence of
a file, so setting a file node to None means you then can't trust
manifest.diff().

This should also make future manifest work slightly easier.
Martin von Zweigbergk - Dec. 15, 2014, 4:20 p.m.
On Mon Dec 15 2014 at 7:23:00 AM Augie Fackler <raf@durin42.com> wrote:

> +        assert v != None


I've been told that one should use the 'is' (or 'is not') operator when
comparing with None. Not valid in this case?
Augie Fackler - Dec. 15, 2014, 4:22 p.m.
On Mon, Dec 15, 2014 at 11:20 AM, Martin von Zweigbergk
<martinvonz@google.com> wrote:
> On Mon Dec 15 2014 at 7:23:00 AM Augie Fackler <raf@durin42.com> wrote:
>>
>> +        assert v != None
>
>
> I've been told that one should use the 'is' (or 'is not') operator when
> comparing with None. Not valid in this case?


Nope, completely valid. I blame consistent exposure to C. I'll send a
V2 shortly.

Patch

diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -17,6 +17,9 @@  class manifestdict(dict):
             flags = {}
         dict.__init__(self, mapping)
         self._flags = flags
+    def __setitem__(self, k, v):
+        assert v != None
+        dict.__setitem__(self, k, v)
     def flags(self, f):
         return self._flags.get(f, "")
     def withflags(self):