Patchwork [2,of,3] lfs: use the localstore download method to transfer from remote stores

login
register
mail settings
Submitter Matt Harbison
Date Jan. 7, 2018, 7:22 a.m.
Message ID <5da1b3ea5aef5e4541f0.1515309754@Envy>
Download mbox | patch
Permalink /patch/26602/
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 1513909200 18000
#      Thu Dec 21 21:20:00 2017 -0500
# Node ID 5da1b3ea5aef5e4541f00b5dd75a97d471d39dd5
# Parent  32b06a5033f350fe73eaac8d27fd153df302257d
lfs: use the localstore download method to transfer from remote stores

Both gitlfsremote and file based remotes benefit from not requiring the whole
file in memory (though the whole file is still loaded when passing through the
revlog interface).  With a method specific to downloading from a remote store,
the misleading 'use hg verify' hint is removed.  The behavior is otherwise
unchanged, in that a download from both remote store types will yield a copy of
the blob via util.atomictempfile.

There's no response payload defined for the non 'download' actions, but the
previous code attempted to read the payload in this case anyway.  This
refactored code made that more obvious, so any payload is printed as a debug
message, just in case.

Patch

diff --git a/hgext/lfs/blobstore.py b/hgext/lfs/blobstore.py
--- a/hgext/lfs/blobstore.py
+++ b/hgext/lfs/blobstore.py
@@ -299,19 +299,21 @@ 
         response = b''
         try:
             req = self.urlopener.open(request)
-            while True:
-                data = req.read(1048576)
-                if not data:
-                    break
-                response += data
+            if action == 'download':
+                # If downloading blobs, store downloaded data to local blobstore
+                localstore.download(oid, req)
+            else:
+                while True:
+                    data = req.read(1048576)
+                    if not data:
+                        break
+                    response += data
+                if response:
+                    self.ui.debug('lfs %s response: %s' % (action, response))
         except util.urlerr.httperror as ex:
             raise LfsRemoteError(_('HTTP error: %s (oid=%s, action=%s)')
                                  % (ex, oid, action))
 
-        if action == 'download':
-            # If downloading blobs, store downloaded data to local blobstore
-            localstore.write(oid, response, verify=True)
-
     def _batch(self, pointers, localstore, action):
         if action not in ['upload', 'download']:
             raise error.ProgrammingError('invalid Git-LFS action: %s' % action)
@@ -385,8 +387,8 @@ 
 
     def readbatch(self, pointers, tostore):
         for p in pointers:
-            content = self.vfs.read(p.oid())
-            tostore.write(p.oid(), content, verify=True)
+            with self.vfs(p.oid(), 'rb') as fp:
+                tostore.download(p.oid(), fp)
 
 class _nullremote(object):
     """Null store storing blobs to /dev/null."""
diff --git a/tests/test-lfs-test-server.t b/tests/test-lfs-test-server.t
--- a/tests/test-lfs-test-server.t
+++ b/tests/test-lfs-test-server.t
@@ -119,7 +119,6 @@ 
   $ rm ../repo1/.hg/store/lfs/objects/d1/1e1a642b60813aee592094109b406089b8dff4cb157157f753418ec7857998
   $ rm ../repo1/*
 
-XXX: suggesting `hg verify` won't help with a corrupt file on the lfs server.
   $ hg --repo ../repo1 update -C tip -v
   resolving manifests
   getting a
@@ -128,8 +127,7 @@ 
   lfs: found 31cf46fbc4ecd458a0943c5b4881f1f5a6dd36c53d6167d5b69ac45149b38e5b in the local lfs store
   getting c
   lfs: downloading d11e1a642b60813aee592094109b406089b8dff4cb157157f753418ec7857998 (19 bytes)
-  abort: detected corrupt lfs object: d11e1a642b60813aee592094109b406089b8dff4cb157157f753418ec7857998
-  (run hg verify)
+  abort: corrupt remote lfs object: d11e1a642b60813aee592094109b406089b8dff4cb157157f753418ec7857998
   [255]
 
 The corrupted blob is not added to the usercache or local store
diff --git a/tests/test-lfs.t b/tests/test-lfs.t
--- a/tests/test-lfs.t
+++ b/tests/test-lfs.t
@@ -714,8 +714,7 @@ 
   updating to branch default
   resolving manifests
   getting l
-  abort: detected corrupt lfs object: 22f66a3fc0b9bf3f012c814303995ec07099b3a9ce02a7af84b5970811074a3b
-  (run hg verify)
+  abort: corrupt remote lfs object: 22f66a3fc0b9bf3f012c814303995ec07099b3a9ce02a7af84b5970811074a3b
   [255]
 
 A corrupted lfs blob is not transferred from a file://remotestore to the