Summary: This preserves the `--noninteractive` flag used by some tools.
Reviewed By: DurhamG
Differential Revision: D24040789
fbshipit-source-id: 8d50f3f3ce6b2015f0ef6c3bd1b4fbb874d0ea7d
Summary:
This restores the compatibility of setting up merge tools using the `ui.merge`
config while still limiting the default `editmerge` tool to interactive
sessions.
Reviewed By: sfilipco
Differential Revision: D24377259
fbshipit-source-id: 3d2befba412b824fc985ddffa131e339644178c2
Summary: Make it testable by allowing specifying paths to load as user hgrc.
Reviewed By: sfilipco
Differential Revision: D24377258
fbshipit-source-id: 969028df64d55ad1f1304e35675d84595ed6a2bf
Summary:
Include a `User-Agent` header in EdenAPI requests from Mercurial. This will allow us to see the version in Scuba, and in the future, will allow us to distinguish between requests send by Mercurial and those sent directly by EdenFS.
Keeping with the current output of `hg version`, the application is specified as "EdenSCM" rather than "Mercurial".
Reviewed By: singhsrb
Differential Revision: D24347021
fbshipit-source-id: e323cfc945c9d95d8b2a0490e22c2b2505a620dc
Summary: This was competely undocumented, and thus undiscoverable. Add some documentation, so that it's at least clear the feature exists
Reviewed By: singhsrb
Differential Revision: D24332961
fbshipit-source-id: 9e73163a9314ceb7f953a3b1ac0f58c9a6e6d4d9
Summary:
The hide-before config was added to encourage people to actively hide unused
drafts for repo performance, instead of keeping unused draft forever, since a
lot of code paths assume `len(draft)` is small. See D13993584 (28b4dfbb38) for more context.
Now our hide-before data is set to 2.5 years ago (2018-2-25), this change
probably only affects a very small number of users.
Reviewed By: DurhamG
Differential Revision: D24298198
fbshipit-source-id: 938aca1222b55e09fdb058ff01bc063733f201dc
Summary:
Rust tests run in multiple threads. Setting environment variables affects other
tests running in other threads and causes random test failures.
Protect env vars using a lock.
Reviewed By: DurhamG
Differential Revision: D24296639
fbshipit-source-id: db0bee85625a7b63e07b95ea76d96029487881d4
Summary:
The shell-script cargo tests seem very flaky. Use a dedicated Python script to
run the tests, with a more concise output that only includes failures, and run
tests in parallel.
Reviewed By: DurhamG
Differential Revision: D24296433
fbshipit-source-id: 1d63146c6c84f1035dded24fcd3d79f116c2e740
Summary:
Ideally we'd just delete the p4 convert functionality, but I'm too lazy
to go through and extract it right now.
This was recently enabled when I enabled all the convert tests. We don't use the p4 logic, so it's safe to just turn back off to get a release out.
Reviewed By: quark-zju
Differential Revision: D24352068
fbshipit-source-id: 6f3a1f88739b2e2348aff00e8cae333473bbe71a
Summary: My recent change accidentally returned early when reading the prompt input, which skipped the \r truncation needed for Windows.
Reviewed By: sfilipco
Differential Revision: D24350672
fbshipit-source-id: 4a589d76bf41cda7fda2518003ef272f9a6ead48
Summary:
We're seeing slow pack fetches in some cases. Let's add some extra
debug output to get more information.
Reviewed By: quark-zju
Differential Revision: D24295593
fbshipit-source-id: b5a5bdf169a8c05a3143da09d69646a7a742ef08
Summary:
We're seeing history fetching take quite a while during checkout and
rebase, but it's not really necessary for a checkout. In addition to it being
slow, if memcache doesn't have the history entry we'll fall back to a full
revision fetch from the server, which includes data. Let's disable prefetching
history during checkout.
Reviewed By: quark-zju
Differential Revision: D24295594
fbshipit-source-id: 70aa6e1925074b6546626a5192a7562d6da31f2b
Summary:
My recent diff changed these lines from `bytes(value)` to
`pycompat.decodeutf8(value)`, since we want these to be strings. Unfortunately,
on python 2 decodeutf8() just passes the value straight through, so in cases
where we're handed bytearray, we pass that through instead of converting it to
bytes. Some down stream consumers requires it to be bytes.
Let's conditionally turn it to bytes.
Reviewed By: krallin
Differential Revision: D24307818
fbshipit-source-id: a0cc64b7e2cf7645586e633e7a4a382b69390e15
Summary:
When the server returns a 429, the intention is that the client will wait for a
little bit then try again later (there is no harm in that, as we haven't really
used many server resources for this). However, it turned out that right now we
just abort. Let's fix it!
Note that this changes the behavior a bit for the error cases, in the sense
that we no longer return `Ok(None)` but instead return an `Err`. Xavier noted
this should make sense here.
I've also had the client send its retry attempt via a header, because who
knows, that might be useful.
Reviewed By: kulshrax
Differential Revision: D24308127
fbshipit-source-id: 35639956f36342dfb0056b0d348dc4ad56bd576c
Summary:
This is Ruby's `iter.each_slice(n)` or Rust's `slice.chunks(n)`.
It will be used by phabstatus to batch requests.
Reviewed By: singhsrb
Differential Revision: D24289050
fbshipit-source-id: 528b35d43874222f1b6783738e965d9a3b159deb
Summary: Make it easier to iterate contexes instead of revs.
Reviewed By: simpkins
Differential Revision: D24289053
fbshipit-source-id: deffdc4a5d4e7e3644a98f8112f5c8b8bdbd154d
Summary:
Previously `--master` falls back to `null` if `-r` is provided for
compatibility. That could be surprising as smartlog's sorting does not work
well without a master branch.
Change `--master` to a consistent value regardless of `-r` is provided or not.
Reviewed By: simpkins
Differential Revision: D24297586
fbshipit-source-id: c6e695333ae17db11dcb98d9cdb57cc2f298ff64
Summary: Introduces fetching of child entry IDs, and child file metadata for a specified tree manifest ID. The aux data lookup will only be performed if `with_file_metadata` is set, which is actually kind of wrong. Instead `with_children` from the wire type should be exposed in the API request type, and `with_*_metadata` should be hidden or used for data other than the child entry `Key`s.
Reviewed By: kulshrax
Differential Revision: D23886678
fbshipit-source-id: 0cba72cea7be47ae3348a406d407a19b60976c0c
Summary: Some of these tests were already passing. Some required minor fixes to pass.
Reviewed By: quark-zju
Differential Revision: D24290635
fbshipit-source-id: 789d3033142466c868057da8f9f23d27aede8ea4
Summary:
They now pass. Didn't test the p4 tests since I don't have p4
available, but I don't think we use it anymore.
Reviewed By: quark-zju
Differential Revision: D24284643
fbshipit-source-id: 184697052c0b7000ad9765b4033964b4d382b811
Summary:
Template filters are expecting the output of the template to be a unicode
string, not a byte string. Thus we need to ensure that diff return a unicode
string.
Reviewed By: DurhamG
Differential Revision: D24290908
fbshipit-source-id: 70899249c3e26edd1714fd901fa6af405b61e544
Summary:
This reverts D24066918 (a684e53a31). That diff changed the phabstatus code to use the last
created revset iterator rather than the first for peekahead purposes.
The first revset iterator does appear to be the correct one to wrap. After
D24066918 (a684e53a31) I consistently see `hg log -r 'draft()' -T'{phabstatus}\n'` make
Phabricator queries for only one diff at a time. Reverting the changes from
that diff fixes the issue.
Note that the revset argument passed to `hg log` appears to make a difference
in the behavior here. When using simple revsets including just specific
commit ranges then the peekahead behavior works as expected both before and
after D24066918 (a684e53a31). However, when using `draft()` in the revset specifier then
the behavior works before D24066918 (a684e53a31), but not after it.
Reviewed By: singhsrb
Differential Revision: D24269910
fbshipit-source-id: c2c2abf6360fe3129a1fc73b5b433b763bce1fd2
Summary:
The infinitepush sqlindex was returning bytes instead of strings, which
caused various formatters to serialize as "b'sadf'" instead of "sadf". Let's fix
the type converter to produce strings on python 3. Note, this is the opposite to
what we do in the hgsql converter. hgsql works with binary data, so we default
to bytes there. The sqlindex doesn't deal with any binary data, so defaulting to
strings is better.
Also updates a comment on the hgsql converter to be accurate. I confirmed there
are no other customer converters in the code base that need updating.
Reviewed By: krallin
Differential Revision: D24282899
fbshipit-source-id: 708732c17626f6ebc2c3cbe7f00afa5771f31d7b
Summary:
Include the client correlator string from the `clienttelemetry` extension in each EdenAPI HTTP request via the `X-Client-Correlator` header.
The `ClientIdentityMiddleware` in `gotham_ext` already understands this header (as it is already used by the LFS server), and `gotham_ext`'s `ScubaMiddleware` will automatically include the provided correlator in the server's Scuba samples.
Reviewed By: farnz
Differential Revision: D24282244
fbshipit-source-id: 13d04e706eda38893cff6e740bd1d7bf104e43dd
Summary:
The logic inside `eden/scm/tests/features.py` script enables certain
features of HG on tests based on the name of the test. Mononoke's integration
tests suit reuses eden/scm's tests suit and as a consequence it triggers the
same `features.py` logic. It was fine until D24201934 (f5988c415c) introduced a feature for
test that is named in the same way as one of the Mononoke tests - the
`test-clienttelemetry.t`.
In order to fix this problem Mononoke will pass a `--nofeatures` flag to the
`eden/scm/run-tests.py` invocations that will turn off the usage of
`features.py`.
Reviewed By: farnz
Differential Revision: D24276294
fbshipit-source-id: eb28ed55a05de3b012e37407603c2370adaaad16
Summary:
If Phabricator takes more than 10 seconds to respond, a `socket.timeout`
exception may be thrown. Treat this like other networking errors, and simply
report the diff phabricator status as `Error`. Previously this exception was
unhandled, causing the entire command to abort.
Reviewed By: singhsrb
Differential Revision: D24272274
fbshipit-source-id: f646d111a91f901e09d9f94a1e0102d6dd4d0952
Summary:
This change introduces two new metadata types, Category and Transience, and a mechanism for Category to provide a default Fault and Transience, which can be overriden by the user.
Also introduces a mechanism for attempting to log exceptions which occur during exception logging, falling back to the previous behavior of just swallowing the exception on failure.
Reviewed By: DurhamG
Differential Revision: D22677565
fbshipit-source-id: 1cf75ca1e2a65964a0ede1f072439378a46bd391
Summary:
It only has benchmark code that led to the use of mincode. Now hgcommits is the
main crate for commit storage. `commitstore` without `hg` in its name was
initially planned to support other kinds of commits including git and bonsai.
However we don't have immediate goal for that at present. So let's just remove
the commitstore directory.
Reviewed By: singhsrb
Differential Revision: D24263618
fbshipit-source-id: 84b4861ae490817377e69d8c2006c63331e3db1f
Summary:
On the hg servers we're seeing crashes due to signals during syscalls.
Rolling back to prior to the signal changes seems to have fixed it, though we
haven't bisected enough to be sure this is the cause.
The ui.threaded option is already used to opt-out of running on a background
thread, let's also use it to opt-out of the custom signal registering in hopes
that it fixes the server issue, until they can be deprecated.
Reviewed By: quark-zju
Differential Revision: D24254804
fbshipit-source-id: 50e4fc8c7e3a88b5baa49394f6f1edffc946187d
Summary: Need to add new quickcheck tests, verify that remove `Serialize` from `TreeEntry` is okay.
Reviewed By: kulshrax
Differential Revision: D23457777
fbshipit-source-id: aa94ed7aa81b41924eba4a8bd1bdc2c737365b77
Summary:
Change
abort: repository repo: timed out waiting for lock held by <lockinfo ...>
to:
abort: repository repo: timed out waiting for lock held by process '842210' on host 'hostname'
Reviewed By: singhsrb
Differential Revision: D24214462
fbshipit-source-id: 65056ebb9764651b2f0126061fafdfdefaa4e9c6
Summary: The rev numbers are almost gone, remove them from the test.
Reviewed By: sfilipco
Differential Revision: D24255156
fbshipit-source-id: 5cbc4a71c2d3f773c5b474d1edce84ceceb50bf9
Summary:
This will allow Eden to control what data is in Mercurial's memory and
what is on disk. This will let it persist the hg_import_helper process longer,
and prevent slow startup times when needing to restart it.
Reviewed By: xavierd
Differential Revision: D24231131
fbshipit-source-id: a4f743740b44957e8d2dd93f07e9831eadfee7ab
Summary:
This was added in D16078908 (1bc6ecf8fe). It turns out fsync followed by a hardlink
on btrfs results in very slow hardlink performance (100-600ms). Since fewer and
fewer of our files use this atomic write code path and since this affects almost
every hg write command, let's roll this back.
This will increase the chance of data loss during a hard reboot, but commit
cloud is good enough to allow recovering from that in almost every situation.
Reviewed By: sfilipco
Differential Revision: D24230056
fbshipit-source-id: aae1a5612eda5f62bb5ec1442b1312ed45c42786
Summary:
With invalidatelinkrev, linkrev stored in revlog shouldn't be used.
This makes some tests pass with segmented changelog.
Reviewed By: singhsrb
Differential Revision: D24201944
fbshipit-source-id: 6473c30266c79aa97a955b1a6c867411cc67de2b
Summary:
The old code assumes `0..len(repo)` are valid revs, which is no longer true
with segmented changelog.
Reviewed By: singhsrb
Differential Revision: D24201948
fbshipit-source-id: b882a215701c57ccdf4af340c889586b040772da
Summary:
Copy segmented changelog, metalog and mutation store for local clones.
This mainly affects tests.
Reviewed By: singhsrb
Differential Revision: D24201941
fbshipit-source-id: c60da9e2bf982a6f66004415e45178749157745e
Summary: This is needed to move our hg servers to python 3.
Reviewed By: quark-zju
Differential Revision: D24204056
fbshipit-source-id: cbaf97893f8f77b535952ac290766f0fd5e14f0c
Summary: Do not show revnum or "changeset" after commit (with --debug or --vrebose).
Reviewed By: singhsrb
Differential Revision: D24201942
fbshipit-source-id: 2f0d15711df67070e50d4bf30f0b1b4401d85524
Summary:
With segmented changelog, linkrev can exceed i32 range and cause "pack" (aka.
bundle) to fail. Workaround it by packing nullrev instead.
~30 tests now pass with segmented changelog with this change.
Reviewed By: singhsrb
Differential Revision: D24201940
fbshipit-source-id: 5f27c185837cd3c1fbe9b65d21ef3cd641eec2e5
Summary:
debugbuilddag assumes revs created start from 0, 1, 2..., which is
no longer true with segmented changelog.
Change it to resolve revs using the local `nodeids` array so it's
compatible with segmented changelog.
Reviewed By: singhsrb
Differential Revision: D24191790
fbshipit-source-id: ca7d1cccbba664128c227d66071b166c799cdf49
Summary:
A previous change updated all tests to remove the use of rev numbers. The
update to the list of results missed this tests case.
Reviewed By: quark-zju
Differential Revision: D24208236
fbshipit-source-id: 289136f31e66eb74b106d7ea9401419fc369d59f
Summary:
Print a message for each EdenAPI method call to stderr if the user has `edenapi.debug` set.
These messages are already logged to `tracing`, but also printing them out when `edenapi.debug` is set makes the debug output more useful, since it provides context for the download stats. This is especially useful when reading through EdenFS logs.
Reviewed By: quark-zju
Differential Revision: D24204381
fbshipit-source-id: 37b47eed8b89438cdf510443e917a5c8660eb43b
Summary: Use a `HashMap` to store user-specified additional HTTP headers. This allows headers to be set in multiple places (whereas previously, setting new headers would replace all previously set headers).
Reviewed By: quark-zju
Differential Revision: D24200833
fbshipit-source-id: 93147cf334a849c4d2fc4f29849018a4c7565143
Summary: The panics can happen when the input sets are out of range.
Reviewed By: kulshrax
Differential Revision: D24191789
fbshipit-source-id: efbcbd7f6f69bd262aa979afa4f44acf9681d11e
Summary: Change fold and metaedit to not show revnum in editor message.
Reviewed By: kulshrax
Differential Revision: D24191787
fbshipit-source-id: 140ec58c8eb00c067c6e40e1a18187f7801246e9
Summary:
Some sort of serialization for the Dag is useful for saving the IdDag produced
by offline jobs load that when a mononoke server starts.
Reviewed By: quark-zju
Differential Revision: D24096964
fbshipit-source-id: 5fac40f9c10a5815fbf5dc5e2d9855cd7ec88973
Summary:
We're seeing an issue on the hg servers where the filecache assertion
that if a value is in obj.__dict__ it's also in obj._filecache is broken. This
occurred about 10% of the time in sandcastle jobs. The diff that caused this
went in in April (D21148446 (73c5cb89de)), so it's unclear why it's only cropping up now.
This is caused by the following steps:
1. repo._bookmarks is accessed while _bookmarks is in the _filecache but not in
the __dict__
2. This causes construction of _bookmarks, before it can set it to __dict__
3. Construction of _bookmarks calls repo.invalidate(clearfilecache=True), which
deletes _bookmarks from _filecache.
4. _bookmarks construction completes, and gets set to __dict__ (but now it's
missing from _filecache, so the invariant will fail next time someone checks).
5. Someone accesses _bookmarks later, and the assertion fires.
The fix is to just not clear the filecache during bookmark construction. The
main purpose of this invalidate was to let the changelog be reloaded, and I
think that will still happen since, if there are any new commits in the
changelog, the file size and time will change, triggering a reload next time the
_filecache entry is checked.
Reviewed By: quark-zju
Differential Revision: D24182914
fbshipit-source-id: fb49137e28d9224c6617d9c84faaf2f9de363aaf
Summary:
Our stdin/stdout bytes/str manipulations caused input() to print
warnings about buffered not being supported. The only reason we do those
manipulations to handle the case in tests where the prompt answer doesn't come
from stdin, so let's just handle that case via readline instead of prompt.
This is what upstream Mercurial does.
Reviewed By: quark-zju
Differential Revision: D24122909
fbshipit-source-id: ab9d989a66d39990b688c65a1fae80bd48b0f42e
Summary: Add `--debug` flag to `read_res cat` command for debug printing entire entry rather than just the data blob.
Reviewed By: kulshrax
Differential Revision: D23999804
fbshipit-source-id: 6955854edab2643cffbe5fae484a398716b48055
Summary:
Initial support for a backend using edeanpi.
Note this is just the first step. Most code paths are not updated to the
streaming API to get commit data, so they will error out with commit not found
errors.
Confirm that commit data can be fetched via edenapi:
$ RUST_LOG=debug lhg dbsh --config experimental.lazy-commit-data=1
In [1]: master= repo['master'].node()
In [3]: cl.inner.getcommitrawtext(master) is None
Out[3]: True
In [4]: s=cl.inner.streamcommitrawtext(repo.nodes('master~10::master'))
In [5]: it=iter(s)
...
[2020-09-25T02:09:16.793Z DEBUG hgcommits::hybrid] >> resolve_remote input=[e6c4e203b66f1416e08dc597a2d63b91e93b1466, bfb610989e9dd701e785b4a3a5998e76d9709cab, 68bbfc79602a153895b761089e9479dd8fa33351, 5366fe39ad538463abae6c648eb5150bbb79d4c7, 5ea45d8ab0f8203837ca1736f36ded4a492571b4, 722da0a32eae12de5e85078beea2ae4b7aafe4a4, 4dbe3eab10d13b30697e1762eb7b9ff3ad0cf630, 430ae91aab8028b6572ccef89f8396dafec622c4, 5abd96c5420f0d512c63e768f8cea83f1c6691c9, c84ab3412cebfade730e95a1bc5ebc9b1dd0747b, 790ed2d40e4a0b08fb22fe9b4246fec0165f8a87]
[2020-09-25T02:09:16.793Z DEBUG hgcommits::hybrid] << resolve_remote input=[e6c4e203b66f1416e08dc597a2d63b91e93b1466, bfb610989e9dd701e785b4a3a5998e76d9709cab, 68bbfc79602a153895b761089e9479dd8fa33351, 5366fe39ad538463abae6c648eb5150bbb79d4c7, 5ea45d8ab0f8203837ca1736f36ded4a492571b4, 722da0a32eae12de5e85078beea2ae4b7aafe4a4, 4dbe3eab10d13b30697e1762eb7b9ff3ad0cf630, 430ae91aab8028b6572ccef89f8396dafec622c4, 5abd96c5420f0d512c63e768f8cea83f1c6691c9, c84ab3412cebfade730e95a1bc5ebc9b1dd0747b, 790ed2d40e4a0b08fb22fe9b4246fec0165f8a87]
...
[2020-09-25T02:09:16.958Z DEBUG zstore::zstore] >> Zstore::contains id=3422a85c3703dd0bf0030d5d4c1bb65775adff90
[2020-09-25T02:09:16.958Z DEBUG zstore::zstore] << Zstore::contains id=3422a85c3703dd0bf0030d5d4c1bb65775adff90
[2020-09-25T02:09:16.958Z DEBUG zstore::zstore] >> Zstore::insert data_len=1010 id=3422a85c3703dd0bf0030d5d4c1bb65775adff90
[2020-09-25T02:09:16.958Z DEBUG zstore::zstore] << Zstore::insert data_len=1010 id=3422a85c3703dd0bf0030d5d4c1bb65775adff90
...
[2020-09-25T02:09:16.959Z INFO zstore::zstore] >> Zstore::flush
[2020-09-25T02:09:16.959Z DEBUG indexedlog::log] >> Log::sync dirty_bytes=7048
[2020-09-25T02:09:16.959Z DEBUG indexedlog::log] << Log::sync dirty_bytes=7048
[2020-09-25T02:09:16.959Z INFO zstore::zstore] << Zstore::flush
In [6]: list(it)
Out[6]: [...]
The logs about `hgcommits::hybrid ... resolve_remote` shows the remote fetching
is working. The logs about `Zstore::insert` and `Zstore::flush` shows the
commit data were written to disk.
Reviewed By: sfilipco
Differential Revision: D23924148
fbshipit-source-id: a3d77999e29395ce5c603fe66412936947456534
Summary:
Support constructing the "hybrid" commits backend, which is similar to
"doublewrite" but read commit text from edenapi via the `streamcommitrawtext`
method.
Reviewed By: sfilipco
Differential Revision: D23924149
fbshipit-source-id: cb15ee4be7953af7798d460557ba2ae2d4f24a52
Summary:
The hybrid backend is similar to the doublewrite backend, except that it does
not use revlog to read commit data, but uses EdenAPI instead.
Note:
- The non-stream API will not fetch commit data from EdenAPI.
- The commit hashes are not lazy yet.
Reviewed By: sfilipco
Differential Revision: D23924147
fbshipit-source-id: eb2cf8d3a7e1704b4efb13ad3ad86f8b6a1b31d0
Summary:
This can be used like:
In [1]: s=cl.inner.streamcommitrawtext(repo.nodes('.%%master')) # repo.nodes returns a generator, becomes stream
In [2]: s
Out[2]: <stream at 0x7f5eec742df0>
In [3]: list(s)
Out[3]: [{'vertex': ..., 'raw_text': ...}, ...]
In [4]: s.typename()
Out[4]: 'cpython_ext::convert::Serde<hgcommits::ParentlessHgCommit>'
Reviewed By: sfilipco
Differential Revision: D23911870
fbshipit-source-id: f54959a551d446ed5b8086a2235fe74e47b29e70
Summary:
This makes it convertible to `PyObject` via `cpython_ext::convert::Serde`
without additional code or dependencies.
Reviewed By: sfilipco
Differential Revision: D23966993
fbshipit-source-id: 74d83524a7c0701cde7aa6d61bb930ff4a1c90f5
Summary:
This API allows us to stream the data. If callsites only use this API, we'll
be more confident that there are no 1-by-1 fetches.
Reviewed By: sfilipco
Differential Revision: D23911865
fbshipit-source-id: 4c7dd8c2b5be33be5a55822845d55345797bacdf
Summary:
The API is basically to resolve `input_stream` to `output_stream`, with a
stateful "resolver" that can resolve locally and remotely.
Reviewed By: sfilipco
Differential Revision: D23915775
fbshipit-source-id: 14a3a37fc897c8229514acac5c91c7e46b270896
Summary:
Introduce `FileMetadata` and `DirectoryMetadata` to `Treeentry`, along with corresponding request API.
Move `metadata.flags` to `file_metadata.revisionstore_flags`, as it is never populated for trees. Do not use `metadata.size` on the wire, as it is never currently populated.
Leaving `DirectoryMetadata` commented out temporarily because serde round trips fail for unit struct. Re-introduced with fields in the next change in this stack.
Reviewed By: DurhamG
Differential Revision: D23455274
fbshipit-source-id: 57f440d5167f0b09eef2ea925484c84f739781e2
Summary:
EdenAPI always checks the integrity of filenode hashes before returning file data to the application. In the case of LFS files, this resulted in errors because the filenode hash is computed using the full file content, but the blob from the server only contains an LFS pointer.
Fix the bug by exempting LFS blobs from filenode integrity checks. (If integrity checks for LFS blobs are desired, the LFS code should be able to do this on its own since LFS blobs are content-addressed.)
Reviewed By: quark-zju
Differential Revision: D24145027
fbshipit-source-id: d7d86e2b912f267eba4120d1f5186908c3f4e9e3
Summary:
`cpython_ext` provides utilities to implement From/ToPyObject directly for
serde types. Lets' use it to simplify the code and set up an example.
debugshell:
In [2]: s,f=api.commitdata(repo.name, list(repo.nodes('master')))
In [3]: list(s)
Out[3]:
[{'hgid': (7, 61, 22, ...), 'revlog_data': '...'}]
Note: `HgId` serialization should probably be changed to use `serde_bytes` somehow
so it does not translate to a Python tuple. That will be fixed later.
Reviewed By: kulshrax
Differential Revision: D23966987
fbshipit-source-id: 9278ccae6f543c387eafe401d4ef8d6ce96d370f
Summary:
This can be used to automate Python/Rust conversions for complex structures
like `CommitRevlogData`.
Reviewed By: kulshrax
Differential Revision: D23966988
fbshipit-source-id: 17a19d38270e6ef0952c13a1cd778487e84a94ff
Summary:
The goal is to implement `FromPyObject` and `ToPyObject` more easily.
Today crates have to dependent on `cpython` to implement `From/ToPyObject`,
which is somewhat unwanted for pure Rust crates.
The `ser` module used to ignore the `variant` field for non-unit enum variants.
They have been fixed so the serialized value can be deserialized correctly.
For example, `enum E { A, B(T) }` will be serialized to `"A"` for `E::A`, and
`{"B": T}` for `E::B`.
Reviewed By: kulshrax
Differential Revision: D23966994
fbshipit-source-id: c50d57bf313caeec65a604ed9b05a5729f3b3635
Summary:
Switch from the default tuple deserialization which only understands the tuple
format, to "bytes" deserialization, which understands not only the existing
"tuple" format (therefore compatible with old data), but also "bytes" and "hex"
formats (for CBOR).
This will unblock us from switching to bytes serialization in the future.
Note: This is a breaking change for mincode serialization. Mincode + HgId users
(zsotre, metalog) have switched to explicit tuple serialization so they don't use
the default deserializaiton and remain unaffected.
Reviewed By: kulshrax
Differential Revision: D23966995
fbshipit-source-id: 83dd53f57bd4e6098de054f46a1d47f8b48133d0
Summary: This will unblock us from switching HgId to bytes serialization by default.
Reviewed By: kulshrax
Differential Revision: D24009039
fbshipit-source-id: a277869ec24652af428cda581faffa62c25d32c4
Summary: Similar to D23966992 (2a2971a4c7), add support to serialize Key differently.
Reviewed By: DurhamG
Differential Revision: D24009041
fbshipit-source-id: 2ecf1610b989a04083196d180bc62307b5162c2f
Summary: Similar to D23966992 (2a2971a4c7), add support to serialize Sha256 differently.
Reviewed By: DurhamG
Differential Revision: D24009040
fbshipit-source-id: b77f6732802f95507e1540f0bbde4d5a92d13cac
Summary:
Add a way to specify different merge tools for interactive and non-interactive
mode.
This will be used for the default `editmerge` merge tool, which pops up the
`EDITOR` (vim) regardless of interactive mode, causing various user complains
and hangs, including `arc pull` running rebase triggering editor, or VS Code
running rebase triggering editor, and some other rebase hangs or vim errors.
Reviewed By: DurhamG
Differential Revision: D24069105
fbshipit-source-id: ec16fdc704cab6daeedb0c23d4028b4309d96d3f
Summary:
The `hg unhide` command acquired the repo lock without acquiring the wlock.
This causes locking order problems, as it calls other parts of the code that
will acquire the `wlock` (such as autopull during revset resolution) while it
is already holding the `lock`.
This can cause `hg unhide` to deadlock with other `hg` commands that acquire
`wlock` before `lock`.
Reviewed By: kulshrax
Differential Revision: D24129559
fbshipit-source-id: cf31ec661123df329f1773d2b67deb474d6476f8
Summary: Instead of returning an error upon receiving an empty request, just return a `Fetch` object that does nothing. This prevents Mercurial from crashing in situations where an empty request somehow makes it to the EdenAPI remote store.
Reviewed By: quark-zju
Differential Revision: D24119632
fbshipit-source-id: cf4ec707b4097656c76d7084a55b2d0b3150b679
Summary:
Previously, EdenAPI was using `remotefilelog.debug` to determine whether to print things like download stats. Let's give EdenAPI its own `debug` option that can be configured independently of remotefilelog.
One notable benefit of this change is that download stats will always be printed immediately after the HTTP request completes. This can help rule out network or server issues in situations where Mercurial appears to be hanging during data fetching. (e.g, if hg had downloaded all of the data but was taking a while to process it, the debug output would show this.)
Reviewed By: DurhamG
Differential Revision: D24097942
fbshipit-source-id: bf9b065e7b97fc7ffe50ab74b1b13e2fe364755c
Summary:
Previously phase calculation was done via a simple ancestor check. This
was very slow in cases that required going far back into the graph. Going a year
back could take a number of seconds.
To fix it, let's take the Rust phaseset logic and rework it to make only_both
produce an incremental public nodes set. In a later diff we can switch the
phaseset function to use this as well, but right now phaseset returns IdSet, and
that would need to be changed to Set, which may have consequences. So I'll do it
later.
Reviewed By: quark-zju
Differential Revision: D24096539
fbshipit-source-id: 5730ddd45b08cc985ecd9128c25021b6e7d7bc89
Summary:
A couple of files were moved but test-check-code.t wasn't updated to reflect
this, causing it to fail.
Reviewed By: DurhamG
Differential Revision: D24113079
fbshipit-source-id: 9a0c0b6f07a6532715bf5ee401036ded0a05b16a
Summary:
Pull Request resolved: https://github.com/facebookexperimental/eden/pull/65
Using $LOCALIP will ensure more consistent behavior when setting up the server in ipv4 or ipv6.
The LOCALIP variable was also abused when it was used to override ssh client address, so SSH_IP_OVEERIDE env was created here.
Lastly the result of `curl` call is now printed whenever the test failed to verify that Mononoke is running.
Reviewed By: farnz
Differential Revision: D24108186
fbshipit-source-id: e4b68dd2c5dd368851f0b00064088ffc442e31e0
Summary: D24070707: `[Thrift] Provide sorted fields to read_field_begin` made a change to the generated rust thrift files, so the eden/scm thrift files have to be regenerated to fix the build.
Reviewed By: farnz
Differential Revision: D24109655
fbshipit-source-id: e8575a76642673a11514fdce8e30f13ca28151f0
Summary:
Formats a subset of opted-in Python files in fbsource.
Black formatting was applied first, which is guaranteed
safe as the AST will not have changed during formatting.
Pyfmt was then run, which also includes import sorting.
The changes from isort were manually reviewed, and
some potentially dangerous changes were reverted,
and the directive was added to those
files. A final run of pyfmt shows no more changes to
be applied.
Reviewed By: zertosh
Differential Revision: D24101830
fbshipit-source-id: 0f2616873117a821dbc6cfb6d8e4f64f4420312b
Summary: This can be used by dag::Vertex and minibytes::Bytes.
Reviewed By: kulshrax
Differential Revision: D23966985
fbshipit-source-id: 3b4b29648e038ef49f26ce2b500119e148544d9e
Summary:
The py_stream_class causes the code to be more verbose. It basically enforces
the bindings crate to define new types wrapping pure Rust types, and then
define py_stream_class.
In a future diff, I'm adding FromPyObject/ToPyObject support for types that
implements serde Deserialize/Serialize. py_stream_class gets in the way,
because the blanket type from cpython-ext cannot be used in the py_stream_class
macro. cpython-ext is not the proper place to define business-related stream
types.
Therefore, define a type-erased Python class, and implement
FromPyObject/ToPyObject automatically for TStream<anyhow::Result<T>> where
T implements FromPyObject or ToPyObject.
The FromPyObject now converts a Python iterator back to a stream. It's
no longer zero-cost. However, I'd imagine such usecases can be short-cut
using pure Rust code.
Background: Initially, I added some FromPyObject/ToPyObject impls to pure
Rust crates gated by a "pytypes" feature. While that works fine with cargo
build, buck does not support dynamic features and the fact that we support
both py2 and py3 makes it extremely hard to support cleanly in buck build.
For example, if minibytes::Bytes defines ToPyObject for Bytes, then any
crate using minibytes would have 2 different versions: a py2 version, a
py3 version, and they both depend on python. That seems to be a bad approach.
Reviewed By: sfilipco
Differential Revision: D23966984
fbshipit-source-id: eafb31ad458dcbdd8e970d8e419a10fbbe30595f
Summary:
Per the feedback on D23920367 (318f5683a5), let's make the human-readable download stats shorter. Example:
```
Downloaded 10.59 MiB in 12.35s over 5 requests (7.19 Mb/s, latency: 123ms)
```
The amount downloaded is now reported in binary-prefixed bytes (so that it can be directly compared to file sizes) whereas the transfer rate is reported in decimal-prefixed bits per second (so that it can be directly compared to a user's measured network speed).
Additionally, we now use the default formatting available from `std::time::Duration`, which will automatically choose the appropriate display units.
Reviewed By: quark-zju
Differential Revision: D24096525
fbshipit-source-id: 39c49f1b08135bbae7a7544b1ffe2bdbfe1533a1
Summary:
The bfs fetching path in Rust was broken because it directly called
getdesignatednodes. getdesignatednodes returns False if it wasn't able to
succeed, so we need to be able to fall back in that case. The _prefetchtrees
function is meant for that, so let's just call that.
Reviewed By: singhsrb
Differential Revision: D24090946
fbshipit-source-id: d16c2c8f80f690a22046385f0e95785996a62949
Summary:
Previously, only the first `__iter__` gets wrapped. With D23095468 (34df768136), the first
`__iter__` is used by the "simplify graph" feature, not the main iteration
loop rendering the graph log output, causing the prefetch feature to fail.
File "edenscm/mercurial/commands/__init__.py", line 4196, in log
return cmdutil.graphlog(ui, repo, pats, opts)
# pats = ()
File "edenscm/mercurial/cmdutil.py", line 3250, in graphlog
ui, repo, revdag, displayer, graphmod.asciiedges, getrenamed, filematcher
File "edenscm/mercurial/cmdutil.py", line 3106, in displaygraph
rustdisplaygraph(ui, repo, dag, displayer, getrenamed, filematcher, props)
File "edenscm/mercurial/cmdutil.py", line 3208, in rustdisplaygraph
for (rev, _type, ctx, parents) in dag:
File "edenscm/mercurial/graphmod.py", line 63, in dagwalker
rootnodes = cl.tonodes(revs)
# revs = <baseset- [7408158, 72057594037927936, ...]
File "edenscm/mercurial/changelog2.py", line 196, in tonodes
return self.inner.tonodes(revs)
# revs = <baseset- [7408158, 72057594037927936, ...]
File "edenscm/hgext/phabstatus.py", line 281, in next
return self.__next__()
Lift the "first time" limitation for wrapping `__iter__` to solve the problem.
Reviewed By: simpkins
Differential Revision: D24066918
fbshipit-source-id: 6bbd244e729724e5143147bde60bcb4c8ee4bc80
Summary: Since this is only used in the manifest target, fold it into it.
Reviewed By: DurhamG
Differential Revision: D24062629
fbshipit-source-id: c3241b53bde7abba8a80a2945661d1a24b7e3034
Summary: We now get progress bar output when fetching from memcache!
Reviewed By: kulshrax
Differential Revision: D24060663
fbshipit-source-id: ff5efa08bced2dac12f1e16c4a55fbc37fbc0837
Summary: These aren't included anywhere, we can remove them.
Reviewed By: DurhamG
Differential Revision: D24062627
fbshipit-source-id: 9ff101eb44965ac3502ada3265ffcc8acc09d2e5
Summary:
This is no longer about datapack, but only about parsing manifest entries, thus
renaming.
Reviewed By: DurhamG
Differential Revision: D24062634
fbshipit-source-id: 5c52b784d20437e87012dd4bc6cb13d879da9cb9
Summary: The code doesn't use anything from libmpatch, we don't need to depend on it.
Reviewed By: DurhamG
Differential Revision: D24055084
fbshipit-source-id: 0f7bac73f1b711da4395e25619577a0a2e0ca959
Summary: These are unused, no need to keep the code around.
Reviewed By: DurhamG
Differential Revision: D24055085
fbshipit-source-id: 6246d746983a575c051ddcb51ae02582a764a814
Summary: This is unused, no need to keep it around.
Reviewed By: DurhamG
Differential Revision: D24054164
fbshipit-source-id: 161b294eb952c6b4584aa0d49d8ff46cd63ee30f
Summary:
This code is effectively unused. The only bit still relevant is that EdenFS
still depends on the Manifest class to parse a manifest.
Reviewed By: DurhamG
Differential Revision: D24037723
fbshipit-source-id: 901ae2ffc8960a95ec655a2e14d79afb8d32dcab
Summary: This is unused, let's remove it.
Reviewed By: DurhamG
Differential Revision: D24037722
fbshipit-source-id: bc8a272809cb1f20f54d651a39ee42ff57169534
Summary:
The 'diff()' template needs to be able to output non-utf8 patches.
To enable this, let's change the templater to output bytes instead of strings.
Note, the templater supports json output, which requires utf8 encoded values. In
that situation we escape any non-utf8 binary and clients would have to be aware
and unescape them.
Reviewed By: quark-zju
Differential Revision: D24029105
fbshipit-source-id: aa011696c0d97281b91c6871c116037d76c178e0
Summary:
This is unused code. It broke a test while I was doing python 3 work,
so let's just delete this.
Reviewed By: quark-zju
Differential Revision: D24063953
fbshipit-source-id: 323b323ff2b9717c9c16eed54adac467a321e2ae
Summary:
From just scanning through the code I identified a couple more spots
that use "" when they should use b"". I also noticed that
_parsecompressedrevision was producing memoryviews instead of bytes, so it's "u"
check would always fail.
Reviewed By: quark-zju
Differential Revision: D24037437
fbshipit-source-id: b4c589e30b5d35a1bcee16e0d6aa37d04f30129d
Summary:
The eden extension was hard coding empty string as the path when
reading from the store. This works fine when the data is available in a local
store (since most data stores don't actually use the path component), but
doesn't work when it needs to go to the server.
This wasn't an issue historically because the above fetch did the fetch for it.
A recent code change caused the above fetch to throw an error though, which was
then eaten, which caused the get code path to try to perform the fetch with an
empty path.
Reviewed By: krallin
Differential Revision: D24020467
fbshipit-source-id: cad717618e947fbdca978b45ce2181d46422d456
Summary:
Check a few backup files for `hg cloud sl` command information.
I added a comment explaining why we would need this.
At the moment some commands like for example `hg cloud switch` or the `hg pull -r` goes through mononoke backend and they will update a different backup cache file comparing with the `hg cloud sync` command that goes through the mercurial backend (write path).
As a result, `hg sl` command displays that some commits haven't been backed up.
Reviewed By: markbt
Differential Revision: D24046449
fbshipit-source-id: 56d924bf641833231cda6e6701e67f63c8453894
Summary:
All the current workspaces should be already populated because the release has
been running for a while. Remove the migration code and also fix a bug with
string to boolean conversion.
Reviewed By: mitrandir77
Differential Revision: D24045272
fbshipit-source-id: 347f0f46d343a13fc1d9f762f912af364813a66f
Summary: Disable CLANGTIDY checks for several places at the code.
Reviewed By: zertosh, benoitsteiner
Differential Revision: D24018176
fbshipit-source-id: b2d294f9efd64b2e2c72b11b18d8033f9928e826
Summary:
This would have been easier if we can upgrade tokio (D24011447).
For now, let's just solve it by using a channel so the mutex is not held for long.
The implementation has some side effects, though:
- panic message is not preserved.
- 'static lifetime is required on Future.
The `'static` lifetime is incompatible with some existing code. The old function
is preserved as `block_on_exclusive` and is used in places where a future does
not have `'static` lifetime.
Reviewed By: sfilipco
Differential Revision: D24033134
fbshipit-source-id: 7b35d1ff636d2a289db9b04e60419c31bdea9453
Summary: Now that we progress bars in Rust, add them to the EdenAPI client bindings and remove any existing progress bars around callsites in the Python code.
Reviewed By: quark-zju
Differential Revision: D24037797
fbshipit-source-id: eb26ccaae35ab23eb76f6f2b2be575a22e1f1e53
Summary: Larges files are chunked and the initial buffer should be bytes.
Reviewed By: quark-zju
Differential Revision: D24034645
fbshipit-source-id: 98156b1901182b345baaeb82c71faeb3cc57b131
Summary: Log changelog backend types so we can filter commands by type.
Reviewed By: DurhamG
Differential Revision: D24022284
fbshipit-source-id: c402aea0ce3bd20d0f310fea167f24cb1b7a3ae6
Summary:
On Windows, some Mercurial commands may create files in `.hg` directory even if
EdenFS is not running. As a result, the command itself will fail while the file
still left in the directory. Users typically will then start EdenFS and repeat
the same command.
However, due to the current design of EdenFS, it will not be able to recognize
these files created when it is stopped and return errors for any attempts to
write/remove the file, generate another error. Users then need to stop EdenFS
and manually remove the file in `.hg` directory to recover the repository. This
creates a very bad user experience.
The correct way to fix this is to teach EdenFS to track the modifications
happened when it is not running, however this will take a few weeks to get
there. For a temporary measurement, we teach Mercurial to abort when EdenFS
isn't running to avoid trickier recoveries.
Reviewed By: quark-zju
Differential Revision: D24001090
fbshipit-source-id: abc1ebcdae3819756fe64b5321f52a6e62c0c360
Summary:
Add a null progress bar implementation that just keeps track of state, similar to the `progress.nullbar` in hg's Python code.
A benefit of this is that code that optionally shows progress can unconditionally update the progress bar rather than wrapping it in an `Option` and checking for presence each time.
Reviewed By: markbt
Differential Revision: D23982318
fbshipit-source-id: ffd762b59cc0c9bd2ad0c67c3ca785350db4850f
Summary: Add Python bindings to the Rust progress wrappers. This may seem pointless since the Rust code just calls right back into Python, but this is a useful step to get the Rust and Python code to use a common interface for progress. (Which, in turn, will allow switching to a Rust progress implementation down the line.)
Reviewed By: markbt
Differential Revision: D23999816
fbshipit-source-id: 9bca0f23170d3ca474a1cb5d547840e63572ec71
Summary: Add Rust wrappers around Mercurial's Python `progress` module, allowing Rust code to create and use Python progress bars. The wrapper types implement the traits from the `progress` crate, so they can be passed to pure Rust crates in `scm/lib`. In typical usage, the Rust bindings will create a `PyProgressFactory`, which will be passed to pure Rust code as a trait object or via generics.
Reviewed By: markbt
Differential Revision: D23982317
fbshipit-source-id: 4c0fde0b2423b6449c7c5155fdfd98f5da042b0d
Summary:
This diff introduces a new `progress` crate that provides an abstract interface for progress bars in Rust code:
- The `ProgressFactory` trait can be used to create new progress bars.
- The `ProgressBar` trait allows Rust code to interact with the progress bar.
- The `ProgressSpinner` trait is similar, but for spinner-type progress indicators.
These traits are intended to be used as trait objects, allowing pure Rust code to accept an opaque `ProgressFactory` and use it to report progress. This kind of abstraction, while not common in idiomatic Rust code, allows the progress implementation to be completed decoupled from the pure Rust code, which is important given that Mercurial's progress bars are currently implemented in Python.
Part of the goal of this crate is to allow a smooth transition to pure Rust progress bars (once we eventually implement them). As long as the Rust progress bars implement the above traits, the can be used as drop-in replacements for Python progress bars everywhere.
Reviewed By: markbt
Differential Revision: D23982319
fbshipit-source-id: 9ccf167f18d9518bb0ed66e1606a5b8188d98428
Summary:
The protocol for getpack is length-prefixed. However, we currently advertise
the number of characters in filenames instead of their byte length. So, the
lengths we send don't necessarily correspond to the amount of data we send.
Indeed, if a filename contains multibyte characters, we'll advertise a lower
byte count than what we actually end up sending. This results in the last
byte(s) of the filename being interpreted by Mononoke as the start of another
piece of data, and eventually causes Mononoke to hang as it waits for more data
that the client will never send.
This fixes that bug in reading, and also fixes an identical instance of the bug
on the server side. I also double checked the gettreepack code, which AFAICT
doesn't have this bug.
Reviewed By: ahornby
Differential Revision: D24013599
fbshipit-source-id: af716f2bf9c02d312c0c8d2f449988e8f8858ab8
Summary:
The `hg cloud switch` command could be nicely used to debug other people workspaces by the Source Control Team.
Sometimes broken workspaces. This option would allow us to switch back from a broken workspace to our original workspace.
Mostly useful for the Source Control Team
Reviewed By: markbt
Differential Revision: D24014167
fbshipit-source-id: f2116cc13897149c8ac79790a31ebcce1f18a260
Summary:
With the new tracing-core (0.1.10 -> 0.1.16), it's no longer effective
to set EDENSCM_TRACE_LEVEL to more verbose within the test.
Let's set it in run-tests.py which spawns the test process instead.
Reviewed By: kulshrax
Differential Revision: D24004327
fbshipit-source-id: db2cadc7334eb59b25a1e0517e3d1a513e31e0fe
Summary:
As EdenFS depends on a few bits of Mercurial code, these needs to be able to
compile with Buck.
Reviewed By: chadaustin
Differential Revision: D24000881
fbshipit-source-id: 078a2a958039a63db1b716785f872b4bbde3bab6
Summary:
This parameter is gone. Let's stop using it. For now we can achieve the
same result by setting the treemanifest config value. In the long term we'll
probably get rid of depth in favor of smarter algorithms, like bfs traversals.
Reviewed By: genevievehelsel
Differential Revision: D23971898
fbshipit-source-id: cabcf0c088c95557edfe07ae85ce7d07e55a3082
Summary: Make `parents`, `data`, and `metadata` optional, and introduce `WireTreeAttributesRequest` for selecting which attributes to request on the wire.
Reviewed By: kulshrax
Differential Revision: D23406763
fbshipit-source-id: 5edd674d9ba5d37c23b12ab4d7b54bbf6c9ff990
Summary:
Adds a `WireTreeQuery` enum for query method, with a single `ByKeys(WireTreeKeyQuery)` available currently, to request a specific set of keys.
Leave the API struct alone for now.
Reviewed By: kulshrax
Differential Revision: D23402366
fbshipit-source-id: 19cd8066afd9f14c7e5f718f7583d1e2b9ffac02
Summary: The size can change with zstd upgrades. Do not test them.
Reviewed By: sfilipco
Differential Revision: D23976933
fbshipit-source-id: d560061b6e4fefc3bb89513bdb12c770ea0bd881
Summary:
This makes it so commit hashes are serialized to bytes instead of tuples in Python:
In [1]: s,f=api.commitdata(repo.name, list(repo.nodes('master')))
In [2]: list(s)
Out[3]: [{'hgid': '...', ...}]
Some `Vec<HgId>`s cannot be changed using this way. It'd be nice if we can change
the default `HgId` serialization to bytes.
Reviewed By: kulshrax
Differential Revision: D23966989
fbshipit-source-id: 4d013525419741d3c5c23621be16e70441bab3c4
Summary:
`HgId` currently serializes into a tuple of 20 items. This is suboptimal in
CBOR, because the items are untyped. A byte might be serialized into one or two
bytes:
In [2]: cbor.dumps([1,1,1,1])
Out[2]: b'\x84\x01\x01\x01\x01'
In [3]: cbor.dumps([255,255,255,255])
Out[3]: b'\x84\x18\xff\x18\xff\x18\xff\x18\xff'
CBOR supports "bytes" type to efficiently encode a `[u8]`:
In [5]: cbor.dumps(b"\x01\x01\x01\x01")
Out[5]: b'D\x01\x01\x01\x01'
In [6]: cbor.dumps(b"\xff\xff\xff\xff")
Out[6]: b'D\xff\xff\xff\xff'
Add `serde_with` with 3 flavors: `bytes`, `tuple`, `hex` to satisfy different
needs. Check the added docstring for details.
Reviewed By: kulshrax
Differential Revision: D23966992
fbshipit-source-id: 704132648f9e50b952ffde0e96ee2106f2f2fbcf
Summary: This just adds some explanation of `-r` to the `hg sparse list` docstring.
Reviewed By: markbt
Differential Revision: D23961027
fbshipit-source-id: 64ab406b07fe5d66fd53d4e520935aad3b0b351b
Summary:
Dynamicconfig can generate configs two ways, 1) via `hg
debugdynamicconfig` and 2) synchronously in-process in an hg command when it
detects that the dynamicconfig is completely missing or has the wrong version
number.
In the first case, dynamicconfig gets the repo name from the standard config
object loaded by the hg dispatch. In the second case, the standard config
object isn't even loaded yet, so dynamicconfig does a mini-load of the user and
repo hgrcs so it can get the repo name and user name (needed for dynamic
conditions).
Unfortunately the second code path computed the wrong path (it had two .hg/'s)
which meant the reponame and user name were always none. This meant that the
dynamicconfig on disk could randomly be either computed with or without a
reponame.
Let's fix the path computation, and add a test. We may want to make
dynamicconfig fail if no repo name is passed, but I'm not sure if we'll want to
support no-repo configuration at some point.
This didn't cause a problem for most people, since it would only happen during a
hg version number change, and 15 minutes later the background 'hg
debugdynamiconfig' process would fix it up. It did affect sandcastle though,
since it often creates new repositories and acts on them immediately.
Reviewed By: quark-zju
Differential Revision: D23955628
fbshipit-source-id: c922f4b523d19df9223aa28c97700b7011fc03eb
Summary:
The old code tried to express 4GB by using ^ to do an exponent. That
operator is actually the bitwise xor, so this was producing a limit closer to 4
bytes. It doesn't seem to have mattered much since a later diff overrode the
default via dynamicconfig. But let's fix this anyway.
Reviewed By: krallin
Differential Revision: D23955629
fbshipit-source-id: 6abebcb7e84b7a47f70ac501fa11b0dc60dfda7b
Summary: Now that the `async_runtime` crate exists, use Mercurial's global `tokio::Runtime` instead of creating one for each EdenAPI store.
Reviewed By: quark-zju
Differential Revision: D23945569
fbshipit-source-id: 7d7ef6efbb554ca80131daeeb2467e57bbda6e72
Summary:
`arc install-certificate` is going away, so let's reference the replacement
Created from Diffusion's 'Open in Editor' feature.
Differential Revision: D23948479
fbshipit-source-id: e629458ed2be1e1c09d9674935e3bcc890b05ad9
Summary:
Now that public phases are separated from draft, hidden behind the
ispublic() api, and no consumers require the entire set anymore, let's make them
lazy by testing commits against a lazy ancestor collection.
Reviewed By: quark-zju
Differential Revision: D23921326
fbshipit-source-id: 7c53345f41ff89d2d0ec11b1c09bbb2f89529c21
Summary:
Previously, _genrevancestor applied it's cutfunc function (which is
usually used to bound an ancestor traversal from processing public nodes) when
producing the parents of the current rev. In the walkrevtree function this means
we have to invoke cutfunc before we enqueue new parents onto the heap. This can
mean we have to process very old commits, even though walkrevtree might never
get to them.
Let's move the cutfunc to be at the deque stage instead of enque. When we make
public phases lazy, this will let us avoid looking up really old commits and
speed things up a lot.
Reviewed By: quark-zju
Differential Revision: D23921324
fbshipit-source-id: 8b520e9184441b9bd7581cdb71414f406cf9b544
Summary:
Unfortunately the current add operator implementation for nameset sorts
the result, which causes lazy namesets to be evaluated. Let's change the phase
revset to avoid that operator for now.
In a future diff we'll make publicrevs lazy, which will require this change to
be performant.
Reviewed By: quark-zju
Differential Revision: D23921325
fbshipit-source-id: c3477c30c1c5061b641c14b8aa73d07da6f7f398
Summary:
In a future diff we'll be changing the way public phase computation
works. Let's start by moving the draft phase computation out to belong to
draftrevs(), so then we can change publicrevs() without worrying about draft. In
the process, let's simplify draftrevs to just be an only() query.
Reviewed By: quark-zju
Differential Revision: D23921327
fbshipit-source-id: 255eab4b796bf7bc4467544cf8231b3ed98c08c8
Summary:
In an upcoming diff we'll be splitting draft and public calculations
apart when in headbased repositories. To make this cleaner, let's stop using
loadphaserevs and start using draftrevs and publicrevs accessors.
Reviewed By: quark-zju
Differential Revision: D23921328
fbshipit-source-id: 26a2a6fdc1201416bf61c7d32e3ede62d9579546
Summary:
These tests rely on sleep to simulate race conditions. They are the
largest source of hgbuild breakages. Let's just disable them. Most of this code
is going away in a couple months anyway when Mononoke takes over and revlogs go
away.
Also fixes a failure in test-bookmarks-strip.t that showed up.
Reviewed By: singhsrb
Differential Revision: D23922660
fbshipit-source-id: 35cacc19de03c031c84c1ebf2124884f4b4248e6
Summary: Now that the EdenAPI server is using the `LoadMiddleware` from `gotham_ext`, each response will contain an `X-Load` header that contains the number of active requests that the server is currently handling.
Reviewed By: quark-zju
Differential Revision: D23922809
fbshipit-source-id: 973143de5ddccf074d28aa3ef38d73f9fc1501b6
Summary:
Network speeds are usually reported in megabits per second (Mb/s), whereas file sizes are usually reported in [mebibytes](https://en.wikipedia.org/wiki/Binary_prefix) per second (MiB/s). Previously, the HTTP client reported neither of those and instead reported megabytes per second (MB/s).
This diff changes the latter to the former so that the numbers are more immediately useful. As a bonus, the speeds are now directly comparable to those reported by `hg debugnetwork`.
Reviewed By: quark-zju
Differential Revision: D23920367
fbshipit-source-id: 46500a42681ab83fc7c4ead82980e8ed620a4d5a
Summary: Now that stats are logged to `tracing` by the `HttpClient` directly, we no longer need to log them here. This commit backs out D23858077 (613fbc858f) which added the logging.
Reviewed By: quark-zju
Differential Revision: D23919308
fbshipit-source-id: 23d3a12c5307bc4b84dd9ffd25bd376718e3cc91
Summary:
Improve the log output of the HTTP client to avoid spewing redundant debug messages.
As part of this change, logging now uses the `tracing` crate instead of the `log` crate for better integration with the rest of Mercurial's logging infrastructure. Right now, `tracing` is just being used as a drop-in replacement for `log`, but now that it's in use we can start using its full capabilities (such as defining tracing spans) in later diffs.
Reviewed By: quark-zju
Differential Revision: D23919310
fbshipit-source-id: 95555ad083ead805ceece39c6e30aaf879bdf2bc
Summary:
Fix the `hg mv` and `hg cp` commands to wait for the lock if the repository is
locked, instead of immediately failing.
The current behavior is quite annoying as `hg mv` can easily fail due to
commitcloud sync commands running in the background and holding the lock.
I can't see any good reason why this command should not wait for the lock to
be held. The current behavior of failing when the lock is held dates back to
2006, in rFBSf5b6e7d2f591, when locking was first added to this command.
Reviewed By: DurhamG
Differential Revision: D23896302
fbshipit-source-id: 29bfeb8fd356abd31b6e1eb30c8f211a31fb044b
Summary:
The remotefilelog.indexedlog{data,history}store configs are no longer in use,
so we shouldn't use them as a way to test whether the indexedlog are present.
Reviewed By: quark-zju
Differential Revision: D23918133
fbshipit-source-id: 902a369f5e1774b8e2fa1e4b3280fbb7cc377775
Summary:
We were using the timeout parameter on `Multi::wait` (equivalent to `curl_multi_wait` in C) incorrectly. Previously, we were passing in the timeout provided by `curl_multi_timeout`.
This is incorrect usage because the value returned by `curl_multi_timeout` is the current value of libcurl's internal timeout (based on the state of the transfers), which will always be respected. The actual intention of the timeout parameter is to allow the caller to specify a hard cap on curl's internal timeout, so we should just pass some reasonable default value here. ([See explanation here.](https://github.com/curl/curl/issues/2996))
The purpose of `curl_multi_timeout` is to allow libcurl to tell the application what its desired timeout is in situations where the application itself is waiting for socket activity (using something like `epoll`), which is not the case when using `curl_multi_wait`.
Reviewed By: DurhamG
Differential Revision: D23914093
fbshipit-source-id: 76a25d7c59a4b08437c8d7be3d24708fb37b9172
Summary: Add appropriate EdenAPI calls to allow for HTTP tree fetching in `treemanifest`. Enabling `remotefilelog.http` essentially reroutes `_prefetch` and `getdesignatednodes` to their HTTP equivalents.
Reviewed By: DurhamG
Differential Revision: D23877319
fbshipit-source-id: 8a71934b47d07d2655fa46c103a14fb99e2f7b1f
Summary: Use the functionality from D23910534 (721f5af278) to set a timeout for EdenAPI requests, configured via the `edenapi.timeout` option.
Reviewed By: DurhamG
Differential Revision: D23911552
fbshipit-source-id: 4a6e3de1094d0faa1daaf6fe4b9b7aafb37a25a8
Summary:
Cast `names` and `nodes` to lists. The reason that they are sets is deduplication, but otherwise the code doesn't rely on them being sets (and in fact casts them to lists at multiple points later).
The main motivation for this is to allow these to be passed to Rust code later. The Rust bindings make a distinction between sequence types and unordered types, so passing a set in place of a list would result in a type error.
Reviewed By: DurhamG
Differential Revision: D23893108
fbshipit-source-id: 9ce2addb824867bcb2d24ba14c589b8791a156e8
Summary: Add the ability to set a timeout on HTTP requests. Equivalent to [`CURLOPT_TIMEOUT_MS`](https://curl.haxx.se/libcurl/c/CURLOPT_TIMEOUT_MS.html).
Reviewed By: DurhamG
Differential Revision: D23910534
fbshipit-source-id: a7aec792ec3c122a01aa44fcfe2e2df6e3a111fc
Summary: These concatenated strings and bytes and needed to be fixed.
Reviewed By: kulshrax
Differential Revision: D23907301
fbshipit-source-id: 0008d7d54469266ecbae8ddaaa7625820f62cb7e
Summary:
There are several places in the HTTP client where we log and discard errors. (Typically, these are "this should never happen" type situations.)
Previously, these were logged at the `trace` log level, meaning that in practice no one would ever know if we did hit these errors.
Let's upgrade them to `error` so that they'll be printed out. (In theory, users should never see these error messages unless something has gone horribly wrong.)
Reviewed By: DurhamG
Differential Revision: D23888268
fbshipit-source-id: 9007205f946ebb0127238c76812cf62524878047
Summary:
This test relies on precise timing of racing processes. This is flakey
in our automated tests. Since this is mainly about revlog based repos, and we
only have that on the servers now, and we're going to delete them soon, let's
delete this test to make our tests more stable.
Reviewed By: kulshrax
Differential Revision: D23908316
fbshipit-source-id: 3fd110a8267d3bc16bbcb4545b9ff921274f7588
Summary: Linkrevs are on their way out. Let's switch to linknodes.
Reviewed By: quark-zju
Differential Revision: D23765176
fbshipit-source-id: 0dc1e0db11d732ce1edd24d863f32f08a5a5ce42
Summary:
The rust contentstore doesn't allow runtime manipulation of the list of
stores, which is required in order to insert the bundle store into the store.
Let's continue using the old python union store in these cases. This still let's
us delete the python pack code later, but we'll have to keep around the python
union store until we come up with a better solution.
Reviewed By: quark-zju
Differential Revision: D23689630
fbshipit-source-id: 0c60e6f268e81804149afa24409f64b5303e1e34
Summary:
The remaining test failures are mostly around bundle support, which
I'll fix in a later diff.
Reviewed By: quark-zju
Differential Revision: D23664037
fbshipit-source-id: 2bdde3cb4fcded6e0cf3afdc23269662544821df
Summary:
The high level prefetch trees API had a depth parameter, but the Rust
prefetch path doesn't support that. In the long run we probably want to get rid
of the depth parameter (or make it more useful), so for now let's get rid of it
from the function signature. You can still set the depth via config, and the few
places that needed depth are changed to use the config.
Reviewed By: quark-zju
Differential Revision: D23772384
fbshipit-source-id: a037d7207d4076a47368366ef7fd2dc1cfbf5cfb
Summary:
Treemanifest needs to be able to write to the shared stores from paths
other than just prefetch (like when it receives certain trees via a standard
pull). To make this possible we need to expose the Rust shared mutable stores.
This will also make just general integration with Python cleaner.
In the future we can get rid of the non-prefetch download paths and remove this.
Reviewed By: quark-zju
Differential Revision: D23772385
fbshipit-source-id: c1e67e3d21b354b85895dba8d82a7a9f0ffc5d73
Summary:
Introduce separate wire types to allow protocol evolution and client API changes to happen independently.
* Duplicate `*Request`, `*Entry`, `Key`, `Parents`, `RepoPathBuf`, `HgId`, and `revisionstore_types::Metadata` types into the `wire` module. The versions in the `wire` module are required to have proper `serde` annotations, `Serialize` / `Deserialize` implementations, etc. These have been removed from the original structs.
* Introduce infallible conversions from "API types" to "wire types" with the `ToWire` trait and fallible conversions from "wire types" to "API types" with the `ToApi`. API -> wire conversions should never fail in a binary that builds succesfully, but wire -> API conversions can fail in the case that the server and client are using different versions of the library. This will cause, for instance, a newly-introduced enum variant used by the client to be deserialized into the catch-all `Unknown` variant on the server, which won't generally have a corresponding representation in the API type.
* Cleanup: remove `*Response` types, which are no longer used anywhere.
* Introduce a `map` method on `Fetch` struct which allows a fallible conversion function to be used to convert a `Fetch<T>` to a `Fetch<U>`. This function is used in the edenapi client implementation to convert from wire types to API types.
* Modify `edenapi_server` to convert from API types to wire types.
* Modify `edenapi_cli` to convert back to wire types before serializing responses to disk.
* Modify `make_req` to use `ToWire` for converting API structs from the `json` module to wire structs.
* Modify `read_res` to use `ToApi` to convert deserialized wire types to API types with the necessary methods for investigating the contents (`.data()`, primarily). It will print an error message to stderr if it encounters a wire type which cannot be converted into the corresponding API type.
* Add some documentation about protocol conventions to the root of the `wire` module.
Reviewed By: kulshrax
Differential Revision: D23224705
fbshipit-source-id: 88f8addc403f3a8da3cde2aeee765899a826446d
Summary: Add log messages for debugging using the `tracing` crate, which allows them to be enabled via `env_logger`.
Reviewed By: quark-zju
Differential Revision: D23858076
fbshipit-source-id: a8ef1afac6c9ecbfb5d6d78232aa0d03a2fe2054
Summary: Log HTTP stats to stderr to assist with ad-hoc debugging. Will not be printed unless `RUST_LOG` is set appropriately.
Reviewed By: quark-zju
Differential Revision: D23858077
fbshipit-source-id: 39acf3de3fd0ca4403a986eb5373a6a79f1d004a
Summary: Similar to D23819023 (c96de76ac0) but works on Python 2, too.
Reviewed By: DurhamG
Differential Revision: D23858273
fbshipit-source-id: b15be07c8657bc8cb37960b631f2b31e4a78892b
Summary:
test-commitcloud-sync.t is a new change and just needs to be made cross
platform.
I have no idea how test-common-commands-fb.t ever worked. When HGRCPATH is set,
I expect the system hgrc to not be loaded, and therefore we can't run hg-clone.
Let's just unset it, since this is meant to test if the new Mercurial can
execute a clone. Ideally we'd redirect the system hgrc to the in-repo
staticfiles, but that's more effort.
Reviewed By: singhsrb
Differential Revision: D23869645
fbshipit-source-id: 66669d9fd9c3a23b01bc43b365723185b7b2ed33
Summary:
Move some commit cloud operations under infinitepush read path:
those are:
* `hg cloud check` command
* `hg cloud sync` command when the local repo is clean
* `hg cloud switch` command will normally use the read path for the dest workspace because we clean up the repo before performing the switch
* `hg cloud rejoin` command we use in fbclone will normally go through the read path as it runs in a fresh repo
If something is broken, there is always a way to rerun any of these command with '--dest' flag pointing it to the write path.
```
./hg cloud check -r 0c9596fd1 --remote --dest infinitepush-write
./hg cloud sync --dest infinitepush-write
./hg cloud switch -w other --dest infinitepush-write
```
Those use cases are limited and the lag of forward filler shouldn't be noticeable for them but we will be able to collect more signal how Mononoke performs with Commit Cloud.
Sitevar to control the routing of read traffic:
https://www.internalfb.com/intern/sv/HG_SSH_WRAPPER_MONONOKE_ROLLOUT/#revisions_list
Reviewed By: mitrandir77
Differential Revision: D23840914
fbshipit-source-id: 40fbe2e72756e7a4cf8bc5be6a0b94f6cf4906b4
Summary:
With segmented changelog backend, the revs can be changed, even if len(repo)
didn't change. Caching revs might not get invalidated properly. Let's cache
head nodes instead.
Reviewed By: DurhamG
Differential Revision: D23856176
fbshipit-source-id: c5154c536298c348b847a12de8c4f582f877f96e
Summary:
On Ubuntu the output is a bit different:
```
$ hg cloud sync --use-bgssh
commitcloud: synchronizing 'server' with 'user/test/default'
- remote: /bin/sh: trashssh: command not found
- abort: no suitable response from remote hg!
+ remote: /bin/sh: 1: trashssh: not found
+ abort: no suitable response from remote hg: '[Errno 32] Broken pipe'!
```
Glob them out to make the test pass.
Reviewed By: DurhamG
Differential Revision: D23824735
fbshipit-source-id: 7f96149ee16daff31fd0a1c68975b5edfa27cc46
Summary:
It seems OSX python2 has SIGINT handler set to SIG_IGN by default when running
inside tests. Detect that and reset SIGINT handler to raise KeyboardInterrupt.
This fixes test-ctrl-c.t on OSX.
As we're here, improve test-ctrl-c.t so it checks a bit more things and run
quicker.
Reviewed By: DurhamG
Differential Revision: D23853455
fbshipit-source-id: 05c47650bc80f9880f724828d307c32786265e2c
Summary:
Phabstatus for smartlog uses `PeekeaheadList` rather than `PeekaheadRevsetIterator` as
all of the commits are known ahead of time, and we don't need to collect together
batches as we iterate across the revset.
However, we should still batch up requests to Phabricator, as users with very high
numbers of commits in their smartlog may hit timeouts.
Add a batching mechanism to `PeekaheadList` that splits the list into chunks to
return with each peekahead.
Reviewed By: liubov-dmitrieva
Differential Revision: D23840071
fbshipit-source-id: 68596c7eb4f7404ce6109e69914f328565e34582
Summary:
This provides a way to fix the local cache of backed up heads if it is in an
invalid state.
The most important, it will allow early dogfooding of write traffic from Mononoke
without the reverse filler in place for developers or for the team.
You could just run `hg cloud backup -f` assuming the repo is backfilled to fix
any inconsistency when switch between the two backends
Reviewed By: markbt
Differential Revision: D23840162
fbshipit-source-id: bbd331162d65ba193c4774e37324f15ed0635f82
Summary:
For Python 3 we must ensure that the displayer messages have all been converted
to unicode before providing them to the Rust graph renderer.
The is because the Python 3 version of `encoding.unifromlocal` is a no-op, so
the result may still be `bytes` that need to be converted to `str`.
Reviewed By: quark-zju
Differential Revision: D23827233
fbshipit-source-id: 8f2b707ceceb210c0a2b5b589b99d4016452c61c
Summary:
D23759711 (be51116cf4) changed the way signal handlers work, which apparently causes
this test to fail. The SIGCHLD signal of the child changing state is received
during os.waitpid, which apparently counts as a signal during a system call,
which throws an OSError.
I'm not sure what the real fix should be. Sleeping gets us past the issue, since
presumably the signal is handled before the system call.
Reviewed By: quark-zju
Differential Revision: D23832606
fbshipit-source-id: 70fca19e419da55bbf546b8530406c9b3a9a6d77
Summary:
This simplifies the code a bit, and avoids creating tokio Runtime multiple
times.
Reviewed By: kulshrax
Differential Revision: D23799642
fbshipit-source-id: 21cee6124ef6f9ab6e165891d9ee87b2feb553ac
Summary:
Exercises the PyStream type from cpython-async.
`hg dbsh`:
In [1]: s,f=api._rustclient.commitdata('fbsource', list(repo.nodes('master^^::master')))
In [2]: s
Out[2]: <stream at 0x7ff2db700690>
In [3]: it=iter(s)
In [4]: next(it)
Out[4]: ('6\xf9\x18\xe4\x1c\x05\xfc\xb0\xd3\xb2\xe9\xec\x18E\xec\x0f\x1a:\xb7\xcd', ...)
In [5]: next(it)
Out[5]: ('}\x1f(\xe1o\xf1a\x9b\x81\xb9\x83}\x1b\xbbt\xd2e\xb1\xedb',...)
In [6]: next(it)
Out[6]: ('\xf1\xf0f\x97<\xf3\xdd\xe41w>\x92\xd1\xc0\x9ah\xdd\x87~^',...)
In [7]: next(it)
StopIteration:
In [8]: f.wait()
Out[8]: <bindings.edenapi.stats at 0x7ff2e006a3d8>
In [9]: str(Out[8])
Out[9]: '2.42 kB downloaded in 165 ms over 1 request (0.01 MB/s; latency: 165 ms)'
In [10]: iter(s)
ValueError: stream was consumed
Reviewed By: kulshrax
Differential Revision: D23799645
fbshipit-source-id: 732a5da4ccdee4646386b6080408c0d8958dd67f
Summary:
Exercises the PyFuture type from cpython-async.
`hg dbsh`:
In [1]: api._rustclient.commitdata('fbsource', list(repo.nodes('master^^::master')))
Out[1]:
([...], <future at 0x7f7b65d05060>)
In [2]: f=Out[1][-1]
In [3]: f.wait()
Out[3]: <bindings.edenapi.stats at 0x7f7b665e8228>
In [4]: f.wait()
ValueError: future was awaited
In [5]: str(Out[3])
Out[5]: '2.42 kB downloaded in 172 ms over 1 request (0.01 MB/s; latency: 171 ms)'
Reviewed By: kulshrax
Differential Revision: D23799643
fbshipit-source-id: d4fcef7dca58bc4902bb0809adc065493bb94bd3
Summary:
Add a `PyFuture<F>` type that can be used as return type in binding function.
It converts Rust Future to a Python object with an `await` method so Python
can access the value stored in the future.
Unlike `TStream`, it's currently only designed to support Rust->Python one
way conversion so it looks simpler.
Reviewed By: kulshrax
Differential Revision: D23799644
fbshipit-source-id: da4a322527ad9bb4c2dbaa1c302147b784d1ee41
Summary:
The exposed type can be used as a Python iterator:
for value in stream:
...
The Python type can be used as input and output parameters in binding functions:
# Rust
type S = TStream<anyhow::Result<X>>;
def f1() -> PyResult<S> { ... }
def f2(x: S) -> PyResult<S> { Ok(x.stream().map_ok(...).into()) }
# Python
stream1 = f1()
stream2 = f2(stream1)
This crate is similar to `cpython-ext`: it does not define actual business
logic exposed by `bindings` module. So it's put in `lib`, not
`bindings/modules`.
Reviewed By: markbt
Differential Revision: D23799641
fbshipit-source-id: c13b0c788a6465679b562976728f0002fd872bee
Summary:
See the previous diff for context. Move the error handling and ipdb logic to
the background thread so it can show proper traceback.
Reviewed By: kulshrax
Differential Revision: D23819022
fbshipit-source-id: 8ddae019ab939d8fb2c89afca2a7769094ebe26a
Summary:
With D23759710 (34d8dca79a), the main command was moved to a background thread, but the
error handling isn't. That can cause less useful traceback like:
Traceback (most recent call last):
File "dispatch.py", line 698, in _callcatch
return scmutil.callcatch(ui, func)
File "scmutil.py", line 147, in callcatch
return func()
File "util.py", line 4358, in wrapped
raise value
Set `e.__traceback__` so `raise e` preserves the traceback information.
This only works on Python 3. On Python 2 it is possible to use
`raise exctype, excvalue, tb`. But that's invalid Python 3 code. I'm
going to fix Python 2 traceback differently.
Reviewed By: kulshrax
Differential Revision: D23819023
fbshipit-source-id: 953ac8bd6108f4c0dae193607bee3f931c2bd13e
Summary:
The parameter `mtimethreshold` should be used instead of a constant of 14 days.
This fixes an issue where sigtrace output takes a lot of space in hg rage
output.
Reviewed By: DurhamG
Differential Revision: D23819021
fbshipit-source-id: e639b01d729463a4822fa93604ce3a038fbd4a9a
Summary:
filter returns a filter object, so the second time we iterate, it is empty
This is only in Python3 I believe, so migration to py3 broke it.
Reviewed By: markbt
Differential Revision: D23815206
fbshipit-source-id: 1a6503b2bbfd44959307c189d17dec9b5d5ff991
