Summary:
Solves an issue building eden GitHub version:
eden/fs/service/EdenServer.cpp:1044:34: error: ‘class facebook::stats::ServiceData’ has no member named ‘addStatValue’
stats::ServiceData::get()->addStatValue(
^~~~~~~~~~~~
Reviewed By: bolinfest, simpkins
Differential Revision: D9029274
fbshipit-source-id: c0e5ea492c1c38f16bfae4b08d41538a2443450f
Summary:
Fixed a data race in Eden's `FuseChannel` implementation that could cause a crash if a `FUSE_INTERRUPT` request was concurrently on a different thread while a first thread was still launching the original request.
Modified `FakeFuse` to use `SOCK_SEQPACKET` instead of `SOCK_STREAM` so that tests can submit several requests in a pipelined fashion without having to first wait until the previous request was received. This requires the `recvResponse()` function to first read the header with `MSG_PEEK` to determine the response size and then subsequently read the entire message atomically to avoid reading a truncated message.
Added a new unit-test that exercises the `FUSE_INTERRUPT` race condition by sending a series of alternating `FUSE_LOOKUP`/`FUSE_INTERRUPT` requests.
Reviewed By: simpkins
Differential Revision: D9023654
fbshipit-source-id: 0eb44669ea8c4f58cf4313adf6ceb11098115a70
Summary:
The `updateOverlayHeader()` only updates the overlay data if the inode is
materialized. This updates the name to clarify that.
(This function name change was previously part of D8884795, and I'm just
splitting it into its own separate diff.)
Reviewed By: bolinfest
Differential Revision: D9011358
fbshipit-source-id: 6024d64a1dee0b5d741bec32ed88f6c8f8dd8a9a
Summary:
I tried to build eden using GitHub version and ran into SELinux issues.
This patch fixed related issues, namely:
- `set(X Y)` sets X to string literal Y. Change it to `set(X ${Y})`.
- `SELINUX_INCLUDE_DIR` could be undefined. Check before use it.
- `./eden/fs/eden-config.h` and `./build/eden/fs/eden-config.h` both exist and
the former is used. Set include dirs so the latter gets used.
Reviewed By: bolinfest
Differential Revision: D9029272
fbshipit-source-id: 0c94bbe2f9e3fa90973802ddde16ad4d9ddfc0e8
Summary:
Many of the tests in FuseChannelTest.cpp perform blocking waits on Futures.
Previously they used a 100ms timeout. This could sometimes result in flaky
test failures if the system was under heavy CPU load. Bumping up the timeout
to 1 second appears to avoid this issue.
Reviewed By: lewissbaker
Differential Revision: D9024730
fbshipit-source-id: 7af78c61008c6b4c1e5e130b3d37b2f3ac787a01
Summary:
Update `InodeMap::updateOverlayForUnload()` to catch exceptions that occur
trying to save state to the overlay. If something goes wrong when saving
state there is not much we can do other than log an error. We still want to
unload the inode, and code that is unloading inodes generally cannot deal with
exceptions at this point. In particular if this error occurred while trying
to shut down an EdenMount the code would crash.
Reviewed By: chadaustin
Differential Revision: D8884795
fbshipit-source-id: c2f850f13d775be4b0a0a10f9df3948c7b2c8f4a
Summary: This commit changes MononokeBackingStore to use new APIs provided by Mononoke API Server.
Reviewed By: chadaustin
Differential Revision: D8882789
fbshipit-source-id: 0f06ca5f850af9fb52f1d593b9abd715a541488a
Summary:
Encountering a truncated overlay file doesn't necessarily indicate a software
bug in Eden. Depending on the underlying filesystem this often happens after
a hard system reboot since we write the overlay files without an `fdatasync()`
call.
Change the code to simply log an error and throw an exception rather than
using `EDEN_BUG()`. This makes it possible to exercise this code path in
tests without having it crash in debug builds.
Reviewed By: chadaustin
Differential Revision: D8988209
fbshipit-source-id: 8c0fe1dae692f4c493413d3939d2e4c21e0da596
Summary:
Sometimes, Eden's overlay (in `$client_dir/local/`) gets corrupt. In
particular, sometimes overlay files can be truncated or missing after a hard
reboot where the underlying filesystem state was not flushed to disk.
For such files, open(), stat(), unlink(), etc. from Eden report ENOENT, yet
readdir() on the containing directory shows that the file does exist.
In other words, the problematic file is undeletable:
```
$ ls -la dir/
/bin/ls: cannot access dir/corrupt_file: No such file or directory
total 0
drwxr-xr-x. 3 strager 0 Jul 10 21:41 .
drwxr-xr-x. 48 strager 0 Jul 10 21:41 ..
-?????????? ? ? ? ? corrupt_file
$ rm dir/corrupt_file
rm: cannot remove ‘dir/corrupt_file’: No such file or directory
```
Allow users to delete these problematic files (if the file was a regular file
and not a directory) by doing the following:
* Allow corrupt regular files to be unlink()d successfully.
* Allow corrupt regular files to be stat()d.
Making stat() succeed is a requirement by FUSE:
* For unlink(), FUSE performs FUSE_LOOKUP before FUSE_UNLINK. If FUSE_LOOKUP
fails, unlink() fails. Therefore, we must make FUSE_LOOKUP succeed for
corrupt files.
* For stat(), FUSE performs FUSE_LOOKUP and sometimes FUSE_GETATTR. Since we
must make FUSE_LOOKUP succeed (for unlink()), it's natural to make
FUSE_GETATTR succeed too.
A future diff will fix corrupted directories.
Reviewed By: chadaustin
Differential Revision: D8884793
fbshipit-source-id: 1100037bf52475fcca66f39946b917ce604f12dc
Summary:
CachedParsedFileMonitor provides cached access to an object created by parsing
a data file. The object can be accessed through "getFileContents().
"getFileContents()" will check if the file has changed and reload/parse if
necessary. A throttle is applied to limit change checks to at most to 1
per throttleMilliSeconds.
Reviewed By: simpkins
Differential Revision: D8903257
fbshipit-source-id: 7ed28610a3e9b40b98b0a9ffaff7b307bbcd32a8
Summary:
This commit improves the FileChangeMonitor interfaces. Changes are:
- on-demand check initiates a call-back
- FileChangeMonitor does not rely on user to call-back to update stat
- updated tests for new api
- fixed issue for determining if file changed (on error)
Reviewed By: simpkins
Differential Revision: D8989042
fbshipit-source-id: cf7a03b2bc8679891c8b9c22a24eed980eeb4060
Summary:
Overall plan to modify Future<T>::then to be r-value qualified and use Future<T>::thenTry or Future<T>::thenValue.
The goal is to disambiguate folly::Future and to improve type and lifetime safety of Future and its methods.
3/n: Codemod rvalue-future<T>.then(callable with operator()(not-a-try)) to rvalue-future<T>.thenValue(callable with operator()(not-a-try)).
Reviewed By: yfeldblum
Differential Revision: D8986716
fbshipit-source-id: 906339d9ffb90b3c38a24ce8bf0cef7be318d946
Summary:
Overall plan to modify Future<T>::then to be r-value qualified and use
Future<T>::thenTry or Future<T>::thenValue.
1/n: Codemod rvalue-future<T>.then(callable with operator()(Try<T>)) to rvalue-future<T>.thenTry(callable with operator()(Try<T>)).
Reviewed By: simpkins
Differential Revision: D8961903
fbshipit-source-id: ff17b7833d240c221197cdf0bf914b8a39f80b07
Summary: Update type information so that `mypy --strict` no longer has any warnings.
Reviewed By: strager
Differential Revision: D8940414
fbshipit-source-id: e55533ad5c7c67bd598fae2ae57e6a8d84b47b49
Summary:
Fix the arguments to `doctor.cure_what_ails_you()`, which were updated in
D8870494.
Reviewed By: chadaustin
Differential Revision: D8940413
fbshipit-source-id: 21d45036f668c9bae9a2002c360532918eb157a5
Summary:
If the `nuclide-connections` command is not installed
`subprocess.check_output()` throws a `FileNotFound` exception. Previously the
code only caught `CalledProcessError` and `ValueError` exception. This
updates the code to catch any Exception type, just in case some other
unexpected exception type can also be thrown too.
Reviewed By: chadaustin
Differential Revision: D8945774
fbshipit-source-id: 5e6f63981c3ed6695a687b4bfbaac97ea440b537
Summary:
Add a new `eden debug logging` command to show and update the current edenfs
logging settings.
I have deprecated the `eden debug set_logging_command`, but not removed it
entirely yet. It now longer shows up in the `eden debug --help` output, and
if run it prints the `eden debug logging` command that the user should run
instead.
Reviewed By: chadaustin
Differential Revision: D8907310
fbshipit-source-id: 7ed056637b236db5a50c619023bd8025b787e227
Summary:
This returns a `Repository` object and not a `Repository` subclass type.
This type information has been broken since D7512320 replaced the
`get_repo_class()` method with `create_repo()`.
Reviewed By: strager
Differential Revision: D8940415
fbshipit-source-id: 7711a2a33bc12fc361dfd0ff3f47eba1e587e6e7
Summary:
Add the FileChangeMonitor class to monitor for a file for changes. It provides
an interface "isChanged()" to check if the file has changed. It limits checks
by doing them on demand and by throttling. The throtte limits checks to 1 per
throttleMilliSeconds. It uses Stat information to determine if the file has
changed.
Reviewed By: simpkins
Differential Revision: D8898783
fbshipit-source-id: 067781489a9a91e5fe77cb613fba9d35b8348548
Summary:
InodePath is difficult to work with because it's just an alias to std::array. Make InodePath look more like std::string so using InodePath instances is more intuitive.
This diff should not change behaviour.
Reviewed By: chadaustin
Differential Revision: D8923073
fbshipit-source-id: 85a2548f0cfa61e50b6590048084076b9bece3da
Summary:
Make use of the TopLevelIgnores class to hold the system and user
GitIgnoreStack details. This is a cleaner implementation making ownership
semantics more intuitive. In later commits we will provide acess to the
TopLevelIgnores as part of the ServerState. It will be dynamically loaded.
Reviewed By: simpkins
Differential Revision: D8906226
fbshipit-source-id: d955436582498861ac4b4113a47f357432c8f32e
Summary:
Update `EdenFS.run_unchecked()` to return the full
`subprocess.CompletedProcess` argument rather than just the return code.
This will make it possible for future tests to check the output of the command
in addition to just its return code.
Reviewed By: strager
Differential Revision: D8905491
fbshipit-source-id: 7f17f5d103a06aeb56a3be0618683bd1c017fdf0
Summary:
D8508489 updated the integration test code so that the `capture_output`
argument to `EdenFS.run_cmd()` was no longer necessary and was not used.
However I forgot to actually remove the argument in that diff. This deletes
the argument.
Reviewed By: strager
Differential Revision: D8905492
fbshipit-source-id: d7a691f7d628a9fd5daa6723575e4454480f6aa1
Summary:
As of D8631281 `eden clone` now makes the mount point directory read-only and
puts a readme file inside of it. Since the directory is read-only the
integration tests are unable to clean it up using a normal `shutil.rmtree()`
call.
This fixes the integration tests to try making the directory writable first
if it encounters an error during cleanup.
Reviewed By: strager
Differential Revision: D8905488
fbshipit-source-id: c6b9d0b12c2b1dcb4a0839757cbcc3aad179bfc1
Summary:
This replaces the `StdoutPrinter` class with a different `Output` class
interface that combines both the `StdoutPrinter` functionality and the
underlying I/O object being printed to.
Previously the doctor code had to pass around two separate `out` and
`printer` objects. This simplifies things so that only one object has to be
passed around.
This also adds some additional error checking around the curses
initialization, and falls back to a plain output object if anything goes wrong
during setup. Previously we had seen some reports of `eden doctor` failing
due to errors in the terminal setup code.
Reviewed By: strager
Differential Revision: D8870494
fbshipit-source-id: a6dbafdde9aa5e1a0336fa7d723d83fa7d2426ca
Summary:
The raw Inode pointer in a DirEntry is more of an association than
ownership, so add comments and have clearInode return the old value.
Reviewed By: strager
Differential Revision: D8842315
fbshipit-source-id: d401dcdf4955ea335b39c2a57b0bedb1f83fdf9b
Summary:
When users run into "Transport endpoint not connected" errors they sometimes
attempt to run `eden status` or `eden doctor` to fix the problem. However
previously the CLI did not deal with being run from an invalid working
directory very well.
This updates the CLI to explicitly check if the current working directory is
valid before doing anything else. If it gets an ENOTCONN error it prints a
message telling the user how to update their shell's working directory. If
the `$PWD` environment variable points to a valid directory it attempts to
continue using that as the working directory. It records the fact that the
original working directory was invalid so that `eden doctor` can report this
error if that is the command being run.
Reviewed By: wez, strager
Differential Revision: D8731005
fbshipit-source-id: 4be55ccb43e22ed878114268e4dcc4c90c20e8e2
Summary:
Add a class to encapsulate details of top-level system and user ignore files.
This approach has the following benefits:
- simplifies memory management of the GitIgnore;
- separate structure that can be created and provided as part of ServerState
interface;
We will make use of it later in this commit stack.
Reviewed By: simpkins
Differential Revision: D8876064
fbshipit-source-id: 35c82b918c09e58068370401883edd8474dd3fbf
Summary: GitIgnore and GitIgnoreStack encapsulate details of ignore files. Further changes (as part of this commit stack) require copy constructors and assignment operators. Also, compiler wants destructors in .cpp file (since otherwise, it cannot tell the object size).Keeping this as a separate commit to simplify review process.
Reviewed By: simpkins
Differential Revision: D8730299
fbshipit-source-id: 7cb443906143c80209278b85fc5ad4dc5ea0bf56
Summary:
This adds initial code for an `eden debug fsck` command.
At the moment this simply scans the overlay and reports errors, but does not
correct any issues. In subsequent diffs I'll update it to support correcting
some problems.
There are also several other issues that I'll improve in subsequent diffs as
well. At the moment this code uses the same functions as other CLI commands
to find the client directory, and these only work if the checkout is already
mounted. For `fsck` we generally want the checkout to be unmounted before
checking it. I also plan to clean up some of the output handling as well.
Reviewed By: strager
Differential Revision: D8813714
fbshipit-source-id: 90238e94540db1cc06c17eabf8ce3a1bb808d4b3
Summary:
The methods were marked const when they were actually mutating the inner
objects, this was working because the pointed to objects were referenced via
`shared_ptr` handles
Reviewed By: yfeldblum
Differential Revision: D8851578
fbshipit-source-id: 4ca49390daf46f5c8681149da4d1e9eeab5aacb5
Summary:
While trying to make destroyWithInitRace non-flaky, it uncovered an
ASAN violation in the case that fuseCompleteFuture finishes during
EdenMount destruction. In that case, path_ gets destroyed prior to the
executor, so it's illegal to construct TakeoverData::MountInfo from
path_. This diff removes path_ entirely and reads it from
ClientConfig.
Reviewed By: simpkins, strager
Differential Revision: D8848663
fbshipit-source-id: f9368aa9eec7dfa8f2897cce55fad6d19723e30c
Summary:
We should only kick off prefetching for the files
that matched the glob. We were prefetching files that
didn't match the glob.
facepalms
Reviewed By: strager
Differential Revision: D8846994
fbshipit-source-id: 593e85d843ffa1cc0707ed1dc86f1385262821f5
Summary:
This makes it easier to add some test coverage.
There's no real functional change in this diff; the only code change is to
throw a system_error instead of a thrift eden error wrapper class from the core
globbing code. There's a little bit of code to restore this exception type in
the callers in EdenServiceHandler; this is covered by existing integration
tests, but I've also expanded that coverage to cover both variants of the glob
thrift calls.
Reviewed By: strager
Differential Revision: D8776767
fbshipit-source-id: 3ea4ea642ae5108aa4b0153541bd3604f010b54c
Summary:
Let's use consistent naming for the user ignore file. We will go with
'userIgnoreFile' as recomended. There is now symetry since we have
'systemIgnoreFile'.
I have another diff (later) that already uses the 'userIgnoreFile'.
Reviewed By: chadaustin
Differential Revision: D8823906
fbshipit-source-id: 47594e8971fa4db809821fc819da05e9eb2c5277
Summary:
Some of the heavier watchman->eden queries in www today
trigger calls to `getFileInformation` with very large numbers
of paths.
For a set of paths with a lot of common prefixes there is a lot
of repeated work to load same inodes over and over again.
eg: `a/b/c/d/A` and `a/b/c/d/B` both have to load `a -> b -> c -> d` before
they can load the leaf node.
This diff pre-processes the list of paths and builds a tree-shaped plan
so that we won't need to load any inode referenced by any of the paths
more than once.
The `getSHA1` method could benefit from a similar change; I'll do that in
a follow-up diff.
Reviewed By: strager
Differential Revision: D8578261
fbshipit-source-id: e899640a2ef6dcdb6b903d2a2735e9240496d74b
Summary:
The `eden debug overlay` code has some logic in python for parsing overlay
files. This moves that logic into a new module so I can re-use it for an
`fsck` command in a subsequent diff.
Reviewed By: wez
Differential Revision: D8813715
fbshipit-source-id: 6c32561db95f5da112cfd3e06b9e4653f431b94a
Summary:
This class name looks like it was copy-and-pasted incorrectly, probably from
the `eden debug set_log_level` code. Update the name to specify "Journal"
rather than "Level"
Reviewed By: strager
Differential Revision: D8813713
fbshipit-source-id: 2a5edd7f2de8279ee70dcc72b200c2b86b60f376
Summary:
Update `eden restart` to explain that the restart will be disruptive when
performing a full restart, and prompt the user for confirmation. This prompt
can be overridden with the `--force` option. Also print more messaging after
the restart completes telling users what to do if they see "Transport endpoint
not connected" errors from programs still using the old mount points.
This also updates `eden restart` to more carefully handle cases where there is
an existing edenfs daemon running but it is not fully healthy.
Reviewed By: wez
Differential Revision: D8731004
fbshipit-source-id: 05762187b47057b2930d0a6b71b0a6fdbd4aa7e5
Summary:
Fix two debug subcommands that were calling `os.getcwd()` while setting up the
argument parser. This caused the eden CLI to throw an exception of the
current working directory was invalid (for instance, if it was inside a stale
Eden mount point).
Reviewed By: strager
Differential Revision: D8731006
fbshipit-source-id: 8b7a67815d61b3476109aad646d21af8282ac68c
Summary:
EdenConfig uses stat to determine if config files have change. However, it
should use st_mtim.tv_nsec rather than st_mtime because this provides
nano-second resolution.
Reviewed By: simpkins
Differential Revision: D8797143
fbshipit-source-id: aadf114f509116d497a4c237ecaf108eb3824d5f
Summary:
This commit focuses on the following functionality:
- Eden configuration should have reasonable default values.
- Eden should allow configuration to be specified in various manners: default, cli, user and system configuration files.
- Configuration values should have the following precedence (hi to low): cli,
user config, system config, default.
- A light-weight mechanism is required to detect and update configuration changes.
Reviewed By: simpkins
Differential Revision: D8594297
fbshipit-source-id: ed195212669b18b450a1eae359e4d23905beadb4
Summary:
This refactors `HgBackingStore::getTreeForCommit()` so that it no longer invokes
`importThreadPool_.get()` if the `LocalStore` contains the `Tree` for the
specified `commitID` (which is commonly the case). When the `LocalStore` has a
cache hit, there is no need to import any data from Mercurial, so there is no
need to involve the `importThreadPool_`.
As explained in T30958650, `importThreadPool_.get()` can be relatively expensive
because it can have the side-effect of invoking
`HgImporterThreadFactory::newThread()`, which creates a new `HgImporter`, which
in turn does some potentially expensive work (it spawns a new subprocess for the
import helper and does some treemanifest-related I/O, if appropriate).
Because `HgBackingStore::getTreeForCommit()` is called as part of `hg status`,
the existing implementation was preventing us from reliably providing <30ms
`hg status` calls because we would frequently get "unlucky" and unnecessarily
create a new `HgImporter`.
Reviewed By: wez
Differential Revision: D8764805
fbshipit-source-id: 0acf7779560ddf182e7eb50daea54c5ee57030d0
Summary:
This is part of "the great r-valuification of folly::Future":
* This is something we should do for safety in general.
* Context: `Future::get(...)` means both `Future::get()` and `Future::get(Duration)`
* Using lvalue-qualified `Future::get(...)` has caused some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
* Problems with `Future::get(...) &`: it moves-out the result but doesn't invalidate the Future - the Future remains (technically) valid even though it actually is partially moved-out. Callers can subsequently access that moved-out result via things like `future.get(...)`, `future.result()`, `future.value()`, etc. - these access an already-moved-out result which is/can be surprising.
* Reasons `Future::get(...) &&` is better: its semantics are more obvious and user-testable. It moves-out the Future, leaving it with `future.valid() == false`.
Reviewed By: LeeHowes
Differential Revision: D8756764
fbshipit-source-id: 5c75c79cebcec77658a3a4605087716e969a376d
Summary:
This is part of "the great r-valuification of folly::Future":
* This is something we should do for safety in general.
* Context: `Future::get(...)` means both `Future::get()` and `Future::get(Duration)`
* Using lvalue-qualified `Future::get(...)` has caused some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
* Problems with `Future::get(...) &`: it moves-out the result but doesn't invalidate the Future - the Future remains (technically) valid even though it actually is partially moved-out. Callers can subsequently access that moved-out result via things like `future.get(...)`, `future.result()`, `future.value()`, etc. - these access an already-moved-out result which is/can be surprising.
* Reasons `Future::get(...) &&` is better: its semantics are more obvious and user-testable. It moves-out the Future, leaving it with `future.valid() == false`.
Reviewed By: yfeldblum
Differential Revision: D8711368
fbshipit-source-id: fbfcb731097cdf9d8d98583956bc7fe614157a6b
Summary:
This moves the fsattr helper program into the helpers/ directory
created in D8373672.
Reviewed By: chadaustin
Differential Revision: D8705391
fbshipit-source-id: 43e04d8b06b5d17caefafdfc750e3e60cad1b0cc
Summary:
An upcoming change to mercurial will make trees only serialize to disk at the
end of the command. Since Eden is bypassing commands and calling prefetch
directly, it needs to also manually tell Mercurial to serialize.
Reviewed By: chadaustin
Differential Revision: D8679211
fbshipit-source-id: 865ecaf340fa6003c48a6563f713479f07a29f16
Summary:
This is part of "the great r-valuification of folly::Future":
* https://fb.facebook.com/groups/fbcode/permalink/1726007767436055/
* This is something we should do for safety in general.
* Using lvalue-qualified `Future::get()` has caused some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
* Drill-down: `Future::get() &` moves-out the result but doesn't invalidate the Future - the Future remains (technically) valid even though it actually is partially moved-out. Callers can subsequently access that moved-out result via things like `future.get()`, `future.result()`, etc., which will cause surprising results - accessing an already-moved-out result. The semantics of `Future::get() &&` are more obvious: it moves-out the Future, leaving it with `future.valid() == false`.
Codemod steps (where `get(...)` means both `Future::get()` and `Future::get(Duration)`):
* expr.get(...) ==> std::move(expr).get(...) // if expr is not already an xvalue
* expr->get(...) ==> std::move(*expr).get(...)
Note: operator precedence of that last step is safe - no need to parenthesize `expr`. Reason: `->` binds more tightly than unary `*`.
Reviewed By: yfeldblum
Differential Revision: D8625157
fbshipit-source-id: 70323ff3189631ce5597d3c9b17629c99f5d457b
Summary:
Update "eden clone" to create a README file inside the mount point directory
before the checkout is mounted. When Eden is running normally this file will
not be visible because the Eden checkout has been mounted on top of this
directory. However if the checkout is not currently mounted users will see
this file instead.
This should hopefully make it easier for users to figure out how to remount
their checkout on their own, and should prevent them from worrying that their
files have been deleted when the mount point simply is not mounted.
Reviewed By: chadaustin, wez
Differential Revision: D8631281
fbshipit-source-id: 7610450983687a839bdbc3558ff35e0422edc3be
Summary:
The eden integration tests currently run hg through the telemetry wrapper.
This wrapper script adds profiling arguments to a percentage of the commands
it runs. The extra arguments that it adds are not entirely transparent to the
user, and are unfortunately reported in the output of "hg journal".
This was causing occasional test failures in the Eden integration tests that
check the output of "hg journal".
This diff changes the Eden code that checks the journal output to try to strip
off these flags before checking the command output.
Reviewed By: wez
Differential Revision: D8631180
fbshipit-source-id: 004d3bcce291ffc2208f85a29cb34a921f16fdde