This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7785

remotefilelog: actually fix (and test this time) a bytes vs str issue
ClosedPublic

Authored by spectral on Jan 2 2020, 10:56 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHGfaf00e30b63f: remotefilelog: actually fix (and test this time) a bytes vs str issue
Summary

I attempted this in 94670e124d29 but we didn't actually have any tests for this
function it seems, and I mistook "it works now" for it being fixed (when what
had actually happened was that my cache had populated enough that I didn't
trigger the minimum threshold on my reattempts).

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

spectral created this revision.Jan 2 2020, 10:56 PM
martinvonz added a subscriber: martinvonz.Jan 3 2020, 1:19 AM

D7729 was my attempt at fixing it. Your patch is clearly better since it includes a test. I'll abandon mine.

spectral added a commit: rHGfaf00e30b63f: remotefilelog: actually fix (and test this time) a bytes vs str issue.Jan 7 2020, 10:26 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGfaf00e30b63f: remotefilelog: actually fix (and test this time) a bytes vs str issue (authored by spectral). · Explain Why
This revision was automatically updated to reflect the committed changes.
pulkit added a subscriber: pulkit.Jan 8 2020, 6:59 AM
pulkit added inline comments.
tests/test-remotefilelog-prefetch.t
253

test-check-code.t fails now:

--- /home/pulkitg/repo/hgpush/tests/test-check-code.t
+++ /home/pulkitg/repo/hgpush/tests/test-check-code.t.err
@@ -32,6 +32,10 @@
   Skipping i18n/polib.py it has no-che?k-code (glob)
   Skipping mercurial/statprof.py it has no-che?k-code (glob)
   Skipping tests/badserverext.py it has no-che?k-code (glob)
+  tests/test-remotefilelog-prefetch.t:253:
+   >         except:
+   naked except clause
+  [1]
 
 @commands in debugcommands.py should be in alphabetical order.
 

ERROR: test-check-code.t output changed

Can you send a followup?

Revision Contents
Changeset List

Diff 19062

hgext/remotefilelog/fileserverclient.py

assert all(store.has(p.oid()) for p in pointers) assert all(store.has(p.oid()) for p in pointers)
def logstacktrace(self): def logstacktrace(self):
import traceback import traceback
self.ui.log( self.ui.log(
b'remotefilelog', b'remotefilelog',
b'excess remotefilelog fetching:\n%s\n', b'excess remotefilelog fetching:\n%s\n',
b''.join(pycompat.sysbytes(traceback.format_stack())), b''.join(pycompat.sysbytes(s) for s in traceback.format_stack()),
) )

tests/test-remotefilelog-prefetch.t

[1] [1]
$ hg mv z2 z3 $ hg mv z2 z3
z2: not copying - file is not managed z2: not copying - file is not managed
abort: no files to copy abort: no files to copy
[255] [255]
$ hg revert -a -r 1 || true $ hg revert -a -r 1 || true
3 files fetched over 1 fetches - (3 misses, 0.00% hit ratio) over * (glob) 3 files fetched over 1 fetches - (3 misses, 0.00% hit ratio) over * (glob)
abort: z2@109c3a557a73: not found in manifest! (?) abort: z2@109c3a557a73: not found in manifest! (?)
# warning when we have excess remotefilelog fetching
$ cat > repeated_fetch.py << EOF
> import binascii
> from mercurial import extensions, registrar
> cmdtable = {}
> command = registrar.command(cmdtable)
> @command(b'repeated-fetch', [], b'', inferrepo=True)
> def repeated_fetch(ui, repo, *args, **opts):
> for i in range(20):
> try:
> hexid = (b'%02x' % (i + 1)) * 20
> repo.fileservice.prefetch([(b'somefile.txt', hexid)])
> except:
pulkitUnsubmitted
Not Done

test-check-code.t fails now:

--- /home/pulkitg/repo/hgpush/tests/test-check-code.t
+++ /home/pulkitg/repo/hgpush/tests/test-check-code.t.err
@@ -32,6 +32,10 @@
   Skipping i18n/polib.py it has no-che?k-code (glob)
   Skipping mercurial/statprof.py it has no-che?k-code (glob)
   Skipping tests/badserverext.py it has no-che?k-code (glob)
+  tests/test-remotefilelog-prefetch.t:253:
+   >         except:
+   naked except clause
+  [1]
 
 @commands in debugcommands.py should be in alphabetical order.
 

ERROR: test-check-code.t output changed

Can you send a followup?

pulkit: `test-check-code.t` fails now: ``` --- /home/pulkitg/repo/hgpush/tests/test-check-code.t +++…
> pass
> EOF
We should only output to the user once. We're ignoring most of the output
because we're not actually fetching anything real here, all the hashes are
bogus, so it's just going to be errors and a final summary of all the misses.
$ hg --config extensions.repeated_fetch=repeated_fetch.py \
> --config remotefilelog.fetchwarning="fetch warning!" \
> --config extensions.blackbox= \
> repeated-fetch 2>&1 | grep 'fetch warning'
fetch warning!
We should output to blackbox three times, with a stack trace on each (though
that isn't tested here).
$ grep 'excess remotefilelog fetching' .hg/blackbox.log
.* excess remotefilelog fetching: (re)
.* excess remotefilelog fetching: (re)
.* excess remotefilelog fetching: (re)