Summary:
TL;DR: File invalidation after checkout is broken on NFS macOS. This proposes a
fix.
Previously, to invalidate things on NFS we opened and closed all the parent
directories of any files/directories that changed during a checkout operation.
This worked on Linux because all open calls result in some request into the
EdenFS daemon (usually a getattr I think). The returned response from this
would show that the directory had update timestamps. So the open would see the
parent directories in their updated state. This would trigger the NFS client to
clear it's caches for that directory and their recursive children to preserve
CTO. CTO or close-to-open consistency guarantees that if on process observed a
file in state A and closed the file, then another process opened that same file
, it will see the file in state A or a later state.
macOS does not seem to do CTO.
It seems that most of the "read" filesystem calls can basically be served from
cache on NFS. Only writes seem to be guaranteed to make it into eden.
So instead of using a "read" filesystem call to trigger invalidation, we need to
use a write one.
I tried opening things in write mode, but the problem is that we need to
invalidate directories (to ensure the entry information is updated) and
directories can not be opened in write mode.
I tried writing 0 bytes to the files themselves that have changed, but this
empty write is short circuited somewhere in the kernel.
The only filesystem call that can be a noop, and seems to trigger a call into
eden is chmod. We are not really working off any guarantees any more, but
it seems to work on both Linux and macOS in my testing so far and is better
than our current broken state of invalidation.
So now to invalidate we chmod parent directories that have changed with their
current mode. Note that this could get extra tricky if we were mixing updating
directory permissions with invalidating them. We would need to ensure we chmod
with the most up to date mode bits. However, because we do not update
permissions anywhere that we also invalidate (checkout does not know how to
update directory permissions) things are a little simpler.
It's important that the chmod can not complete until it seems an updated view of
the parent directory. So we have to be careful about locking here. (Although
that hasn't really changed since we switched from open to chmod.)
One thing that does change is that since chmod is technically a write, it could
cause extra materialization that would be bad for checkout and status
performance. To prevent these problems I have updated setattr to not materialize
a directory when the attribute change is a noop which it should be in our
invalidation technique unless another client modified the directory in the
meantime in which case the directory should be modified anyways. This would
mean that we could end up wiping out clients changes to permissions in the
working copy during checkout. but this matches non eden checkout behavior. So I
think this is ok.
Reviewed By: xavierd
Differential Revision: D35435764
fbshipit-source-id: 196c4995b130b595f9582204237e726e5212993f
Summary: Forgot to update these when I changed changed the signature.
Reviewed By: DurhamG
Differential Revision: D35748246
fbshipit-source-id: f1102b60a8dd3d4af8312d7c67a0895691774f6a
Summary:
In Mercurial terminology, a `hg reset` operation simply updates the working
copy parent but doesn't affect the state of the repository. A diff/checkout
operation however would compare the on-disk state of the repository with the
working copy parent. Thus EdenFS must respect this behavior.
On Linux and macOS, a reset operation merely updates some state in the
EdenMount, but since these platforms reads data from the inode hierarchy, the
reset operation doesn't affect future reads to the repository. These would
still read the data from the previously checked out revision.
On Windows however, ProjectedFS requires us to read data from the currently
checked out commit, and thus we bypass the inode hierarchy. The downside is
that a reset operation changes the RootId in the EdenMount which is used to
walk Mercurial's tree hierarchy. What this means is that a read operation
performed after a reset would read the content of the file as it is in the
reset revision. This is not the behavior that Mercurial expects.
To solve this, we need to keep track of both the last checked out revision to
be used in response to read callbacks and the last reset revision, to be used
in diff operations. To achieve this, we can use the newly introduced SNAPSHOT
file format.
Reviewed By: fanzeyi
Differential Revision: D35293079
fbshipit-source-id: 5cfae510db8bc759d51447e6c24f021c104b7353
Summary:
On macOS unlink(dir) results in a PermissionError instead of a
IsADirectoryError.
Reviewed By: DurhamG
Differential Revision: D35756194
fbshipit-source-id: bbdc828f112e600c1dcecedb5165dd3b7a8694b2
Summary:
The test was flaky because the output sometimes is in different order. The test already had `sort` command for the walker output, however the strings that were sorted included much more than just the failed types:
```
# The output has globbing, which consumes the beginning (after the "Validation failed: ") and the end of the string
Validation failed: *hg_link_node_populated* (glob)
# while the actual string looks like this:
Validation failed: {"int":{"check_fail":1,"seq":1,"time":1650377693},"normal":{"build_revision":"","build_rule":"fbcode:eden/mo
nonoke/walker:new_walker","check_type":"bonsai_phase_is_public","node_key":"changeset.blake2.2b06a8547bfe6a3ac79392aef3fa7f3f45a82
f4e0beb95c4fa2b914c34b5b215","node_type":"PhaseMapping","repo":"repo","server_hostname":"testingnamespace","session":"kX8ip3P1uUxV
58Mt","src_node_key":"changeset.blake2.2b06a8547bfe6a3ac79392aef3fa7f3f45a82f4e0beb95c4fa2b914c34b5b215","src_node_type":"Changese
t","via_node_key":"hgchangeset.sha1.26805aba1e600a82e93661149f2313866a221a7b","via_node_type":"HgChangeset","walk_type":"validate"
}}
```
So when the numbers changed in between the test runs the order of the results changed too.
Now let's sort the output properly: only the failed types.
Reviewed By: yancouto
Differential Revision: D35747592
fbshipit-source-id: eddca83bc9f1ecc202f87f0079ce2e65abd0c479
Summary:
The current UX of `eden status` isn't good. It leaks too much internal details and generates confusing error message for the user.
This diff fixes the error reporting by moving the exact detail into trace logging while the actual error is more user-friendly.
This should reduce the confusions.
Reviewed By: chadaustin
Differential Revision: D35663792
fbshipit-source-id: 0b76f9ff9ee4542cce7349ccfa6724f096e1305f
Summary: Eden's `getEntryInformation` API currently loads Inodes for all paths queried, force-materializing the path component's Inodes. This often isn't required, and eden could be fetching non-loaded data from the object store instead.
Reviewed By: chadaustin
Differential Revision: D35372760
fbshipit-source-id: e31a450a20b09249f03339dcd1aeca2eb363046e
Summary: There's no need to materialized iNodes to request the content Sha1 and/or BlobMetadata, as that can be fetched from the objectStore.
Reviewed By: chadaustin, xavierd
Differential Revision: D35467564
fbshipit-source-id: 2848f4d21725a9f5d40251fde2e0eb29ea81302e
Summary: Instead of stripping anyhow context off all errors, only strip within the concrete error handler "specific_error_handler". This way, the fallback error handler (that produces the beloved RustError) gets the top level error with all the anyhow context.
Reviewed By: DurhamG
Differential Revision: D35602091
fbshipit-source-id: 547ae7e1a2352af74817f90ab08cadb0fce65efc
Summary:
Following diff D35738033 adds another crate and reindeer generates unrelated changes.
Run
```
fbcode/common/rust/tools/reindeer/vendor
```
without any changes.
Reviewed By: dtolnay
Differential Revision: D35738034
fbshipit-source-id: 0539b8abbd694479dbe89939cb8c5ddc6272bd71
Summary: This code has enough risk of a copy paste error that it deserves a unit test.
Reviewed By: chadaustin
Differential Revision: D35161787
fbshipit-source-id: 5691d13a74a0f059dfd6a93ea2852dca8399a165
Summary:
Most hghave checks are "immutable" results. Memorize them to reduce cost.
If some checks depend on actual tests, we can special case them later to
not be memorized.
Reviewed By: DurhamG
Differential Revision: D34725137
fbshipit-source-id: f010bddb856df1950854b839aa8c311c6a40bdbd
Summary:
This allows using the hghave library without shelling out to a `hghave` binary
in tests.
Reviewed By: DurhamG
Differential Revision: D34725136
fbshipit-source-id: 1b8115a791414a68ebd2e7ec077f77bca2a0833a