Patchwork [3,of,3] lfs: remove the verification option when writing to the local store

login
register
mail settings
Submitter Matt Harbison
Date Jan. 7, 2018, 7:22 a.m.
Message ID <b58ace49e2be02d47577.1515309755@Envy>
Download mbox | patch
Permalink /patch/26603/
State Accepted
Headers show

Comments

Matt Harbison - Jan. 7, 2018, 7:22 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1515305692 18000
#      Sun Jan 07 01:14:52 2018 -0500
# Node ID b58ace49e2be02d475772dda828398fa15a518c9
# Parent  5da1b3ea5aef5e4541f00b5dd75a97d471d39dd5
lfs: remove the verification option when writing to the local store

This partially reverts 417e8e040102 and bb6a80fc969a.  But since there's now a
dedicated download function, there's no functional change.  The last sentence in
the commit message of the latter is wrong- write() didn't need the one time hash
check if verification wasn't requested.  I suspect I missed 'read()' in there
("... but _read()_ also needs to do a one time check..."), because that did fail
without the hash check before linking to the usercache.  The write() method
simply took the same check for consistency.

While here, clarify that the write() method is *only* for storing content
directly from filelog, which has already checked the hash.

If someone can come up with a way to bridge the differences between writing to a
file and sending a urlreq.request across the wire, we can create an upload()
function and cleanup read() in a similar way.  About the only common thread I
see is an open() that verifies the content before returning a file descriptor.

Patch

diff --git a/hgext/lfs/blobstore.py b/hgext/lfs/blobstore.py
--- a/hgext/lfs/blobstore.py
+++ b/hgext/lfs/blobstore.py
@@ -134,20 +134,20 @@ 
             self.ui.note(_('lfs: adding %s to the usercache\n') % oid)
             lfutil.link(self.vfs.join(oid), self.cachevfs.join(oid))
 
-    def write(self, oid, data, verify=True):
-        """Write blob to local blobstore."""
-        if verify:
-            _verify(oid, data)
+    def write(self, oid, data):
+        """Write blob to local blobstore.
 
+        This should only be called from the filelog during a commit or similar.
+        As such, there is no need to verify the data.  Imports from a remote
+        store must use ``download()`` instead."""
         with self.vfs(oid, 'wb', atomictemp=True) as fp:
             fp.write(data)
 
         # XXX: should we verify the content of the cache, and hardlink back to
         # the local store on success, but truncate, write and link on failure?
         if not self.cachevfs.exists(oid):
-            if verify or hashlib.sha256(data).hexdigest() == oid:
-                self.ui.note(_('lfs: adding %s to the usercache\n') % oid)
-                lfutil.link(self.vfs.join(oid), self.cachevfs.join(oid))
+            self.ui.note(_('lfs: adding %s to the usercache\n') % oid)
+            lfutil.link(self.vfs.join(oid), self.cachevfs.join(oid))
 
     def read(self, oid, verify=True):
         """Read blob from local blobstore."""
diff --git a/hgext/lfs/wrapper.py b/hgext/lfs/wrapper.py
--- a/hgext/lfs/wrapper.py
+++ b/hgext/lfs/wrapper.py
@@ -86,7 +86,7 @@ 
 
     # git-lfs only supports sha256
     oid = hashlib.sha256(text).hexdigest()
-    self.opener.lfslocalblobstore.write(oid, text, verify=False)
+    self.opener.lfslocalblobstore.write(oid, text)
 
     # replace contents with metadata
     longoid = 'sha256:%s' % oid