Summary:
_transient_unmanaged_systemd_user_service_manager has a few inner functions and a few variables shared between these inner functions. The function is pretty long and hard to follow.
I think a class is more familiar than closures. Refactor _transient_unmanaged_systemd_user_service_manager into a class: turn inner functions into methods, and shared variables into instance attrs.
This diff should not change behavior.
Reviewed By: simpkins
Differential Revision: D13017642
fbshipit-source-id: d20f7476142fa5d7ba0ae09291228ec63e127338
Summary:
If you enable type checking of util.py, mypy complains about _remove_readonly having the wrong type:
eden/integration/lib/util.py:87: error: Argument "onerror" to "rmtree" has incompatible type "Callable[[Callable[[str], Any], str, Tuple[Type[Any], BaseException, TracebackType]], None]"; expected "Optional[Callable[[Any, _PathLike[str], Any], Any]]"
Fix the error by making _remove_readonly compatible with onerror's signature in typeshed [1]:
overload
def rmtree(path: _AnyPath, ignore_errors: bool = ...,
onerror: Optional[Callable[[Any, _AnyPath, Any], Any]] = ...) -> None: ...
This diff should not change behavior.
[1] 4dc21f04dd/stdlib/2and3/shutil.pyi (L90-L92)
Reviewed By: simpkins
Differential Revision: D13086137
fbshipit-source-id: 222e5fa2e06a26464483a0f09545089a7ecc5234
Summary:
While debugging flakiness in test_running_simple_service_is_active, I noticed that sometimes ActiveState=active despite the service process being dead. This can happen if the service process is killed outside systemd. When this happens, SubState=exited instead of the expected SubState=running.
Prevent false positives in SystemdServiceTest tests by checking SubState in addition to ActiveState.
Reviewed By: simpkins
Differential Revision: D13032619
fbshipit-source-id: 2d5754291a19290d29a817115923e9cc5efc90ab
Summary:
A few times, I've needed to manually make _is_system_booted_with_systemd return False in order to emulate how Sandcastle behaves. Make this easier by introducing an environment variable, EDEN_TEST_FORCE_SYSTEMD_USER_SERVICE_MANAGER_TYPE, which allows choosing how to start `systemd --user` when running tests.
When EDEN_TEST_FORCE_SYSTEMD_USER_SERVICE_MANAGER_TYPE is unset, this diff should not change behavior.
Reviewed By: simpkins
Differential Revision: D13031404
fbshipit-source-id: ecbd5f90ff55f4dffa47ba797686db5c25a7198c
Summary: Add type annotations to all functions in eden/fs/service/client.py
Reviewed By: strager
Differential Revision: D13051091
fbshipit-source-id: 23b93008352664336ad155a7f5cc281bd5529702
Summary:
Add type annotations for class member variables. The pyre type checker has
some limited automatic type detection for member variables set in
`__init__()`, but in general it expects member variables to be explicitly
declared at the top-level of the class.
Reviewed By: strager
Differential Revision: D13051092
fbshipit-source-id: 080259ab3f422ffae2b908ed610062237105ccbe
Summary:
Update `EdenRepoTest` to explicitly declare its own `get_thrift_client()` that
just calls through to its parent class' implementation.
I'm not sure why, but mypy can't seem to figure out the correct return type of
this method without this extra declaration. It gets confused when this method
is called from `EdenRepoTest` subclasses, and either complains that it doesn't
know the type at all and needs an explicit type declaration, or in some cases
it thinks the type is `Optional[EdenClient]` rather than `EdenClient`.
Currently mypy doesn't complain about these errors by default, but it seems to
complain when I start adding more type information for other class members.
Adding this extra declaration makes it possible to start adding type
annotations in more places in the code without triggering new errors from
mypy.
Reviewed By: strager
Differential Revision: D13051089
fbshipit-source-id: 69776642949d293c645686ba00d5d6b71c0cd31c
Summary:
The fb303 setOption() call does not return a value. Delete the code that was
assigning the result to a variable. This variable was not used, and was
always overridden below with the result from a `getOption()` call.
Reviewed By: strager
Differential Revision: D13051093
fbshipit-source-id: e86c47d56424abaa6b89fb2335b868b0eddc2c43
Summary:
The thrift APIs accept path names and commit IDs as binary data (python bytes)
rather than unicode strings. Our python code got this wrong in several
locations. It looks like mypy didn't previously flag this since mypy doesn't
actually figure out the correct type for the thrift `client` object, and
seemed to just be largely ignoring it. I plan to update the code so that mypy
can figure out out the client type correctly. Fixing these type errors is
required to make sure we won't get type errors once that is changed.
This just simply uses `encode("utf-8")` for now. In the future the path
arguments should be converted to `pathlib.Path`, which will do a slightly
smarter conversion, and avoid errors on non-UTF-8 binary data. In the
meantime, I believe that just using `encode("utf-8")` preserves what the
thrift code was doing implicitly before, and does not make handling of
non-UTF-8 data any worse than it was before.
Reviewed By: strager
Differential Revision: D13051094
fbshipit-source-id: 94cb62f3dd78b8e854a72a392fe8fdfad5ffd4cb
Summary:
Now that we have a standardized StartingGate for benchmarks, use it
everywhere we used to use folly::Baton.
(Note: this ignores all push blocking failures!)
Differential Revision: D13012244
fbshipit-source-id: 5841ab74cfa408e87d021fe5591557e79e677e5c
Summary:
eden/benchmarks/ is a good place to write external benchmarks for FUSE
and Thrift APIs.
(Note: this ignores all push blocking failures!)
Reviewed By: wez, strager
Differential Revision: D12970347
fbshipit-source-id: 81fa35897fc6e626254eea6e1ee44a8d35052261
Summary:
As we start to build out both FUSE and Thrift benchmarks, we'll want a
standard library. Introduce a benchharness and have both the thrift
sha-1 and parallel_open_close benchmarks use it.
(Note: this ignores all push blocking failures!)
Differential Revision: D12969306
fbshipit-source-id: 89c8bbcc37d53560decffb9281af4aba20345787
Summary:
Use `Path` objects and its associated APIs in most locations rather than using
plain strings.
Reviewed By: wez
Differential Revision: D13035415
fbshipit-source-id: ef3fb72e884b1f5dbc2a9bcd9f36ba1397a7c21a
Summary: Update the eden CLI tests to make pyre happy.
Reviewed By: wez
Differential Revision: D13035414
fbshipit-source-id: 39d623e430f4da34448b5477571cfa2f8b3920f2
Summary: Update the CLI code to make the pyre type checker happy.
Reviewed By: wez
Differential Revision: D13035413
fbshipit-source-id: d201f2e65667e0ce1bf4a73fbb05878e8711ad16
Summary:
This removes the logic that would completely avoid reporting most of the
integration tests if we could not run Eden.
This used to be an issue in the past before we could run Eden in our CI build
environment. To avoid noise in the test reports with hundreds of tests
reported as skipped we would simply hide these tests entirely. Our CI
environment does run all the Eden tests now, so this is no longer an issue.
I'm removing this logic now since it confuses the pyre type checker. Rather
than modifying the code to work around the problem, it seems preferable to
just remove this logic entirely. Now that the CI environment should be
running all the tests it seems like we probably would want to be notified if
something goes wrong and the tests think they cannot successfully run Eden for
some reason.
Reviewed By: wez
Differential Revision: D13035412
fbshipit-source-id: 73c0ce7fa759a1fbb9aff08ccad475b36f332646
Summary:
chadaustin noticed this as part of fixing up the ESTALE
handling. The issue is that we were using `inodeMap->lookupInode` and
assuming that it will always return one of our magical inodes. This
isn't guaranteed so it is better to resolve the inode by name from
the root, so that's what this diff does.
Reviewed By: chadaustin
Differential Revision: D12970034
fbshipit-source-id: 8207660cbc71577b276cb092d1ef19e1076b4946
Summary:
yfeldblum were talking about whether this code might make sense in
folly. That led to polishing it a bit more. The hot path is only six
instructions now. It's not any faster in a tight generateUniqueID loop
but uses only one thread_local slot and a bit less global storage.
Reviewed By: strager
Differential Revision: D12927275
fbshipit-source-id: 94a5872c61dfe9dd441f1f34fab65f93c12997d8
Summary:
In a later diff, I needed generateUniqueID to be
noexcept. folly::ThreadLocal does not guarantee that (and it allocates
the first time a thread calls get()), so use C++ thread_local
instead. Bonus: it's about half a nanosecond faster now.
Reviewed By: strager
Differential Revision: D12914625
fbshipit-source-id: 9ddbe65d0ba1d317907f821c03dea5a207a73a68
Summary:
Update the HgUI object used by hg_import_helper to always return true from the
`plain()` function, regardless of whether HGPLAIN is set in the environment or
not.
This should help ensure that this script is never affected by user-defined
configuration settings. This should also help ensure that mercurial won't try
to print progress bars or other strange things.
Reviewed By: quark-zju
Differential Revision: D13008639
fbshipit-source-id: afe581958470c4c4b89825a259c460ece4e20fe7
Summary:
My changes to command line argument parsing in D12927803 conflicted with one
of the new tests added in D10863987.
My changes made `edenfs` and `fake_edenfs` reject any trailing non-option
arguments. The new test runs `fake_edenfs` with an extra
`--commandArgumentsLogFile` argument asking it to log all non-option arguments
to a file.
This diff updates `fake_edenfs` to allow non-option arguments when
`--commandArgumentsLogFile` is specified.
Reviewed By: strager
Differential Revision: D13014079
fbshipit-source-id: 82ec5fd758716c2b66ac0738b7aacdf884f31233
Summary:
It is desirable to be able to reference the same variable
multiple times in the RHS of a config setting. This diff makes that
possible.
Reviewed By: strager
Differential Revision: D12906500
fbshipit-source-id: 4277f12105d0a0fb3dca880d3dad6b0238acedc0
Summary:
With the mononoke service being configured with proxygen
in front of the servers we need to ensure that we're correctly
setting up our http request if we want them not to fail with a
400 bad request error.
This diff allows setting mononoke.hostname to a DNS resolvable
name rather than an IP address and passing that hostname through
so that we can set it in the HTTP request.
Reviewed By: strager
Differential Revision: D12906498
fbshipit-source-id: b5aaabfd6f2f4c48d45128eaad8406e648477f75
Summary:
Move the error printing logic up into the CLI-specific `FsckCmd` class and out
of the lower-level `fsck.FilesystemChecker` class.
Reviewed By: wez
Differential Revision: D12955043
fbshipit-source-id: 4b5a1fcb791e784c736672484990b5406ee29b65
Summary:
Improve the way `eden fsck` processes its arguments. Previously it required a
single checkout path as an argument. If an `--overlay` argument was specified
it used this instead of the checkout path to find the directory to check, but
the checkout path argument was still required (but was ignored).
This changes the code to accept one or more paths as arguments. These paths
can either be the path to an Eden checkout (which does not have be currently
mounted), or the path to a checkout state directory inside the `.eden` state
directory. The `fsck` code automatically figures out what type of directory
the argument refers to and processes it correctly.
If no paths are specified `eden fsck` now automatically checks all configured
checkouts that are not currently mounted.
Reviewed By: wez
Differential Revision: D12955041
fbshipit-source-id: c37bc6752746d8ecd0f4a672d0d3b25d1c3a4fa1
Summary:
The `path` parameter to `cmd_util.find_checkout()` is optional, so make sure
the typing documents that correctly.
Also use the `CLIENTS_DIR` symbolic constant consistently in `config.py`
Reviewed By: wez
Differential Revision: D12955042
fbshipit-source-id: da8bd94dbabc2a7c0e2f9991a4513f1e6be45899
Summary:
This is the basic framework for some new tests for `eden fsck` that work by
unpacking one of the existing saved snapshot files, breaking it in various
ways, and then running `eden fsck` to fix it.
Using an existing snapshot file rather than creating a new mount point on the
fly in the test has a few advantages:
- This lets us confirm that newer versions of Eden can still correctly repair
and mount old file formats, even if we update the mount point data formats
in the future.
- Unpacking a snapshot is much faster than starting Eden, creating an hg
repository, cloning a new Eden checkout from the repo, and then unmounting
the checkout.
- The inode number allocations for the snapshot are fixed, which makes it
easier for the test code to manipulate specific inode numbers and always
know which path this refers to. If we created a mount point on the fly we
can't guarantee ahead of time which inode numbers would map to each file,
and we would need to do more work to look up this information after creating
the checkout.
These tests are pretty basic at the moment, but I plan to expand them as I
check in more of the fsck logic to repair errors.
Reviewed By: wez
Differential Revision: D12955045
fbshipit-source-id: 5d5a96cec812f8e72caf93e57bf0f1311e28aab8
Summary:
Update all of the C++ unit tests that create temporary files and directories
to use the new `facebook::eden::makeTempFile()` and
`facebook::eden::makeTempDir()` functions.
Note that this likely changes the behavior of some code paths in meaningful
ways: `/dev/shm` normally does not support `getxattr()`, and Eden's overlay
code attempts to store the SHA-1 for materialized files as using extended
attributes. This means that the tests will now typically hit the fallback
code path and will not store SHA-1 data in the overlay.
Reviewed By: chadaustin, strager
Differential Revision: D12971162
fbshipit-source-id: 6cc5eba2e04be7e9a13a30e90883feadfb78f9ce
Summary:
Add new helper files for creating temporary files and directories.
The main advantage of these functions is that they prefer creating files in
`/dev/shm` by default instead of `/tmp`. Some of the eden unit tests are
fairly I/O intensive (particularly the checkout tests, which create and
destroy many test mounts). This can be quite slow on hosts where `/tmp` is a
a spinning hard disk. Putting the temporary files in a ramdisk greatly speeds
up the tests in this case.
These test functions also default to using the prefix `eden_test.` for the
temporary file names.
This does not yet change any of the test code to use these functions. I will
do that in a subsequent diff.
Reviewed By: chadaustin, strager
Differential Revision: D12971161
fbshipit-source-id: 3f74be7a467e8080185d4d97d114288b4528755a
Summary:
This function behaves similarly to the python `os.path.expanduser`
function, except that it is restricted to expanding only information for the
current user.
Use this new function to process the hgcache path returned via the
rust config parsing code.
In the longer term I want to centralize and add accessors for
the rust config and move this stuff into EdenConfig, but for
now this is the one place that does this and it is OK to
process it this way here.
Reviewed By: strager
Differential Revision: D12988902
fbshipit-source-id: 96b10640359c3b8c0adac1e14cd42dd592023c3d
Summary:
This makes it possible to change configuration options
for the LocalStore while the server is running.
As you'll see in the next diff, our current layering makes using
the config a bit more awkward, but at least this diff doesn't
look gross :-p
This diff doesn't introduce any new functionality or configuration.
Reviewed By: strager
Differential Revision: D12949577
fbshipit-source-id: cf897ba676b9359f92865170faa42ff17329b85f
Summary:
Sometimes users accidentally run `edenfs start` or `edenfs restart` instead of
`eden start` or `eden restart`. This adds a new `--edenfs` flag to the
`edenfs` binary, and asks users if they meant to run `eden` instead if they do
not pass in this flag.
This used to be less of a problem since `edenfs` required users to also
explicitly specify several other configuration flags (like `--edenDir`).
However `edenfs` can not automatically figure out these settings, so these
flags are no longer required. Therefore `edenfs` would still try to start
normally when invoked with `edenfs restart`, since it did not require these
flags and it did not complain about unhandled command line arguments.
Reviewed By: wez
Differential Revision: D12927803
fbshipit-source-id: dbf7ce2449c391ca218652439eb68ff43c2ebd46
Summary:
When unpacking a snapshot, rewrite Eden's inode metadata table to change the
UID and GID values to the current user.
This is needed so that the current user can access files inside the mounted
checkout correctly.
Reviewed By: wez
Differential Revision: D12966640
fbshipit-source-id: eec4aba690117bf7b8f944221b31b7c7cc66fc0c
Summary: folly/Subprocess is not compatible with Windows and has broken the Windows build. It's not used so removing it.
Reviewed By: wez
Differential Revision: D12967451
fbshipit-source-id: 54d33bf6fe2ec3ede9d68eccd99e53c5eb6ed53d
Summary:
Add a script for unpacking saved snapshot files.
This unpacks the tarball and then fixes up absolute paths inside the snapshot
so it will work at the unpacked location. It also emits a small helper script
to make it easier to invoke eden for the snapshot.
Reviewed By: wez
Differential Revision: D12927842
fbshipit-source-id: f46c07a79894877842f2a78aa0924fd66e4c8969
Summary:
In a later diff, I needed to make generateUniqueID() noexcept. This
made me think that C++'s thread_local might be a little faster than
folly::ThreadLocal. So I added this benchmark.
Reviewed By: yfeldblum
Differential Revision: D12914249
fbshipit-source-id: 2c4acfff2162f66d13f456439d91df2ecb4167e3
Summary:
Thanks to some bpf tracing by strager, we traced the ESTALE response to
`d_splice_alias` and noted this comment above the implementation in the kernel:
> If a non-IS_ROOT directory is found, the filesystem is corrupt, and
> we should error out: directories can't have multiple aliases.
Well, our magic `.eden` directory is a directory with aliases and we were
seeing the error trigger on that dir. So, this diff replaces hardlinking
directories into each tree with a hardlink to a symlink in each tree!
At mount time we create `.eden/this-dir` as a symlink to `/abs/path/to/mount/.eden`
so that `readlink("/abs/path/to/mount/sub/dir/.eden/socket")` still
resolves as it did prior to this diff.
Reviewed By: strager
Differential Revision: D12954819
fbshipit-source-id: 7f3b1b53f2bd5b9c51e64055fc34110657a19110
Summary:
When giving arguments to the edenfs program, 'eden start' only strips `--` if it is the first positional argument. The position of `--` shouldn't matter as long (as it's before any options).
In other words, given the following command invocation:
$ eden start --opt-a arg-b -- --opt-c arg-d
Current behavior:
* `--opt-a` is interpreted as an option for the 'eden start' command.
* `arg-b -- --opt-c arg-d` is passed to edenfs as four arguments.
Desired behavior:
* `--opt-a` is interpreted as an option for the 'eden start' command.
* `arg-b --opt-c arg-d` is passed to edenfs as three arguments.
* The `--` argument is not passed to edenfs.
Fix 'eden start' by stripping `--` regardless of its position.
Reviewed By: chadaustin
Differential Revision: D10863987
fbshipit-source-id: 094da3f3674e775fe3e7eb8441ec95c37c34ff05
Summary:
Sandcastle has several cases where we chown the entire
repository which performs terribly on Eden. As a workaround we have a
command to do this in eden without loading all the files.
Reviewed By: chadaustin
Differential Revision: D12857956
fbshipit-source-id: 36cebcc710fbcf4e1eb265df901513cf50a227b9
Summary:
The `test_hg_clone_non_eden_repo_within_eden_repo()` test had been disabled
when running with treemanifest enabled since the treemanifest code did not
correctly detect which repositories should actually use treemanifest.
This issue appears to have since been fixed in the treemanifest code, so this
test passes now.
Reviewed By: quark-zju
Differential Revision: D12927801
fbshipit-source-id: b9e2e041f7eab5e24007888e2dba142e1f0b2251
Summary:
- Update mypy in fbcode to upstream version 0.641.
- Add/fix some type stubs
- Disabled some pieces that break mypy (T35976411 and T35973384)
This diff will likely cause some unit test failures, due to incorrect type annotations that should be fixed by the team owning the relevant code.
mathchecksout
Reviewed By: jeffmo
Differential Revision: D10414250
fbshipit-source-id: 282ea62aef6793a2f4baef78afd58ddd4ddf60f5
Summary:
I've included a missing `.via` for the mononoke tree import path. It's hard to
measure the difference here, but it is more technically correct than letting
that second stage run in an event base thread. desired.
Reviewed By: chadaustin
Differential Revision: D12896023
fbshipit-source-id: 0af0f7f94c3274a3e4b570fed0af4d4d3c783bc3
Summary:
FileInode::prefetch was entirely redundant - it queried for metadata
upon inode lookup after getattr() was called already (which requires
the blob metadata to be loaded).
Reviewed By: wez
Differential Revision: D12896473
fbshipit-source-id: 9ba5104a43860e1f22b88726b9e3e977d0b50e89
Summary:
Eden used to load materialized entries at startup. It no longer does
and this code is dead.
Reviewed By: wez
Differential Revision: D12896210
fbshipit-source-id: 398363724a661f87208cf05313e61755a451edb7
Summary:
This adds methods to enable and disable traces as well a
command to dump traces to an internal format.
Reviewed By: strager
Differential Revision: D10384073
fbshipit-source-id: 93122c6484f93cd1e1f8ae3ca7475c4ebcd5e06d
Summary:
With this the eden cli can dump tracepoints and translate
them to various formats or perform any processing
Reviewed By: chadaustin
Differential Revision: D10384072
fbshipit-source-id: 8b38e7f6b551a2bd98b3e748ba1cceafeceeec8c
Summary:
This uses the tracing infrastructure to create blocks around
the getSHA1 invocation as well as the getOrLoadChild (called for each
element of the path-walk) method.
Reviewed By: chadaustin
Differential Revision: D10384074
fbshipit-source-id: a06fbe38e8d3f35fcb248e6bd724e5572724d27d
Summary:
In order to get better insights into where time is spent on
various requests, some tracing infrastructure is helpful. This will
record tracepoints into a per-thread ring-buffer which can then be
dumped out-of-band.
The trace format is quite simple, a trace is composed of blocks with
begin and end points and each block may have a parent block. We can
then translate this format to many other formats as we desire.
Reviewed By: chadaustin, strager
Differential Revision: D10384071
fbshipit-source-id: f9e0f11521c68b96ab40d517c7a83cf89375b101
Summary:
This diff implements getBlob on top of the mercurial rust
datapack code. It adds a C++ binding on top of the rust code to
make it easier to use and hooks it up in the hg backing store.
Need to figure this out for our opensource and windows builds:
* Need to teach them how to build and link the rust code
* need to add a windows version of the methods that accept paths;
this is just a matter of adding a WCHAR version of the functions.
Reviewed By: strager
Differential Revision: D10433450
fbshipit-source-id: 45ce34fb9c383ea6018a0ca858581e0fe11ef3b5
