Patchwork [2,of,2] lfs: improve the error message for a missing remote blob

login
register
mail settings
Submitter Matt Harbison
Date Jan. 8, 2018, 4:28 a.m.
Message ID <5dd81792f99292985a10.1515385718@Envy>
Download mbox | patch
Permalink /patch/26605/
State Accepted, archived
Headers show

Comments

Matt Harbison - Jan. 8, 2018, 4:28 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1515356476 18000
#      Sun Jan 07 15:21:16 2018 -0500
# Node ID 5dd81792f99292985a10a04f49217b1ee97ced25
# Parent  e9185c88cf15324fc5e9b590dbb415f07f6d4f18
lfs: improve the error message for a missing remote blob

It seems better to print the name known to the user, not the internal file.  The
previous code unconditionally set 'p.filename'.  That potentially made the
attribute None, and would be printed as such in
_gitlfsremote._checkforservererror() instead of "unknown".  Normally files are
printed relative to CWD, but I don't see a way to get the repo path to make that
adjustment.

The test modified here apparently only runs within Facebook, but a print
statement confirmed the name change.  I tried uploading the blob to a different
remote store (so the git server never saw it), and also killing the git server
and removing the blob directory, and removing the 'lfs.db' file.  All resulted
in a message:

  abort: LFS server claims required objects do not exist:
  bdc26931acfb734b142a8d675f205becf27560dc461f501822de13274fe6fc8a!

Patch

diff --git a/hgext/lfs/blobstore.py b/hgext/lfs/blobstore.py
--- a/hgext/lfs/blobstore.py
+++ b/hgext/lfs/blobstore.py
@@ -240,7 +240,7 @@ 
                     filename = getattr(p, 'filename', 'unknown')
                     raise LfsRemoteError(
                         _(('LFS server error. Remote object '
-                          'for file %s not found: %r')) % (filename, response))
+                          'for "%s" not found: %r')) % (filename, response))
                 raise LfsRemoteError(_('LFS server error: %r') % response)
 
     def _extractobjects(self, response, pointers, action):
diff --git a/hgext/lfs/wrapper.py b/hgext/lfs/wrapper.py
--- a/hgext/lfs/wrapper.py
+++ b/hgext/lfs/wrapper.py
@@ -60,7 +60,7 @@ 
     oid = p.oid()
     store = self.opener.lfslocalblobstore
     if not store.has(oid):
-        p.filename = getattr(self, 'indexfile', None)
+        p.filename = self.filename
         self.opener.lfsremoteblobstore.readbatch([p], store)
 
     # The caller will validate the content
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
@@ -185,7 +185,7 @@ 
   $ rm -rf `hg config lfs.usercache`
   $ hg --config 'lfs.url=https://dewey-lfs.vip.facebook.com/lfs' clone test test2
   updating to branch default
-  abort: LFS server error. Remote object for file data/a.i not found:(.*)! (re)
+  abort: LFS server error. Remote object for "a" not found:(.*)! (re)
   [255]
 
   $ $PYTHON $RUNTESTDIR/killdaemons.py $DAEMON_PIDS