Summary:
# Problem
The `eden clone` and `hg clone` command currently allow you to specify that you want to use FilteredFS, but they don't allow you to specify the filter to active after the clone completes. Instead, the user has to manually set the filter themselves by running `hg filteredfs enable {path_to_filter}`.
This is annoying for many reasons, and it prevents us from easily rolling out filters on Sandcastle/ODs.
# Solution
We should allow users to pass in the Filter they want to enable at clone time. This will allow them to skip the `hg filteredfs enable {path_to_filter}` step. Even better, we'll allow passing this filter via an Hg Config. That way we can easily enable FilteredFS + use a specific filter just by setting a config value.
# This diff
All of these should take an optional filter_path argument. This will be used by clone to set the active filter at clone time.
Reviewed By: jdelliot
Differential Revision: D53599527
fbshipit-source-id: c6573cd4886620a21471101e5cc2151e815de7f1
Summary:
# Problem
The `eden clone` and `hg clone` command currently allow you to specify that you want to use FilteredFS, but they don't allow you to specify the filter to active after the clone completes. Instead, the user has to manually set the filter themselves by running `hg filteredfs enable {path_to_filter}`.
This is annoying for many reasons, and it prevents us from easily rolling out filters on Sandcastle/ODs.
# Solution
We should allow users to pass in the Filter they want to enable at clone time. This will allow them to skip the `hg filteredfs enable {path_to_filter}` step. Even better, we'll allow passing this filter via an Hg Config. That way we can easily enable FilteredFS + use a specific filter just by setting a config value.
# This diff
Instead of making the caller create the FilterID themselves, have them pass in a filter path which will be used to create the FilterID. This will be used in the next diff since clone will now take a --filter-path argument.
Reviewed By: jdelliot
Differential Revision: D53599529
fbshipit-source-id: 3d200b817e9928180a8cfd319f39188bd41dae8f
Summary: We will log the occurrences for now so we can track how often this occurs.
Reviewed By: kmancini
Differential Revision: D53595535
fbshipit-source-id: acaf331d9eb6ed297eafd174c8141b066422f478
Summary:
# Context
The "eden du" command displays the actual storage used on disk. This is used when users want to know how much actual storage is being used by eden for purposes such as clearing up disk space.
# This Diff
This diff migrates the thrift endpoint from getScmStatus to getScmStatusV2
# Technical Details
getScmStatusV2 packages the previous values of getScmStatus into a substruct and adds some new field. This diff moves the previous values of getScmStatus into that substruct and uses default values for the fields not previously present.
Reviewed By: MichaelCuevas
Differential Revision: D53581822
fbshipit-source-id: 7fcb756d5096a2a0a53075870bae979a301f0e87
Summary:
The first time handle.exe is called is shows a popup asking the user to accept the EULA.
Passing the argument -eulaaccepted bypasses this popup.
Reviewed By: MichaelCuevas
Differential Revision: D53589443
fbshipit-source-id: 2cdb56c157a297f3107b822982074a89817335d2
Summary:
During my first test of rm I saw that if rm is passed a file (that exists) instead of a directory the message is unclear (correct, but not obvious if you don't realize you passed the wrong thing).
I think just reporting that it's a file and not a directory and still allowing to delete it is a bit better.
Reviewed By: MichaelCuevas
Differential Revision: D53551256
fbshipit-source-id: 1ec367267d53185dec5ddac69d44039e39ea743c
Summary:
# Context
The "eden du" command displays the actual storage used on disk. This is used when users want to know how much actual storage is being used by eden for purposes such as clearing up disk space.
# This Diff
This diff adds a new details section to the output that displays the individual storage usage of each redirect. This lets them
Old:
```
Summary
-------
Materialized files: 311.2 MB
Redirections: 29.9 GB
Ignored files: 1.3 MB
Backing repos: 46.0 MB
Shared space: 254.7 MB
```
New:
```
Summary
-------
Materialized files: 311.3 MB
Redirections: 29.9 GB
Ignored files: 1.3 MB
Backing repos: 49.6 MB
Shared space: 254.7 MB
Details
-------
29.9 GB: buck-out
0: fbcode/buck-out
0: fbcode/.cpplsp-buck-out
0: .pylsp-buck-out
```
# Technical Details
The detail output is formatted as
```
Storage : local path
```
Storage is sorted by size descending.
I chose to have the path after the size due to variable length pathnames making it difficult to easily compare the storage being used
# Discussion Points
1. Should folders of size 0 be listed as 0 B
2. Other folder listings are displayed as fullpaths eg.
```
Buck redirections
-----------------
/data/users/cqd/fbsource/buck-out
/data/users/cqd/fbsource/fbcode/buck-out
```
should detail also display the fullpath, eg as
```
Details
-------
29.9 GB: /data/users/cqd/fbsource/buck-out
0: /data/users/cqd/fbsource/fbcode/buck-out
0: /data/users/cqd/fbsource/fbcode/.cpplsp-buck-out
0: /data/users/cqd/fbsource/.pylsp-buck-out
```
Reviewed By: MichaelCuevas
Differential Revision: D53290943
fbshipit-source-id: 20f5e1ce32131130c2f6a9ff5bb8f8bd7a46e482
Summary:
The DC for storage should all sit in the same place, i.e.
remote_execution/cas/capacity. This fixes it.
Differential Revision:
D53535912
Privacy Context Container: L1141686
fbshipit-source-id: e9383c2513ae731918ac37e95b7a4acf183d8145
Summary:
Our upcoming compiler upgrade will require us not to have shadowed variables. Such variables have a _high_ bug rate and reduce readability, so we would like to avoid them even if the compiler was not forcing us to do so.
This codemod attempts to fix an instance of a shadowed variable. Please review with care: if it's failed the result will be a silent bug.
**What's a shadowed variable?**
Shadowed variables are variables in an inner scope with the same name as another variable in an outer scope. Having the same name for both variables might be semantically correct, but it can make the code confusing to read! It can also hide subtle bugs.
This diff fixes such an issue by renaming the variable.
- If you approve of this diff, please use the "Accept & Ship" button :-)
Reviewed By: palmje
Differential Revision: D53478938
fbshipit-source-id: 686af60cacd2c6d356f2ad6464b51712b59ed96f
Summary:
Our upcoming compiler upgrade will require us not to have shadowed variables. Such variables have a _high_ bug rate and reduce readability, so we would like to avoid them even if the compiler was not forcing us to do so.
This codemod attempts to fix an instance of a shadowed variable. Please review with care: if it's failed the result will be a silent bug.
**What's a shadowed variable?**
Shadowed variables are variables in an inner scope with the same name as another variable in an outer scope. Having the same name for both variables might be semantically correct, but it can make the code confusing to read! It can also hide subtle bugs.
This diff fixes such an issue by renaming the variable.
- If you approve of this diff, please use the "Accept & Ship" button :-)
Reviewed By: palmje
Differential Revision: D53478924
fbshipit-source-id: 619cd5814bdc1804edb6366e381f26bfe75bb56d
Summary:
Our upcoming compiler upgrade will require us not to have shadowed variables. Such variables have a _high_ bug rate and reduce readability, so we would like to avoid them even if the compiler was not forcing us to do so.
This codemod attempts to fix an instance of a shadowed variable. Please review with care: if it's failed the result will be a silent bug.
**What's a shadowed variable?**
Shadowed variables are variables in an inner scope with the same name as another variable in an outer scope. Having the same name for both variables might be semantically correct, but it can make the code confusing to read! It can also hide subtle bugs.
This diff fixes such an issue by renaming the variable.
- If you approve of this diff, please use the "Accept & Ship" button :-)
Reviewed By: palmje
Differential Revision: D53478940
fbshipit-source-id: da0e0a0043f87fa27599ec051c3c772eba0721d4
Summary:
Our upcoming compiler upgrade will require us not to have shadowed variables. Such variables have a _high_ bug rate and reduce readability, so we would like to avoid them even if the compiler was not forcing us to do so.
This codemod attempts to fix an instance of a shadowed variable. Please review with care: if it's failed the result will be a silent bug.
**What's a shadowed variable?**
Shadowed variables are variables in an inner scope with the same name as another variable in an outer scope. Having the same name for both variables might be semantically correct, but it can make the code confusing to read! It can also hide subtle bugs.
This diff fixes such an issue by renaming the variable.
- If you approve of this diff, please use the "Accept & Ship" button :-)
Reviewed By: palmje
Differential Revision: D53451014
fbshipit-source-id: aec2e35fa3b105380c6d4115543bf753ceb259ea
Summary:
Our upcoming compiler upgrade will require us not to have shadowed variables. Such variables have a _high_ bug rate and reduce readability, so we would like to avoid them even if the compiler was not forcing us to do so.
This codemod attempts to fix an instance of a shadowed variable. Please review with care: if it's failed the result will be a silent bug.
**What's a shadowed variable?**
Shadowed variables are variables in an inner scope with the same name as another variable in an outer scope. Having the same name for both variables might be semantically correct, but it can make the code confusing to read! It can also hide subtle bugs.
This diff fixes such an issue by renaming the variable.
- If you approve of this diff, please use the "Accept & Ship" button :-)
Reviewed By: palmje
Differential Revision: D53451028
fbshipit-source-id: fb39814f7e9a79349f7d780d835ac3b6476bf9a9
Summary: StringKeyedUnorderedMap is a shim whose purpose is to provide F14Map. Just use the latter.
Reviewed By: genevievehelsel
Differential Revision: D53376952
fbshipit-source-id: c61ef2fdfe611ef9adb5ffefcc72b286e5ef093d
Summary:
Changes resulting from {D53374511}. I think this was supposed to be in that diff.
It doesn't necessarily make sense to have these fbcode target names in Autocargo-generated code that gets shipped to GitHub. I can look into getting autocargo to omit these.
Reviewed By: zertosh, shayne-fletcher
Differential Revision: D53379521
fbshipit-source-id: 6377e6181b0b1e0a86f6144e208773959fda0ac0
Summary:
I came across this failure in `oss-mononoke-linux-getdeps` likely caused by {D53302058}:
> error: document output filename collision
> The lib `bonsai_globalrev_mapping_thrift` in package `bonsai_globalrev_mapping_thrift_types v0.1.0 (/data/sandcastle/temp/skycastle/fbcode_builder_getdeps/build/mononoke/source/eden/mononoke/bonsai_globalrev_mapping/if/types)` has the same name as the lib `bonsai_globalrev_mapping_thrift` in package `bonsai_globalrev_mapping_thrift v0.1.0 (/data/sandcastle/temp/skycastle/fbcode_builder_getdeps/build/mononoke/source/eden/mononoke/bonsai_globalrev_mapping/if)`.
> Only one may be documented at once since they output to the same path.
> Consider documenting only one, renaming one, or marking one with `doc = false` in Cargo.toml.
We don't need Cargo-generated docs of Thrift targets.
Reviewed By: zertosh
Differential Revision: D53339579
fbshipit-source-id: 3dc0d404777ca436e0f6de7ae7491925d709e331
Summary: Users will get the best results if they pass in `~/*` as the path argument for the apfs.util command.
Reviewed By: genevievehelsel
Differential Revision: D53326600
fbshipit-source-id: 521dcf05cae001747beaa3f117d215b33a7895db
Summary:
The Rust Thrift implementation presently compiles to 4 crates:
```lang=mermaid
flowchart TD;
foo --> foo-clients;
foo --> foo-types;
foo --> foo-services;
foo-clients --> foo-types;
foo-services --> foo-types;
```
Sadly, the one with the best name is the most useless. That top-level foo crate just contains the following:
```
pub use ::foo_types as types;
pub use ::foo_clients as client;
pub use ::foo_services as server;
pub use ::foo_types::consts;
pub use ::foo_types::errors;
pub use ::foo_types::services;
pub use ::foo_clients::mock;
pub use self::consts::*;
pub use self::errors::*;
pub use self::types::*;
```
Furthermore, it is extremely uncommon for anything to want to be both a client and a server of the *same* Thrift interface. It arises occasionally for proxies, but otherwise you are either implementing the server for a particular service, or accessing it as a client, not both.
I am interested in eliminating the useless crate, and renaming the types subcrate to the nice name.
```lang=mermaid
flowchart TD;
foo-clients --> foo;
foo-services --> foo;
```
### Implementation plan
1. This diff
2. Migrate everything that uses `:foo-rust` and needs a client to `:foo-rust-clients`, and everything that needs a server to `:foo-rust-services` -- after this point, everything still depending on `:foo-rust` only uses types
3. In the macros, atomically delete `:foo-rust` and rename `:foo-rust-types` into its place (this will be a small diff)
Reviewed By: zertosh, shayne-fletcher
Differential Revision: D53302058
fbshipit-source-id: 8d28c90f3a13e71e0cedd3ce3c1cb6fe40cffe27
Summary:
# Before:
If a Thrift library A depends on Thrift library B, all of the dependencies on B are bottlenecked through B's main library. That means building A's clients requires building B's servers, and building A's servers requires building B's clients, neither of which should be necessary.
```lang=mermaid
flowchart TD;
A --> A-clients;
A --> A-types;
A --> A-services;
A-clients --> A-types;
A-services --> A-types;
B --> B-clients;
B --> B-types;
B --> B-services;
B-clients --> B-types;
B-services --> B-types;
A --> B;
A-clients --> B;
A-services --> B;
A-types --> B;
```
# After:
A tidy lattice. Projects that implement a Thrift service can build only the servers for A and B, without also building all their clients. Projects that need Thrift clients for various services can build just the clients without also building a bunch of unused infrastructure for writing a server implementation.
```lang=mermaid
flowchart TD;
A --> A-clients;
A --> A-types;
A --> A-services;
A-clients --> A-types;
A-services --> A-types;
B --> B-clients;
B --> B-types;
B --> B-services;
B-clients --> B-types;
B-services --> B-types;
A --> B;
A-clients --> B-clients;
A-services --> B-services;
A-types --> B-types;
```
Reviewed By: shayne-fletcher
Differential Revision: D53246158
fbshipit-source-id: 37cc5cb111c39c567c69e8fb2eaf23fd940082b3
Summary: `folly::StringKeyedUnorderedMap` used to be a complicated class, but is now a simple typedef to `F14NodeMap<std::string, Mapped>`. Just use the latter.
Reviewed By: bcardosolopes, xavierd
Differential Revision: D53232021
fbshipit-source-id: 4b545d1c912dae2bbf89e31354f831d818c85167
Summary:
They still haven't released 0.5.0, but made some improvements on the path
there. This brings us back to their newest commit
To keep backwards compatibility (and fix the tests that break because of broken bc), this needs to be landed together with D53111463.
Reviewed By: dtolnay
Differential Revision: D53074256
fbshipit-source-id: 124badef3c923acda758e75f1a4437d8826d5f7c
Summary:
Update libbpf-sys to version 1.3.0+v1.3.0. This release includes libbpf in
version v1.3.0, which is what we were waiting on. Because this release contains
some incompatibilities, we also have to update libbpf-rs to the most recent
snapshot. A new libbpf-rs release will come once sufficient features have
accumulated.
Reviewed By: shayne-fletcher
Differential Revision: D53102023
fbshipit-source-id: 47b74d38b04237d4d5ba1f93fe19e83b9d2b6125
Summary:
## Problem
Having an "automatic remediation" for full storage is misleading:
1) Our remediation may not actually solve the full disk space issue
2) Our remediation doesn't actually work since the `apfs.util` command we run requires `sudo` to correctly purge all purgeable space.
## Solution
Instead of claiming that we automatically remediated the issue, let's inform the user of the potential problem, tell them how they can check if that's the *actual* problem, and then give them a remediation they can run to fix the problem. While this is strictly worse than automatically running the remediation for them, it's much better than running a remediation that doesn't actually work and claiming it fixed the problem.
Reviewed By: genevievehelsel
Differential Revision: D53200901
fbshipit-source-id: 23dc142b4c0d236d8b31bba85d0a8f98fe76aeba
Summary:
# Summary:
Keeping a vector of config options, manually deserializing/serializing them, and then using string-comparisions to check the type seems like a bad long-term trend. Let's move towards using enums for these types of things (like I introduced in D53146609).
# Solution:
This diff enum-izes (enum-erizes) the InodeCatalogType that we parse from the CheckoutConfigs.
Reviewed By: jdelliot
Differential Revision: D53151826
fbshipit-source-id: b646f92f5a308a46d37d43758535cc5dfdd58164
Summary: See title. I liked this message better than the old one.
Reviewed By: jdelliot
Differential Revision: D53151834
fbshipit-source-id: 05dad98d79572972675653ec873f497ae8ab9ee3
Summary:
# Summary:
Keeping a vector of config options, manually deserializing/serializing them, and then using string-comparisions to check the type seems like a bad long-term trend. Let's move towards using enums for these types of things (like I introduced in D53146609).
# Solution:
This diff enum-izes (enum-erizes) the MountProtocols that we parse from the CheckoutConfigs.
Reviewed By: jdelliot
Differential Revision: D53151833
fbshipit-source-id: b2e71e6a8ba451fcf46e7d2119b6be3b3ce0f4c4
Summary:
# Problem:
FilteredFS repos write FilteredRootIds to the Snapshot file so that they can track the current working copy parent, active FilterID, and parse those two pieces of information as needed.
This means that the current getSnapshot logic naively returns FilteredRootIds instead of the expected commit hashes as expected.
# Solution:
We can modify getSnapshot to properly parse the FilteredRootIds and return only the commit hash to the caller.
Reviewed By: jdelliot
Differential Revision: D53146665
fbshipit-source-id: c38980f00b977f525fba654e146e88f8e1693c43
Summary:
# Problem:
The next diff implements correct getSnapshot() logic which requires doing a comparison of the repo_type config field. Instead of comparing raw strings, we should compare enums.
# Solution:
We can parse the repo_type CheckoutConfig option as an enum to avoid string comparison for determining repo types.
This requires implementing to/from string implementations for the new RepositoryType enum. We can use the strum crate to do this for us.
We can then use the to/from string implementations to deserialize (and serialize) CheckoutConfigs.
Reviewed By: kmancini
Differential Revision: D53146609
fbshipit-source-id: 29cc702695381021b210f85f83855b84dc11534e
Summary:
I discovered a case where `eden du` passes in a FilteredFS commit hash into getScmStatus (v1). This causes `eden du` to crash.
This bug stems from the Rust version of get_snapshot() not parsing out the filter information for FilteredFS Snapshot files: https://www.internalfb.com/code/fbsource/[8496d7622d579ebf9253522ec56b35a1353c25ad]/fbcode/eden/fs/cli_rs/edenfs-client/src/checkout.rs?lines=557-612
We correctly parse out the Filter info in the Python CLI (since we have varint utils implemented), but we don't in the Rust CLI because we don't have varint encode/decode functions available.
This diff adds the utils to the Rust CLI so I can correctly parse out filter info and fix `eden du`
Reviewed By: kmancini
Differential Revision: D52928980
fbshipit-source-id: 3258acacde639adaf66e53e0c49578fbb0b1e782
Summary:
X-link: https://github.com/facebookincubator/velox/pull/8558
The attribute `[[fallthrough]]` is standard C++17. We no longer need to wrap it.
Reviewed By: Orvid
Differential Revision: D53034074
fbshipit-source-id: 19b1e8314e70f3e08882e76b93f7ded4df7ab9f2
Summary: Use the commit with the fix while we wait for a new release.
Differential Revision: D53127691
fbshipit-source-id: 6128ea1ffe00d7e8d8081d7ea5093e8bb813c04f
Summary:
make it possible to call `shlex::try_join` and remove the temporary `#[allow(deprecated)]` from buck2_wrapper_common.
buck2/Cargo.toml is restored to the state it had in b9dddef8e77c334a30071810e4ef851e8ad945bb before the workaround applied in D53087714 (which implies at this time shlex-1.3.0).
Reviewed By: ndmitchell, samkevich
Differential Revision: D53001954
fbshipit-source-id: 4267b80b5884b1a1e75c938af641e3f2656bf9a7
Summary:
Users are seeing the 10K write notification pretty frequently from
`eden doctor`, even when things are working ok.
From what I understand 10K was kinda an arbitrary number that seemed reasonable
to start with when we introduced this warning.
I'm gonna attempt to pick a better threshold.
We know that we see problems at 100K - George was able to repro slowness when
things got this high.
Looking at the percentiles p99.5 is less than the 10K in 10 minute limit.
but the p99.9 is pretty much the max - >100sK in 10 minute. We don't have
global numbers which would probaly yeild more helpful percentiles. So I looked
at the top few users.
There is only a handful of users over 100K/10 min, but generally the chart is
much dense under 50K/10min. 50K probably better to warn a reasonable amount.
Let's try 50K per 10 minutes.
Differential Revision: D53066716
fbshipit-source-id: 5f99438f83db51f6d093ecb0124616ffb05fdf21
Summary:
The nested-name lock-holder `folly::SharedMutex::ReadHolder` accepts `folly::SharedMutex const&`, i.e., as ref-to-const, and it internally does a `const_cast` to remove the `const`. This internal `const_cast` is necessary because members `folly::SharedMutex::lock_shared` and related member functions are not `const`-qualified.
This lock-holder interface is a convenience since a shared-lock on a shared-mutex instance are commonly acquired in `const`-qualified class member functions, where the shared-mutex instance is a class data member and access to it from a `const`-qualified member function implicitly has only `const`-access to the shared-mutex instance.
But `std::shared_lock` does not have this convenience. It is the canonical lock-holder for shared locks and we wish to migrate all uses of `folly::SharedMutex::ReadHolder` to `std::shared_lock`. So we must mark relevant shared-mutex instances with `mutable`.
As a small shortcut, we search for and mark all class data members of type `folly::SharedMutex` as `mutable`.
Reviewed By: brekhus, mogeb
Differential Revision: D52921279
fbshipit-source-id: 99f0fc750a975d181b35c1320c964183f93caefd
Summary:
This version is working, see integration test in following diff
It can correctly do partial glob matching so that directories won't be filtered out prematurely
However, this version still needs improvement:
1. I can probably do similar thing to Michael's promise in HgSparseFilter
2. pass u8 to rust instead of a vec of string
3. check on windows
Reviewed By: kmancini
Differential Revision: D50427596
fbshipit-source-id: b95f1c6333978ee79ad1ba1f28cac9cd64441fc5
Summary:
There is a typo in this config name. The config we use in the CLI that
determines the state of the rollout is `experimental:windows-symlinks` instead
of `experimental:windows-symlinks-enabled`. This causes warning for the users.
Confirmed we don't use the `experimental:windows-symlinks-enabled` value
anywhere. and checked that we don't use `EdenConfig.windowsSymlinksEnabled`
value anywhere in the daemon.
I'm changing the default value to false too because the cli was asking the
daemon for the config and defaulting to false when the config was not set.
This ensures that symlinks stay disable.
Reviewed By: sggutier
Differential Revision: D52891836
fbshipit-source-id: cd25f16a73a6d2e2202dc16a4bfa8e052c8240b7
Summary:
This config is only used in the CLI so the daemon doesn't know how to parse it
and shows a warning. Warning is noisy and makes users think there is something
wrong. Getting rid of the warning.
Reviewed By: genevievehelsel
Differential Revision: D52889490
fbshipit-source-id: f2525580c86c0cf78b71869eb27396ac2cdc282b
Summary: After removing HgImporter there are still a few places where the name persists. Let's fix that. Renaming the thread pool factory to SaplingRetryThreadFactory and the thread pool member to retryThreadPool_. Also updated some docs to reflect and converted from Mercurial to Sapling.
Reviewed By: mshroyer, genevievehelsel
Differential Revision: D52571338
fbshipit-source-id: 4738c522d642d7a8aa66a368440e54b144a241b3
Summary: With the removal of all HgImporter code, we no longer need to differentiate fetch_miss sources. If in the future we have other data stores that fetch from we can add this back. For now, it is just taking up extra space in our scuba table.
Reviewed By: genevievehelsel
Differential Revision: D52404111
fbshipit-source-id: 792d424d5b0459cdd0e47dad45d4bfb1298a1b3d
Summary: HgImporter is no longer in use in our codebase. We can now safely remove it.
Reviewed By: genevievehelsel
Differential Revision: D51911685
fbshipit-source-id: 239d70fb89a5db364606cb0140bc585fc5128324
Summary: We had already removed fetching trees and blobs. Now, we need to remove the last bits of HgImporter.
Reviewed By: genevievehelsel
Differential Revision: D51911205
fbshipit-source-id: 07bdaf14a62f7af951eafa7e8720f681914ba3ab
Summary: HgImporter is being removed - this change removes its support for fetching trees.
Reviewed By: kmancini
Differential Revision: D51872978
fbshipit-source-id: 5b373565c6b29ecc3dc7988695b6e4f62d9a04fb
Summary: HgImporter is being removed - this change removes its support for importing blobs.
Reviewed By: kmancini
Differential Revision: D51832464
fbshipit-source-id: 504a307e3c5cb66366f50a43362667e709512552
Summary: The comment was somewhat unclear. Expand on the soft vs hard choice and add a man page that future devs can reference to understand each option.
Reviewed By: genevievehelsel
Differential Revision: D52710081
fbshipit-source-id: 53bc2eae3799e775a8bbc5dad9bb99ca6e713632
Summary: Some users are reporting seeing symlinks enabled even with the config for explicitly disabling symlinks set to false. This was caused by the Rust CLI always defaulting this to true, regardless of anything else (which this diff fixes)
Reviewed By: kmancini
Differential Revision: D52665462
fbshipit-source-id: 3e4e60c2a7d95c863abdc3d2064a67a808fc5c45
Summary:
Adds a generalized check for network connectivity that sees whether we can
establish a TCP connection to the configured EdenAPI endpoint (not just
mononoke.edge.x2p.facebook.net).
In addition to determining whether name resolution succeeds, this updated
check:
- Confirms that we have a configured route to the endpoint. In particular, if
we're configured to use an IPv6-only endpoint but don't have an IPv6 default
route configured, this check will fail.
- Verifies that we can actually establish a TCP connection to the endpoint (so
we know we aren't firewalled off, for example).
Reviewed By: kmancini
Differential Revision: D52631801
fbshipit-source-id: 3137cc602ac6cfd577da6f467567ac907ae236b6
Summary:
Help noobs such as myself with occasional corruptions on Windows.
For me, I was getting this error when running `eden doctor`:
```
<snip>
File "C:\tools\eden\libexec\edenfsctl.real.par\eden\fs\cli\config.py", line 1320, in _read_config
File "C:\tools\eden\libexec\edenfsctl.real.par\eden\fs\cli\config.py", line 1916, in load_toml_config
eden.fs.cli.config.FileError: toml config is either missing or corrupted : Reserved escape sequence used (line 18 column 1 char 386)
```
Which is completely useless because it doesn't say which file fail to parse.
After this change:
```
<snip>
File "C:\tools\eden\libexec\edenfsctl.real.par\eden\fs\cli\config.py", line 1320, in _read_config
File "C:\tools\eden\libexec\edenfsctl.real.par\eden\fs\cli\config.py", line 1916, in load_toml_config
eden.fs.cli.config.FileError: toml config file C:\Users\liorda\.eden\clients\fbsource\config.toml is either missing or corrupted: Reserved escape sequence used (line 18 column 1 char 386)
```
The error, btw, was in this line:
```
active = [ "c:\open\fbsource\arvr\projects\xpd\projects\smials\*",]
```
Created from CodeHub with https://fburl.com/edit-in-codehub
Reviewed By: xavierd
Differential Revision: D52531754
fbshipit-source-id: 298422ff283e2bd2688b4dcb9613751a6705c48a
Summary: A newer version of colored adds support for true color terminal text coloring. The changelog only indicates "Alter Color interface to return Cow<'static, str>" besides the true color support as a "change" in the changelog and I'm not expecting people to really use this. If they do, I'll see it in Sandcastle?
Reviewed By: dtolnay
Differential Revision: D52391497
fbshipit-source-id: c08fffc171f7d5f9b75f96617aa673867ab45312
Summary: Doesn't hurt to add some more tests, and I have a change I want to add a test for, so sets up some initial tests for the semi-complex functions that read from disk from `edenfs_config_util.py`
Reviewed By: MichaelCuevas
Differential Revision: D52383458
fbshipit-source-id: a512b75219740da8bef3d009c9d8a14b84b80083
Summary: Nit to have both clis show the same global flag descriptions
Reviewed By: MichaelCuevas
Differential Revision: D52422910
fbshipit-source-id: 47027f0d4aa12aa22f45bef0d7df33bed2d4732a
Summary: For readability/clearness, group the global CLI options separately from subcommand options in `--help`
Reviewed By: MichaelCuevas
Differential Revision: D52422889
fbshipit-source-id: 13b8d9361f4d5cc0b66713053ec0208c1ed8172e
Summary:
`edenfsctl_config_manager` was previously causing cmd window to flash on Windows: https://fb.workplace.com/groups/edenfswindows/posts/1414741589441715
I attempted to use a subprocess like how we do for `edenfsctl start --if-necessary` and `CoCo`, but it was still popping up a window. I missed this instance of `subprocess`, which looks like it still causes the command window to flash.
Testing the config manager with this change gets rid of the command window flashing.
Reviewed By: MichaelCuevas
Differential Revision: D52371460
fbshipit-source-id: a1e791f122d44a7a00063b5ab33949d25fb7055a
Summary:
Pull Request resolved: https://github.com/facebook/sapling/pull/794
`-Wextra-semi` or `-Wextra-semi-stmt`
If the code compiles, this is safe to land.
Reviewed By: palmje
Differential Revision: D51995090
fbshipit-source-id: 4267224b633ab3af176f11752d33b74b59bab5b0
Summary: renderObjectID should probably show the actual FilteredObjectID even if it breaks tests. I'll update the broken tests in later diffs. For now, I'll skip them.
Reviewed By: kmancini
Differential Revision: D52302469
fbshipit-source-id: 6607c7bad95fd4e66b0701a88bfe58c3e4ad47b7
Summary:
I initially only added Filter parameters to thrift endpoints that were used by Mercurial. This is because Mercurial is the only tool that's allowed to change filters and therefore change the view of the repo.
However, this causes issues for other Thrift endpoints that accept Commit hashes or RootIds. Eden expects these passed in hashes/IDs to be FilteredBackingStore friendly, and they are usually not (instead they usually are only valid for the underlying BackingStore. In most cases these are Mercurial commit hashes).
To fix this, we assume that any RootIds/hashes we receive from non-Filtered endpoints should be transformed into a FilteredRootId/ObjectID and use whatever the last used Filter was. This allows us to pass valid IDs into the FilteredBackingStore without requiring the user to supply the filter they want to use.
In the future, we can add filter params to all thrift endpoints to allow all callers to use arbitrary filters, but we should carefully think through the implications of allowing any caller to apply any filter to the repo.
Note: another option was to apply the null filter to any IDs that were passed in by these thrift endpoints. That feels wrong to me, since most callers would want to match and get results based on the view of the repo that the user sees. Since the user will see the repo with whatever filter was last applied via checkoutRevision(), it makes sense to apply this filter to the passed in IDs.
Reviewed By: kmancini
Differential Revision: D51926554
fbshipit-source-id: b4f2190904096ca68a2679ac3c7b2d7f449ffeda
Summary: The intent of RenderObjectId and RenderRootId is that it returns a string that's "human readable" or sane for a caller to examine. Returning a FilteredObjectId or FilteredRootId isn't really the correct thing to do here (callers don't care about the internal details of the FilteredBackingStore). Instead, we should render the Object/Root Id from the underlying BackingStore since that's what most callers would expect.
Reviewed By: kmancini
Differential Revision: D51926553
fbshipit-source-id: 1e23c2b9501a75d0b817e60b6f698e3b821f2551
Summary:
Two lifetime issues:
1) We need to ensure the ParentCommit structure stays alive long enough to parse the VarInt and filterID from it.
2) The prefetchBlobs method expected the caller to preserve the lifetime of the ObjectIDs that were passed in. Since FilteredBackingStore::prefetchBlobs() transforms FilteredObjectIDs into regular ObjectIDs and then passes those into the underlying BackingStore, it needs to ensure that the regular ObjectIDs it created stay alive long enough to be used by the underlying BackingStore. This can be guaranteed with an ensure().
Note: I'd love to use a UniquePtr to guarantee the lifetime of the newly created ObjectIds, but that would require changing the signature of prefetchBlobs.
Reviewed By: kmancini
Differential Revision: D51926559
fbshipit-source-id: 45b52079f8081ee76368ae4167c29c11f5c97908
Summary:
In some cases, it may be necessary to determine what filter was last applied to the repo. This would be important for filter-unaware tools that want to issue thrift queries but don't have filter context. By default, these tools probably want to use the last applied filter, and therefore we need a way to figure out what the last applied filter was.
Luckily Eden already stores this information (indirectly) in the Snapshot file. The Snapshot file contains two things: the working copy parent RootID and the last checked out commit RootID. We can derive the last applied filter by looking at either one of these, since all FilteredBackingStore RootIDs contain filter information.
Reviewed By: kmancini
Differential Revision: D51926555
fbshipit-source-id: 818548f760046364a9abe758094d8234defc51f7
Summary: We can now share parsing logic in the CheckoutConfig layer when we're parsing the last RootID from the SNAPSHOT file.
Reviewed By: kmancini
Differential Revision: D52185706
fbshipit-source-id: f9aa8a9d4b7d349917b80da38b6eb5ec368ba227
Summary:
When running `eden info`, users probably want the commit hash WITHOUT filter information attached to it. This diff removes filter information (and therefore fixes the `eden info` test).
I can consider adding a "Filter" field to Eden info. However, this is low priority right now since I want to get other tests fixed first.
Reviewed By: kmancini
Differential Revision: D51926557
fbshipit-source-id: 98480a252816384681008e9059b479e48639f9a2
Summary:
When using FilteredBackingStores, Eden now stores FilteredRootIds in the Snapshot file. FilteredRootIds consist of the original underlying BackingStore's RootId, the length of the FilteredRootId (in varint format), and the filter that was applied to the repo during the last checkout.
To properly parse out the original underlying RootId (in most cases a Mercurial commit hash), we need to parse the varint that describes the length of the underlying RootId and parse the Snapshot file accordingly.
This diff introduces the utils that will be used to do that varint parsing logic.
Reviewed By: kmancini
Differential Revision: D51926558
fbshipit-source-id: f35ba4af4cf59e593fe86b4411d7f94cd722946d
Summary: Readdir tests previously failed because the render method tried to pass a FilteredObjectID into the underlying BackingStore's renderObjectId() method. This diff parses the underlying ObjectID out and passes that into the underlying BackingStore instead of passing in an uninterpretable FilteredObjectId.
Reviewed By: kmancini
Differential Revision: D51829836
fbshipit-source-id: 865cbf8f5e5f73bce77fd8ab32a315c307a741e9
Summary:
If a new repo was to be cloned with symlinks disabled on Windows, this would make the repo have a wrong config for symlinks.
In order for symlinks to be enabled for a working copy, they have to be enabled both in the EdenFS config for that checkout as well as having them inabled in hg by having `windowssymlinks` in `.hg/requires`.
Having symlinks enabled **in only one of the two** can cause really wonky behavior.
Some users were reporting having issues with symlinks being partially enabled on their working copies. This was caused by the backing repo having `windowssymlinks` enabled on the backing repo, since the requires is copied from there into the newly cloned repo.
This diff makes changes to that, preventing copying the requires file into a new one for both symlinks and FilteredFS, since both can be affected by this kind of behavior.
Reviewed By: MichaelCuevas
Differential Revision: D52311410
fbshipit-source-id: bca76757f1b9b6f2cafd4e024b8eafc0735e7e89
Summary:
Generated changes from {D52231478}.
NOTE: This needs to be folded into the MSDK-built autocargo binary diff. https://www.internalfb.com/intern/msdk/bump/?schedule_fbid=342556550408072
Reviewed By: zertosh
Differential Revision: D52231497
fbshipit-source-id: baadabc9dfabeb2ab0232568c2e4bf4df55e26eb
Summary:
This diff adds oncall to Buck TARGETS files in fbcode repository based on their contbuild information.
This diff was generated by this command:
./fbcode/ownership/coverage/submit_diff_with_file_oncall_mapping.sh /data/users/bayarmunkh/target_oncalls_26
see more context here: https://fb.workplace.com/groups/fbcode/posts/6551897871513663 and https://fb.workplace.com/groups/fbcode.devx/permalink/3149549092006627/
drop_conflicts
Reviewed By: markisaa
Differential Revision: D52166498
fbshipit-source-id: 5bf25a26cbd700b32bbb585d987d21137c8fc545
Summary:
`None` means "remove this dep from `Cargo.toml`". These deps are either
not autocargo managed or not actually used, so there is nothing to
remove.
Reviewed By: dtolnay
Differential Revision: D52216173
fbshipit-source-id: e94473d247f19b01be9099a50d97712144b40299
Summary:
These were missed before because they're listed in exclusions or are in
non-standard locations (e.g. `xplat/`).
Reviewed By: dtolnay
Differential Revision: D52217525
fbshipit-source-id: 0bd5a9832ead34869c46b7396521ecb364787001
Summary: There are some users reporting issues with symlinks again, so let's disable them for now. This diff essentially undoes the changes done on D51284233 without actually reverting most of the code in case we want to permanently disable them again
Reviewed By: jdelliot
Differential Revision: D52211990
fbshipit-source-id: 50f541ee984793c13c45b093377a4caeb11b7fc8
Summary:
It's a pain to add and remove extensions from our warn and blocklists in
eden doctor. It it takes a while for things to rollout since the change
is dependent on the eden package which only rolls out once a week.
Let's move these to eden configs so that we can change them with chef and
the new configerator configs.
Reviewed By: genevievehelsel
Differential Revision: D52140236
fbshipit-source-id: 737595f4fdc964b334cc3479a3a4a6369251306d
Summary: HgImporter was the only way to get the repo name. With the removal of it, we a new way. Adding that to SaplingNativeBackingStore.
Reviewed By: xavierd
Differential Revision: D51911195
fbshipit-source-id: 7fcb61d72219d4e5851b31507c53c62f68df8476
Summary: The rollout of this feature has completed, but users need to reclone. Let's update the users checkout config to set the inode catalog type to be inmemory on Windows.
Reviewed By: kmancini
Differential Revision: D52005759
fbshipit-source-id: 0e3c6948fb57496bae87e71eb25e5eb1c88979f4
Summary: This is the Sapling form of EDENSCM_LOG.
Reviewed By: kmancini
Differential Revision: D52001131
fbshipit-source-id: 5932f96d0647e064cffa6279adebbf1c16a521d6
Summary:
`-Wextra-semi` or `-Wextra-semi-stmt`
If the code compiles, this is safe to land.
Reviewed By: palmje
Differential Revision: D51995015
fbshipit-source-id: c92e3c06ae30b98afdcb204c52db8ea3a4e28729
Summary: We have counters for retry success, but not for failure. Now that failure will not always result in a fallback to HgImporter we should add them.
Reviewed By: genevievehelsel
Differential Revision: D51794512
fbshipit-source-id: 108463e55c4feb20c19f7ab602365c089930601f
Summary:
To get a full picture of the kinds of misses we have at each step adding in the backingstore misses on retry.
NOTE: we could go in a refactor HgImporter to return folly::Try<[Object]Ptr>, clean up the code a bit and unify the retry reporting here, but we are planning to remove that code soon.
Reviewed By: genevievehelsel
Differential Revision: D51784901
fbshipit-source-id: b0cb876c892ba2de077d38df49e73fa4785069b1
Summary:
The test for configuration falling back to HgImporter was placed earlier in the flow preventing using this test after falling back to backingstore. This change moves that test to between the fallback to backingstore and the actual fallback.
This is done so that we can add a GK (D51784706) to control rolling out configuration changes to stop the fallback path - the final step before removing it completly.
To ensure we only apply this new behavior to the versions of the code that include these changes, we are creating a new config setting (hgImporterFetchFallback) to replace the previous, similar settings. This will ensure that during rollout previous versions of EdenFS are not affected.
Reviewed By: genevievehelsel
Differential Revision: D51784900
fbshipit-source-id: fd3e6a160ceaaf2a125812bb42f2382324239e83
Summary: D51538112 increased the stack size on Windows for the `eden/fs/edenfs-oss` target, which is only actually used for development. We should have done the same for the non-oss target, which this diff does.
Reviewed By: jdelliot
Differential Revision: D51929793
fbshipit-source-id: 3b749584718ac3211e9a2b2e9e3a564a249575d1
Summary:
Sometimes when there are two EdenFS's attempting to start at the same time
we run into flock error. It's possible we need to take the lock earlier to
avoid two eden's from getting this far in the process. Seems like this
locking code throws instead of returning false, so maybe this error should just
be swallowed and we fall back to the "another eden is running code path".
While we sort that out let's at least give the user a more informative error
message.
Reviewed By: genevievehelsel
Differential Revision: D51903606
fbshipit-source-id: 062921a6899a9e62b7770a199d329bf233f1168e
Summary: I am planning on moving these into `edenfs.rc`, so in order to avoid the confusing parse error startup message, this adds them.
Reviewed By: jdelliot
Differential Revision: D51829423
fbshipit-source-id: 7d8a45b33aaeed92a6f81c1b8819985d9ff5d992
Summary:
SaplingNativeBackingStore "eats" errors for single object fetches. This prevents certain kinds of logging to scuba and other actions that might be taken by higher layers of code.
Eventually, we wil changes these APIs to return futures, but in the meantime, we will switch them to return folly::Try's.
The propagation goes from SaplingNativeBackingStore to HgDatapackStore to HgBackingStore and all of their callers.
Reviewed By: kmancini
Differential Revision: D51777559
fbshipit-source-id: 177e97fc12a5438abe0ba83f97aa3b1367074b3a
Summary:
Our object fetch miss reporting done via two different events: EdenApiMiss and HgImportFailure. EdenApiMiss is misleading (no pun intented) as it represents backingstore misses (local and remote). Neither of these events capture all that we want and EdenApiMiss had some wonky names.
This change fixes that, replacing both with ObjectFetchMiss. Additonally, it adds miss events for BlobMetadata.
Up next: we need to expose the failure causes from single object fetches through HgDatapackStore->SaplingNativeStore. Today they are eaten and only a pointer is returned.
Reviewed By: genevievehelsel
Differential Revision: D51733914
fbshipit-source-id: c592520da56a58a1d3e5a6198695857fe6c19451
Summary:
We were reporting EdenApiMiss events if a) there was an error and b) we had set the hg<Tree|Blob>FetchFallback flag.
We should have been reporting the even if there was an error and falling back if both conditions are true.
Reviewed By: genevievehelsel
Differential Revision: D51730692
fbshipit-source-id: e6471a706d4263321bfb4a0c56c97224c4365d5c
Summary: Users may still want to run debug commands that involve sparse profiles on EdenFS repos, so we cannot really disable sparse profiles everywhere. This diff re-enables them for all non-filteredhg EdenFS repos.
Reviewed By: MichaelCuevas
Differential Revision: D51772393
fbshipit-source-id: 30052aebcf0f16e90de50ee5a2dccc0e380e8245
Summary: I'm not sure this matters or not, but to clean up `zbgs`, replace `experimental_systemd` with a different bool config
Reviewed By: kmancini
Differential Revision: D51728414
fbshipit-source-id: 3b6c6756e84ccee16e7c8222f46f06c816616197
Summary: Changes the config source to the configerator made before. It'll be easier to maintain and change
Reviewed By: genevievehelsel
Differential Revision: D51720288
fbshipit-source-id: 7854f324c694f7850454fafddb2eb403b1c22d98
Summary:
Release notes: https://blog.rust-lang.org/2023/10/05/Rust-1.73.0.html
This release is coupled with an update of the `anyhow` and `thiserror` crates because the unstable standard library API for backtraces has changed.
Fbcode changes:
- `feature(default_free_fn)` deleted (D50300881)
- `noop_method_call` lint becomes warn-by-default (D50486032, D50516201)
- stronger `invalid_reference_casting` detection (D50488164)
- `feature(unix_chown)` has been stabilized
- `feature(provide_any)` and `std::provider` deleted
- `clippy::unwrap_or_else_default` renamed to `clippy::unwrap_or_default`
- type inference ambiguities (D51780425)
- `nu-command` build error (D51779062)
Reviewed By: AndreasBackx, shayne-fletcher
Differential Revision: D50294321
fbshipit-source-id: 0fac87f6ba072ad029f9ce41ce94ed813e855b20
Summary:
`-Wextra-semi` or `-Wextra-semi-stmt`
If the code compiles, this is safe to land.
Reviewed By: dmm-fb
Differential Revision: D51777971
fbshipit-source-id: 773e23e38af97640a244306fa4205d0f2c9e40c5
