Patchwork [1,of,3,V2] lfs: allow a pointer to be extracted from a context that removes the file

mail settings
Submitter Matt Harbison
Date Feb. 11, 2018, 2:41 a.m.
Message ID <b1c1762fe8b9bb0c8960.1518316887@Envy>
Download mbox | patch
Permalink /patch/27520/
State Accepted
Headers show


Matt Harbison - Feb. 11, 2018, 2:41 a.m.
# HG changeset patch
# User Matt Harbison <>
# Date 1517097384 18000
#      Sat Jan 27 18:56:24 2018 -0500
# Node ID b1c1762fe8b9bb0c89602810e2db8b1e5e63b8c9
# Parent  de0666564bde7c67c4d40a44dcf50515fef2f367
lfs: allow a pointer to be extracted from a context that removes the file

This is needed to let 'set:lfs()' and '{lfs_files}' work normally on removed

Yuya suggested returning a null pointer for removed files, instead of the
pointer from the parent.  The first attempt at this was to return None for a non
LFS file, and a (pointer, ctx) tuple to hold the pointer and context (or parent
pointer and context for a removed file).  But this complicated the callers, even
the ones that didn't care about removed files.

Instead, let's use {} to represent a removed pointer.  This has the added
convenience of being a useful representation in the template language, and only
affects the callers that care about removed files (and only slightly).  Since
pointers are explicitly serialized with a call to a member function, there is no
danger of writing these to disk.


diff --git a/hgext/lfs/ b/hgext/lfs/
--- a/hgext/lfs/
+++ b/hgext/lfs/
@@ -349,26 +349,45 @@ 
             pointers[p.oid()] = p
     return sorted(pointers.values())
-def pointerfromctx(ctx, f):
+def pointerfromctx(ctx, f, removed=False):
     """return a pointer for the named file from the given changectx, or None if
-    the file isn't LFS."""
+    the file isn't LFS.
+    Optionally, the pointer for a file deleted from the context can be returned.
+    Since no such pointer is actually stored, and to distinguish from a non LFS
+    file, this pointer is represented by an empty dict.
+    """
+    _ctx = ctx
     if f not in ctx:
-        return None
-    fctx = ctx[f]
+        if not removed:
+            return None
+        if f in ctx.p1():
+            _ctx = ctx.p1()
+        elif f in ctx.p2():
+            _ctx = ctx.p2()
+        else:
+            return None
+    fctx = _ctx[f]
     if not _islfs(fctx.filelog(), fctx.filenode()):
         return None
-        return pointer.deserialize(fctx.rawdata())
+        p = pointer.deserialize(fctx.rawdata())
+        if ctx == _ctx:
+            return p
+        return {}
     except pointer.InvalidPointer as ex:
         raise error.Abort(_('lfs: corrupted pointer (%s@%s): %s\n')
-                          % (f, short(ctx.node()), ex))
+                          % (f, short(_ctx.node()), ex))
-def pointersfromctx(ctx):
-    """return a dict {path: pointer} for given single changectx"""
+def pointersfromctx(ctx, removed=False):
+    """return a dict {path: pointer} for given single changectx.
+    If ``removed`` == True and the LFS file was removed from ``ctx``, the value
+    stored for the path is an empty dict."""
     result = {}
     for f in ctx.files():
-        p = pointerfromctx(ctx, f)
-        if p:
+        p = pointerfromctx(ctx, f, removed=removed)
+        if p is not None:
             result[f] = p
     return result