Summary:
Previously, `SpanSet::from_sorted_spans` allows having adjacent spans like
`[1..=2, 3..=4]`, while `SpanSet::from_spans` would merge them into `[1..=4]`.
Change it so `SpanSet::from_sorted_spans` merges them too. This simplifies
the `contains` logic and could make some Sets more efficient.
Reviewed By: sfilipco
Differential Revision: D23385248
fbshipit-source-id: 85b5ba9533f15034779e93255085a4fa09c6328a
Summary:
See the test change. Partially successful auto restack should have bookmarks
moved.
Reviewed By: DurhamG
Differential Revision: D23441932
fbshipit-source-id: 07e509a70bcc5cf81f702d40ec1b8dc4a5a781ff
Summary:
This commit introduces a new process spawning class derived
from the ChildProcess class in the watchman codebase.
`SpawnedProcess` is similar to folly::Subprocess but is designed around the
idea that we will use a system provided spawning API to start a process, rather
than assuming the use of `fork`.
`fork` is to be avoided because it can be expensive for processes with large
address spaces and also because it interacts poorly with threads on macOS. In
particular, we see the objC runtime terminating our process in some scenarios
where fork and threads are mixed.
There are some important differences from `folly::Subprocess` and that means
that some assumptions and uses need to be altered slightly from their prior
workings. For example, detaching a SpawnedProcess moves the responsibility of
waiting on the child to a periodic task as there is no way to detach via
posix_spawn without also using fork.
On the plus side, this commit allows unifying spawning between posix and
windows systems, which simplifies the code!
Reviewed By: xavierd
Differential Revision: D23287763
fbshipit-source-id: b662af1d7eaaa9ed445c42f6c5765ae9af975eea
Summary:
This commit introduces a few types from the watchman codebase:
`FileDescriptor` which is on posix systems represents a file descriptor,
and on Windows is a HANDLE (which can be a file, pipe or socket descriptor).
`Pipe` is a convenience struct that holds the read and write ends of a Pipe.
Note that we have a conceptual class with a windows specific Pipe type under
eden/fs/win/utils/Pipe.h; I remove that in the next diff in the stack.
There are a couple of differences from the watchman code
Reviewed By: chadaustin
Differential Revision: D23287819
fbshipit-source-id: 6ca90ba345037c6c3e308f588d690a899c9866a5
Summary:
We want to be able to report more than just on one prefix. Instead, let's add a regex-based reporting. To make deployment easier, let's keep both options for now and later just remove prefix-based one.
Note: this diff also changes how a situation with absent `undesired_path_prefix_to_log` is treated. Previously, if `undesired_path_prefix_to_log` is absent, but `"undesired_path_repo_name_to_log": "fbsource"`, it would report every path. Now it won't report any, which I think is a saner behavior. If we do ever want to report every path, we can just add `.*` as a regex.
Reviewed By: StanislavGlebik
Differential Revision: D23447800
fbshipit-source-id: 059109b44256f5703843625b7ab725a243a13056
Summary:
Those commands are broken right now: they try to write bytes but don't use
`writebytes`.
Reviewed By: DurhamG
Differential Revision: D23450968
fbshipit-source-id: 5d554771459f81718d90e5bad9a4c439cbb05d97
Summary:
When Python 3 wants to upload a file-like object, it does something a bit
awkward: it sets the `Transfer-Encoding` to `chunked`, but doesn't actually
chunk the data. Also, for some reason ,it still sets the `Content-Length`. I'm
not sure where that is coming from.
The thing is, when you set `Transfer-Encoding` to `chunked`, you do need to
chunk, or the other end is going to get very confused.
Unfortunately, this is not what happens here (note that the "send" logs are
from enabling http tracing in Python here, and those logs are basically one
line before `.send()` into a socket, so the chunking doesn't appear to happen
elsewhere):
```
[torozco@devbig051]~/opsfiles_bin % echo "aaaa" | ~/fbcode/buck-out/gen/eden/scm/__hg-py3__/hg-py3.sh debuglfssend https://mononoke-lfs.internal.tfbnw.net/opsfiles_bin
send: b'PUT /opsfiles_bin/upload/11a77c3d96c06974b53d7f40a577e6813739eb5c811b2a86f59038ea90add772/5 HTTP/1.1\r\nAccept-Encoding: identity\r\nContent-length: 5\r\nx-client-correlator: tQT3yBfFEzhVtqI5\r\naccept: application/mercurial-0.1\r\ncontent-type: application/x-www-form-urlencoded\r\nhost: mononoke-lfs.internal.tfbnw.net\r\ntransfer-encoding: chunked\r\nuser-agent: mercurial/4.4.2_dev git/2.15.1\r\n\r\n'
sendIng a read()able
send: b'aaaa\n'
reply: 'HTTP/1.1 400 Bad request\r\n'
header: Content-Type: text/html; charset=utf-8
header: Access-Control-Allow-Origin: *
header: proxy-status: client_read_error; e_upip="AcLKajO63Vab0hC4kzGZQsqck3P_YOu7HsBzshC-NCbuo31tlWWqCiVw5xVLh44LYYe7qioCPqYSb8-1cBpdvFDZb_t5oYRP1Q"; e_proxy="AcJjRKHG02qo6Bv6fEPCUVF7DpCyrq3rmSnXhRLWakKWREEvVpk4jc-tzDyG6l9jvn3vNo8PYPG_5hLtC3L1"
header: Date: Tue, 01 Sep 2020 13:10:35 GMT
header: Connection: close
header: Content-Length: 2959
```
What's a bit confusing to me here is where this Content-length header comes
from. Indeed, normally Python 3 will:
- Not infer a content-length for file-like objects (which is what we have)
https://fburl.com/ms94eq31
- Set Transfer-Encoding if no Content-Length is present:
https://fburl.com/f81g8v2j
So, it's a bit unexpected that a) we have a Content-Length (we shouldn't), and
that we b) also have a Transfer-Encoding header. That said, setting the
Content-Length does fix the problem, so that's what this diff does.
Reviewed By: DurhamG
Differential Revision: D23450969
fbshipit-source-id: e1f535ff3d0b49c0c914130593d9aebe89ba18ca
Summary:
This diff modifies how we rewrite file paths when we import into a repo by allowing the tool to apply multiple movers.
Motivation:
When we try to import into a small repo that pushredirects to a large repo, we have decided to import into the large repo first, then backsync to the small repo. To do that, we have to set a couple of flags related to importing into the large repo (see: D23294833 (d6895d837d)): bookmarks and import destination path. Previously, we fixed the destination path in large repo by applying the small_to_large repo syncer's mover on the destination path in small repo. e.g:
if small_to_large repo syncer mover = {
default_action = prepend(**large_dir**)
map = [...]},
then **destination_path** in small repo becomes **large_dir/destination_path** in large repo.
After this, we prepended the imported files with the new prefix with another mover: prepend(**large_dir/dest_path**)
a -> large_dir/dest_path/a
Consequently, all directories and files under **destination_path** would get imported under **large_dir/destination_path** in large repo with this logic. e.g.
However, it's possible that with push-redirections, some directories would get remapped to a different place in large repo. e.g
small_to_large syncer mover = {
default_action = prepend(**large_dir**)
map = [
dest_path/b -> random_dir/b
]},
but with the current repo_import implementation dest_path/b would get prepended to large_dir/dest_path/b.
To avoid this, we apply multiple movers on the imported files. e.g.
1. we prepend all files with dest_path:
mover = {
default_action: prepend(**dest_path**)
map={}} =>
a -> dest_path/a
b -> dest_path/b
2. we remap the files using the small_to_large repo syncer mover:
mover = {
default_action: prepend(**large_dir**)
map =
{dest_path/b -> random_dir/b}} =>
dest_path/a -> large_dir/dest_path/a
dest_path/b -> random_dir/b
Reviewed By: StanislavGlebik
Differential Revision: D23371244
fbshipit-source-id: 0bf4193b24d73c79ed00dfb38e2b0538388d1c0f
Summary: This is streaming clone warmup binary as per https://fb.quip.com/hfuBAdYnzr9M
Reviewed By: StanislavGlebik
Differential Revision: D23347029
fbshipit-source-id: f187a2f3529a7eae5998bab199228bfbe6057e6e
Summary:
As previous diffs in the stack show there were at least one place in the
codebase which used incorrect object context logger and that resulted in "blind
spots" in undesired file fetches logging i.e. undesired file fetches were
logged, but neither pid nor cmd-line was logged.
There are quite a few places in the codebase that use null
object fetch context, and threading the correct object fetch context to all of
them might be hard. Threading the context is a bit annoying, so it would be good to know something like "EdenDispatcher code is responsible for most of the blind spots, so let's thread the correct context there first". Or it would be equally good to know that none of the null object context are responsible for blind spots.
This diff might help us decide where we need to thread real object fetch context
first. Instead of passing null object fetch context let's pass null object
fetch context with causeDetail field. This field will be logged to scuba (see
BackingStoreLogger::logImport code), and instead of getting "Unknown" interface
we'll get e.g. "Unknown - EdenDispatcher::create", and that would highlight
where we need to thread the context.
A note about implementation - getNullContextWithCauseDetail returns a raw pointer
which is expected to be static i.e. it should work similarly to current
getNullContext implementation. It's quite a hack, but allows us to get rid of
memory allocations (we'd have one memory allocation per place in the code where
getNullContextWithCauseDetail). Let me know if you are ok with this hack.
Reviewed By: kmancini
Differential Revision: D23422526
fbshipit-source-id: e576bba9fc09e160fc42771c7589cdd1694d93c0
Summary:
As a follow up to the previous diff, let's also warn if dirstate includes
marker files that should not be included in any sparse profiles.
Reviewed By: DurhamG
Differential Revision: D23414361
fbshipit-source-id: 3d171328bf0ba5754e5bacde85f09abb4fed8603
Summary:
I'm not sure if that's the right thing to do, so I'd like feedback on that.
Check for opcode was added in D22050919 (fdb1af8bc9), but there was also a comment from
chadaustin
```
Why do you think fuseHeader isn't staying valid? It's a struct that's copied
into the RequestData. It does look like stealReq() sets the opcode to 0, but I
don't see anything that would affect the pid field.
```
So I wonder if this code can be responsible for some of the blind spots in
logging?
Reviewed By: chadaustin
Differential Revision: D23422635
fbshipit-source-id: 4d9a7171d685eb3f6f69da7b8a191df2f65ad897
Summary: There seems to be no need to use a shell.
Reviewed By: DurhamG
Differential Revision: D23124756
fbshipit-source-id: 7de1c23e2325fe88dc4c6a2c90563d06f109ed2f
Summary:
The Rust process utility avoids issues with interaction with Python and can do file
redirection on Windows.
Reviewed By: DurhamG
Differential Revision: D23124755
fbshipit-source-id: f72b88bafd19b3b41e53afbf6a4095d0d6bcb93a
Summary:
The Rust bindings handle the cross-platform differences and avoids issues
with Python / Rust interaction. Use it.
As we're here, extend the API to support cwd and env.
Reviewed By: DurhamG
Differential Revision: D23124171
fbshipit-source-id: fdc13f6eaeb25c05b53d385eb220af33dad984e1
Summary:
Spawning processes turns out to be tricky.
Python 2:
- "fork & exec" in plain Python is potentially dangerous. See D22855986 (c35b8088ef).
Disabling GC might have solved it, but still seems fragile.
- "close_fds=True" works on Windows if there is no redirection.
- Does not work well with `disable_standard_handle_inheritability` from `hgmain`.
We patched it. See `contrib/python2-winbuild/0002-windows-make-subprocess-work-with-non-inheritable-st.patch`.
Python 3:
- "subprocess" uses native code for "fork & exec". It's safer.
- (>= 3.8) "close_fds=True" works on Windows even with redirection.
- "subprocess" exposes options to tweak low-level details on Windows.
Rust:
- No "close_fds=True" support for both Windows and Unix.
- Does not have the `disable_standard_handle_inheritability` issue on Windows.
- Impossible to cleanly support "close_fds=True" on Windows with existing stdlib.
https://github.com/rust-lang/rust/pull/75551 attempts to add that to stdlib.
D23124167 provides a short-term solution that can have corner cases.
Mercurial:
- `win32.spawndetached` uses raw Win32 APIs to spawn processes, bypassing
the `subprocess` Python stdlib.
- Its use of `CreateProcessA` is undesirable. We probably want `CreateProcessW`
(unless `CreateProcessA` speaks utf-8 natively).
We are still on Python 2 on Windows, and we'd need to spawn processes correctly
from Rust anyway, and D23124167 kind of fills the missing feature of `close_fds=True`
from Python. So let's expose the Rust APIs.
The binding APIs closely match the Rust API. So when we migrate from Python to
Rust, the translation is more straightforward.
Reviewed By: DurhamG
Differential Revision: D23124168
fbshipit-source-id: 94a404f19326e9b4cca7661da07a4b4c55bcc395
Summary:
The Rust upstream took the "set F_CLOEXEC on every opened file" approach and
provided no support for closing fds at spawn time to make spawn lightweight [1].
However, that does not play well in our case:
- On Windows:
- stdin/stdout/stderr are not created by Rust, and inheritable by
default (other process like `cargo`, or `dotslash` might leak them too).
- a few other handles like "Null", "Afd" are inheritable. It's
unclear how they get created, though.
- Fortunately, files opened by Python or C in edenscm (ex. packfiles) seem to
be not inheritable and do not require special handling.
- On Linux:
- Files opened by Python or C are likely lack of F_CLOEXEC and need special
handling.
Implement logic to close file handlers (or set F_CLOEXEC) explicitly.
[1]: https://github.com/rust-lang/rust/issues/12148
Reviewed By: DurhamG
Differential Revision: D23124167
fbshipit-source-id: 32f3a1b9e3ae3a9475609df282151c9d6c4badd4
Summary:
It uses `sys.argv`, which might be rewritten by `debugshell`. Capture
`sys.argv` to make hgcmd more reliable.
Reviewed By: DurhamG
Differential Revision: D22993215
fbshipit-source-id: 5fa319e8023b656c6cdf96cb3229ea9f2c9b9b99
Summary: This allows us to run commands after changes were made to the repo.
Reviewed By: DurhamG
Differential Revision: D22993218
fbshipit-source-id: d9943dcda94da42970fb9107f48f4caa14b6a9d4
Summary:
Some code paths (ex. metalog.commit) use `util.timer()` as a way to get
seconds since epoch, and get 0 for tests. Other use-cases of `util.timer()`
are ad-hoc time measure for displaying speed / progress. They do not need high
precision or strong guarantee that the clock does not go backwards. Drop the
`time.perf_counter()` to meet the first use-case's expectation.
Reviewed By: singhsrb
Differential Revision: D23431253
fbshipit-source-id: 8bf2d1ed32e284e17285742e1d0fd7178f181fb3
Summary:
With segments backend, the revision numbers will be longer than commit hashes
and are confusing.
Reviewed By: DurhamG
Differential Revision: D23408971
fbshipit-source-id: e2057fa644fc7b6be4291f879eee3235bb4e687b
Summary:
Pulling from older repos (ex. years ago) could require GBs of commit text data.
Flush commit data if they exceed certain size.
This is for revlog compatibility.
In the future we probably just make commit text lazy to avoid this kind of issues.
Reviewed By: DurhamG
Differential Revision: D23408834
fbshipit-source-id: 273384f5a05be07877bb1c9871c17b53ba436233
Summary: This would be used to avoid excessive memory usage during pull.
Reviewed By: DurhamG
Differential Revision: D23408833
fbshipit-source-id: 8edd95ab8201697074f65cc118d14755a230567d
Summary:
`addcommits` is designed to be more efficiently if called with a batch of
commits. So let's buffer the commits to add then only call it once.
This avoids some N^2 behaviors, for example, the NameDag internally will
prepare "snapshot" of itself which involves coping the pending Rust vecs
about the segments and id <-> hash map.
The change makes `pull` usable from unusably slow:
Original Python Revlog backend:
```
In [1]: %trace repo.pull(bookmarknames=['master'],quiet=False)
5191 +466 | Apply Changegroup edenscm.mercurial.bundle2 line 516
| - Commits = 125 :
| - Range = a1d1b3ade136:2e3fe78af189 :
5191 +466 | changegroup.cg1unpacker.apply edenscm.mercurial.changegroup line 313
5192 +416 | Progress Bar: commits (progressbar)
5192 +415 | changelog.changelog.addgroup edenscm.mercurial.changelog line 536
5192 +409 | revlog.revlog.addgroup edenscm.mercurial.revlog line 2116
5215 +371 | changelog.changelog._addrevision (125 times) edenscm.mercurial.changelog line 558
```
DoubleWrite (Segments + Revlog) backend, Before:
```
In [2]: %trace repo.pull(bookmarknames=['master'],quiet=False)
2396 +154059 | Apply Changegroup edenscm.mercurial.bundle2 line 516
| - Commits = 323 :
| - Range = cb0b100180ba:5fb57c74f72e :
2396 +154059 | changegroup.cg1unpacker.apply edenscm.mercurial.changegroup line 313
2397 +151433 \ Progress Bar: commits (progressbar)
2397 +151433 | changelog2.changelog.addgroup edenscm.mercurial.changelog2 line 334
```
DoubleWrite (Segments + Revlog) backend, After:
```
In [2]: %trace repo.pull(bookmarknames=['master'],quiet=False)
4629 +512 | Apply Changegroup edenscm.mercurial.bundle2 line 516
| - Commits = 45 :
| - Range = cf23c6972934:1ff0c5f0e7ad :
4629 +512 | changegroup.cg1unpacker.apply edenscm.mercurial.changegroup line 313
4630 +494 | changelog2.changelog.addgroup edenscm.mercurial.changelog2 line 334
```
Reviewed By: DurhamG
Differential Revision: D23390435
fbshipit-source-id: dd97a5008dedd844d4134b87bfef190fa739a80b
Summary:
The users of addrevisoncb are gone.
This also removes the "alwayscache" parameter of "_addrevision".
Reviewed By: DurhamG
Differential Revision: D23390437
fbshipit-source-id: 7edd9dd0b93d4cb9d4f35d088a1aef719b450ec1
Summary: It is about legacy revlog formats that are no longer relevant.
Reviewed By: DurhamG
Differential Revision: D23390436
fbshipit-source-id: 58c2c432804181bcc6517d6c988777b843fc9ba4
Summary:
We have a few safeguards against creating full checkouts. However we have
sparse profiles that are not full, but that include very large directories
which normally should not be included.
This diff adds a logic that checks if a new sparse profile has any of the "marker"
files i.e. some files from a folder that should not be included. Operation
aborts if that the case, however there's always a way to workaround that.
Reviewed By: DurhamG
Differential Revision: D23414200
fbshipit-source-id: 626f392319eb1be8b35f39cadafb61f3c1dfefe3
Summary:
Scuba logging that tracks undesired file fetches has some blind spots i.e. a
lot of fetches have null pid and null cmd line. This diff tries to fix another
part of the problem.
TreeInode::getOrLoadChild() has TODO `pass a fetch context down through
getOrLoadChild to track this load`. This diff fixes this TODO, and also starts
to pass context from EdenDispatcher:lookup method.
Note that it adds quite a lot of new `ObjectFetchContext::getNullContext()`
calls, and potentially those might be responsible for blind spots in logging.
I'll try to address this problem in the next diffs.
Reviewed By: kmancini
Differential Revision: D23418218
fbshipit-source-id: 319d7436494d8dce3580289aae9963aa13bfc191
Summary:
Scuba logging that tracks undesired file fetches has some blind spots i.e. a
lot of fetches have null pid and null cmd line. This diff fixes at least part
of the problem.
TreePrefetchContext which is used from TreePrefetchLease didn't logged client
pid at all (in fact, it logged almost nothing). This diff fixes at least one blind spot, however it doesn't look like this is the only one.
Reviewed By: kmancini
Differential Revision: D23417451
fbshipit-source-id: 107884e94c6b40de999328ec2ef78fe22174c1ca
