[PATCH RFC] lfs: clean up a variety of strings/bytes issues

[Matt Harbison]

On Sat, 26 Jan 2019 01:22:01 -0500, Matt Harbison <mharbison72 at gmail.com>  
wrote:

> On Sat, 26 Jan 2019 01:00:03 -0500, Matt Harbison  
> <mharbison72 at gmail.com> wrote:
>
>> On Fri, 25 Jan 2019 17:33:54 -0500, Augie Fackler <raf at durin42.com>  
>> wrote:
>>
>>> # HG changeset patch
>>> # User Augie Fackler <augie at google.com>
>>> # Date 1548455589 18000
>>> #      Fri Jan 25 17:33:09 2019 -0500
>>> # Node ID d59571aba933f16db8e58ec9b61365ea4db0e2fa
>>> # Parent  ffdac0067a147f878ac45dc2c4b3b3e490865252
>>> lfs: clean up a variety of strings/bytes issues
>>>
>>> Mostly these are around json encoding/decoding, which is a hassle for
>>> us. After this change there are fewer tracebacks in the lfs tests on
>>> Python 3, but I'm very confused by the remaining failures: it looks
>>> very much like destrepo.ui.setconfig() in mercurial.hg.clone() isn't
>>> working: I can see a repo object with a stable repr() (which includes
>>> the object's id) that appears to _lose_ the setconfig()ed
>>> paths.default by the time we get into the lfs blobstore setup.
>>
>> I don't think it is losing `paths.default`.  There's a typo in the  
>> followup hacks patch, such that it is looking up 'path.default' (not  
>> plural).  I adjusted that, and it prints the path fine in the prompt  
>> assertion.
>>
>> The blobstore is selected during reposetup(), and cached.  A local copy  
>> is recreated in the exchange.push() and prefetch() wrappers, which is  
>> happening here.  Therefore, the `if not store.has(oid)` in  
>> readfromstore() must be busted somehow.
>
> Looking more closely at the debug output, the prefetch request is asking  
> to download an empty list of objects.  You can see it if you add  
> `--debug` to `hg clone -q http://localhost:$HGPORT  
> $TESTTMP/client3_clone` around test-lfs-serve.t:236.  Therefore prefetch  
> is doing nothing, so then it wants to call out individually using the  
> original blobstore as the filelogs are accessed.

It seems like some wires got crossed with r'' vs not:

diff --git a/hgext/lfs/wireprotolfsserver.py  
b/hgext/lfs/wireprotolfsserver.py
--- a/hgext/lfs/wireprotolfsserver.py
+++ b/hgext/lfs/wireprotolfsserver.py
@@ -133,7 +133,7 @@ def _processbatchrequest(repo, req, res)
      lfsreq = json.loads(req.bodyfh.read())

      # If no transfer handlers are explicitly requested, 'basic' is  
assumed.
-    if 'basic' not in lfsreq.get('transfers', ['basic']):
+    if r'basic' not in lfsreq.get(r'transfers', [r'basic']):
          _sethttperror(res, HTTP_BAD_REQUEST,
                        b'Only the basic LFS transfer handler is supported')
          return True
@@ -149,7 +149,7 @@ def _processbatchrequest(repo, req, res)

      localstore = repo.svfs.lfslocalblobstore

-    objects = [p for p in _batchresponseobjects(req,  
lfsreq.get('objects', []),
+    objects = [p for p in _batchresponseobjects(req,  
lfsreq.get(r'objects', []),
                                                  operation, localstore)]

      rsp = {
@@ -263,9 +263,9 @@ def _batchresponseobjects(req, objects,
                  r'Accept': r'application/vnd.git-lfs'
              }

-            auth = req.headers.get(r'Authorization', r'')
-            if auth.startswith(r'Basic '):
-                hdr[r'Authorization'] = auth
+            auth = req.headers.get(b'Authorization', b'')
+            if auth.startswith(b'Basic '):
+                hdr[b'Authorization'] = auth

              return hdr

That fails close to the same spot in test-lfs-serve.t#lfsremote-on, but  
seems to be trying to upload/download blobs now.  The 500 reason is  
"TypeError: Object of type bytes is not JSON serializable" now, but it's  
way too late to debug any further.