Summary:
Revive integration tests. These had broken from a bunch of changes along the way
- fix jest config to match normal jest tests and transforms and version upgrades
- make sure tests don't import from `isl/src`
- use explitily `refresh()` to make it less flakey
- wrap cleanup in `act()` to remove warnings
- bump timeouts
- make logging output be annotated with the source: `[server]`, `[client]`, `[test]` (for messages during test or in test infra), `[c -> s]` and `[c <- s]` (for RPC messages)
- add README to explain how to write integration tests
TODO in another diff: get this working on CI
Reviewed By: zzl0
Differential Revision: D56916269
fbshipit-source-id: 4b2c997a26fb2476ae996859f7205205b769cc1f
Summary: If you click refresh, it should refresh as much data as possible. Somehow that wasn't already including merge conflicts. Make sure clicking refresh rechecks the conflict state. Most of the time, this does the cheap `.sl/merge` file check and doesn't requiring the full `sl resolve --tool internal:dumpjson`.
Reviewed By: zzl0
Differential Revision: D56916270
fbshipit-source-id: b34a472e587873b51b22044b1d0e83489c512c93
Summary:
When dequeuing items from the zelos derivation queue, first we query zelos for all nodes in the ready directory, and if the ready directory is empty, we will end up sleeping for 10 ms (current value of "scm/mononoke:derivation_worker_sleep_duration_ms") and then querying zelos again. For most repos this is excessive and puts a large load on zelos unnecessarily.
This diff instead makes use of Zelos' ability to set watches, by setting a watch on the ready queue when it's empty, and only querying zelos again when the watch returns a notification.
Differential Revision: D56939625
fbshipit-source-id: 6ac6c420b63c793ffd7b94d195f77173c4020a13
Summary:
We'll be needing it soon to support backsyncing and I'm working on fixing a bug that's partly caused by the fact that `CommitSyncRepos::new` takes submodule deps but drops it silently when creating `LargeToSmall`.
The call to `get_submodule_deps` then always returns `NotNeeded`.
This is the case because when I initially implemented this, we were under the assumption that we would only be supporting forward syncing.
Reviewed By: mitrandir77
Differential Revision: D56881703
fbshipit-source-id: bb90fbc39b25d4fadc5342dafa35c02f93b25bd0
Summary:
Small refactoring to simplify the logic in `CommitSyncRepos::new` and also reuse code later.
Also simplified an if-else block in commit_syncer
Reviewed By: mitrandir77
Differential Revision: D56826691
fbshipit-source-id: 5c24538d5c1bc5079d6cd0e88113afb11077e600
Summary:
Just implementing debug for some data types so I can use `dbg!` with these.
Also small formatting changes
Reviewed By: mitrandir77
Differential Revision: D56826269
fbshipit-source-id: 3c6cc44bda6fa3f59a5d441d6bb61a5edeb2b046
Summary:
In D56706360 I started automatically deriving fsnodes, because to expand submodules from a commit, you need the fsnodes from its parents.
But in the merge process, we currently manually backfill derived data after running the initial import.
I don't think this makes much sense, so let's automatically derive everything by default.
One note though: I don't want derivation to block the import's progress, so I'm spawning a tokio task for all the types except fsnodes.
Differential Revision: D56877240
fbshipit-source-id: c0da1998a8b73c75c88c9bc0cd08f7c3730ca3f4
Summary:
I'll try to make these small changes as I work on the codebase.
Forward syncer always uses small repo as source and large repo as target, so naming them properly will make its code easier to understand.
Differential Revision: D56817782
fbshipit-source-id: 9e0cba01755e2b08d1c2b3875711258ba41f63c1
Summary:
This Diff implements the EdenAPI Suffix Query handler.
The handler extracts the changeset from the eden context and uses it to call find_files_with_bssmv3.
The output of that function is a stream of MPaths. These are transformed into SuffixQueryResponses and returned in and async stream.
This Diff also implements several traits for CommitID to help in the functionality of above.
Reviewed By: markbt
Differential Revision: D56433194
fbshipit-source-id: 28085e6c345cb33bfa33f9cef9c9ed5708d483dd
Summary: This Diff implements a stub handler for handling suffix queries on EdenAPI. The stub handler only returns a static message. It will be implemented in a followup diff
Reviewed By: liubov-dmitrieva
Differential Revision: D56275449
fbshipit-source-id: 3234d7d1fe48744bde3e0c4c7eead75cbd482075
Summary:
This PR implements the client and py bindings for EdenAPI suffix queries.
The endpoint will be at /suffix_query
Return types are currently placeholder. They will be updated once the handler is set up
Reviewed By: liubov-dmitrieva
Differential Revision: D56275448
fbshipit-source-id: 68b0dfb9ba6ba4893a95c37b2f13d11ded9ecb34
Summary:
This diff impelemnts the type skeletons that will be used for making suffix queries to EdenAPI.
SuffixQueryRequest should be close to what the final version will look like, but SuffixQueryResponse should change when the handler is actually hooked up to the BSSM querier
Reviewed By: liubov-dmitrieva
Differential Revision: D56219419
fbshipit-source-id: 0d6cc11b0932c32bdfa7f4d1e647cadcaccc4ec0
Summary: This is done in preparation for D56766291
Reviewed By: muirdm
Differential Revision: D56828983
fbshipit-source-id: 182004749ea74b422440c221b8601c5903ad03b1
Summary:
D56592499 accidentially changed the order of things so load_dynamic never got a auth_proxy.unix_socket_path (since `self` is now empty).
Fix by generating and inserting the dynamic config later. This way, auth_proxy.unix_socket_path can be configured anywhere (except for the dynamic config).
Reviewed By: quark-zju, sggutier
Differential Revision: D56942632
fbshipit-source-id: c98e616993b03374667702552d3c07be00ffdb95
Summary: All the directories needed by the zelos derivation queue are setup when constructing the queue, so if the ready directory is missing that should be propagated as an error not just a warning.
Differential Revision: D56939624
fbshipit-source-id: 9aa6ba6541ce121ff1b12ae1071cfd762dfaed32
Summary: Using `whoami::devicename` causes issues on macOS sometimes, plus it gives us funny looking hostnames on POSIX in general, so let's drop it. Using environment variables to getting this value as well as getting the username is probably fine, so let's do that instead.
Reviewed By: mitrandir77
Differential Revision: D56724499
fbshipit-source-id: b36a08a48121cecfdb34ed33f68422d687b0346d
Summary: Don't truncate bookmarks as aggressively, it makes them hard to read
Reviewed By: jakebolam
Differential Revision: D56943465
fbshipit-source-id: e849ba75c0b1d614cf3c063f5238ee613d41715b
Summary: The current state of this code causes an infinite loop if trying to use `getStats` for the `eden stats` cli in D56453268. This diff introduces a specific helper function to extract these singular values. Also, we don't need to collect the whole of the `Stats` objects anyways to feed these two dynamic counters anyways.
Reviewed By: kmancini
Differential Revision: D56728227
fbshipit-source-id: 22523f97a6864464beed875e82fef1d99348fe2a
Summary:
getdeps can generate a windows wrapper script that can be used to run build artifacts from build directory.
In github actions for large projects we set a getdeps option delete the build dir as soon as we've successfully installed artefacts to save disk space. This option was enabled for windows in D56165825. Turns out that didn't work, this diff adds the missing conditional so that it should.
Reviewed By: vitaut
Differential Revision: D56930778
fbshipit-source-id: 0cb9ac94ef9b39f4e33af8fb91098dc0d833731b
Summary:
NOTE: If your `hg bisect` brings you here & the error you are seeing looks like ` expected Result<&PyAny, PyErr>, found Result<Bound<'_, PyAny>, PyErr>` then see these [migration notes](https://pyo3.rs/v0.21.0/migration.html#from-020-to-021) for the fix or click on `fbcode/security/ace/pyo3/authz.rs` or similar files from bellow to see what the fix is!
In order to upgrade `pyo3` to [`0.21.1`](https://github.com/PyO3/pyo3/releases/tag/v0.21.1), the following had to take place:
## [PyO3]
* Address [migration notes](https://pyo3.rs/v0.21.0/migration.html#from-020-to-021) for `Bound<'py, T>`
* Address [#3595](https://github.com/PyO3/pyo3/pull/3595) - this is done in a crude way for now since there are many call sites depending on `fbcode/dba/rust/common/service_address/py/pyo3_conversion_helper.rs` which would require a more thorough review.
* Address [#3821](https://github.com/PyO3/pyo3/pull/3821) - `pyo3-build-config` is now dependent on and used by PyO3 macros. Currently, the only thing that gets checked is `abi3` compatibility. To address that, we introduce a fixup for that automatically generate `pyo3-build-config*.txt` configuration files, given an `fbsource` Python version. We are tryin to stay as close to `pyo3-build-config*.txt` spec as possible even though only a single bool from that file is ever since there is validation for the other fields but also to future proof future changes. By also generating this file ourselves, we prevent PyO3 from attempting to locate a Python interpreter some other way which seems to start leaking into the PyO3 API as an implementation choice already.
## [PyPi + Rust]
* Upgrade `orjson` to [`3.10.1`](https://github.com/ijl/orjson/releases/tag/3.10.1)
* Added `README.md` notes for future upgrades
* Upgrade `py-polars` to [`0.20.22`](https://github.com/pola-rs/polars/releases/tag/py-0.20.22)
* Removed `py-polars` and `polars` from `target_os = "windows"`. The `third-party/pypi/polars` Python extension has only been supported for Mac and Linux for a while now so its only natural to do that on the Rust side as well. What is more, `polars-util` is bringing in `stacker = = "0.1.14"` which does not build on Windows mostly because its using a much more recent version of `libc` than we use in `third-party/rust` (see P1228807344)
* Upgrade `pydantic-core` to [`2.18.2`](https://github.com/pydantic/pydantic-core/releases/tag/v2.18.2)
* Removed old `third-party/pypi/pydantic-core` versions
* Upgrade `safetensors` to [`0.4.3`](https://github.com/huggingface/safetensors/releases/tag/v0.4.3)
* Patch `third-party/pypi/cryptography/41.0.7` to account for PyO3's [#2975](https://github.com/PyO3/pyo3/pull/2975) (`0.19.0`) where `pyo3::once_cell` was renamed to `pyo3::sync` (see D56826865)
* Upgrade `tokenizers` and `tokenizers-python` to [`0.19.1`](https://github.com/huggingface/tokenizers/releases/tag/v0.19.1)
* Removed old `third-party/pypi/tokenizers` versions
* Fixed `third-party/pypi/tokenizers/BUCK`
* Migrated `third-party/pypi/tokenizers/0.19.1/BUCK` to mirror other Python packages that bind to Rust crates e.g. `libcst`, `polars` etc.
* Removed Windows support from `third-party/pypi/tokenizers`
## [Rust]
* Upgrade `indexmap` to [`2.2.6`](https://github.com/indexmap-rs/indexmap/releases/tag/2.2.6)
* Both latest `pydantic-core` and `c2pa` depend on `serde_json > 1.0.112` which brings in `indexmap = 2.2.1`. The latter has deprecated `.take` and `.remove` on both `IndexMap` and `IndexSet` leading to a bunch of errors (see bellow), all addressed:
```bash
error: use of deprecated method `indexmap::set::IndexSet::<T, S>::take`: `take` disrupts the set order -- use `swap_take` or `shift_take` for explicit behavior.
--> fbcode/hphp/hack/src/package/types.rs:76:16
|
76 | self.0.take(value)
| ^^^^
|
= note: `-D deprecated` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(deprecated)]`
```
Reviewed By: capickett
Differential Revision: D56671179
fbshipit-source-id: 3ae69c069b7f005570c1a06d37194cf056282a18
Summary:
This test was added a few days ago in D56701219 and has been failing ever since.
In [the sandcastle logs](https://www.internalfb.com/sandcastle/workflow/1265511495293864711/actions) for oss-mononoke-linux-getdeps, we can see:
```
failures:
---- test::test_get_stats_no_max_set stdout ----
thread 'test::test_get_stats_no_max_set' panicked at common/memory/src/lib.rs:63:9:
assertion failed: stats.is_err()
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
failures:
test::test_get_stats_no_max_set
test result: FAILED. 2 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
```
Reviewed By: gustavoavena
Differential Revision: D56830459
fbshipit-source-id: 62145b8d3972d8c1b3263fb31e3559390d1f58ba
Summary:
I noticed when re-importing `whatsapp/voip` (which has ~2k tags) that even though most tags were already imported,
it took multiple seconds to iterate over each one with `generate-bookmark`.
Looking at the code, we used to upload the git tag objects every time, even if the tag
already existed.
That's at least two blobstore puts, which explains the delay.
After this change, the speed is closer to ~5-10 tags per second, as expected.
We could still optimize further by parallelizing the bookmark generation, but this diff
only fixes an obvious pessimization.
Reviewed By: YousefSalama
Differential Revision: D54684266
fbshipit-source-id: 5cddcbc7164a84f81fba184e0d83f6226d41a035
Summary:
The diff that was reverted broke one the tests. I believe CI didn't catch it because it was disabled.
Anyway this diff undoes that backout but of course fixes the test.
Reviewed By: kmancini
Differential Revision: D56884252
fbshipit-source-id: 9a54c2305a3d85793b2c673865528541d8cac43e
Summary: Undo backout (this diff wasn't the one that introduced the regression)
Reviewed By: kmancini
Differential Revision: D56883957
fbshipit-source-id: 034eead51816bf5ca6ee97fecb15fdd40e8c1fb8
Summary: (original stack was reverted completely, but this diff wasn't the one causing the problem)
Reviewed By: kmancini
Differential Revision: D56883912
fbshipit-source-id: ee12e30b15658f2b4d53b7871dc2f314c2764abb
Summary:
Was seeing hg build errors with
```
Target `fbcode//eden/scm/saplingnative/bindings/modules/pygitcompat:pygitcompat` has unused dependencies:
fbcode//eden/scm/lib/spawn-ext:spawn-ext: spawn_ext
```
Looks like this spawn-ext is not used in pygitcompat anymore (seems like it was used in a previous version of D56635196 but no longer)
Reviewed By: muirdm
Differential Revision: D56900733
fbshipit-source-id: 8d9a89d06ff1f36489074f6cb7d457825bbba67f
Summary: This diff checks the file context type with isinstance instead of explicitly passing an parameter. Another issue of passing the parameter is that, it can be "incorrect" sometimes, for example, the `workingfilectx` variable will be an `absentfilectx` for 'destination deleted & source updated' conflict.
Reviewed By: evangrayk
Differential Revision: D56865103
fbshipit-source-id: ae5b71e510eb916e2c7b32a573cb951f06511217
Summary:
The behavior of using `--debug` to disable progress bar traces back to the
initial commit of the progress bar extension: https://repo.mercurial-scm.org/hg/rev/ad104a786d35#l1.175
It does not explain why --debug should disable progress bars. My guess is that
some code path naively writes to terminal and might break the progress
rendering. But our IO implementation should not have such issues, since IO
write methods take care of clearing the progress bars.
Reviewed By: sggutier, zzl0
Differential Revision: D56887730
fbshipit-source-id: 154ab94874cad5713c0f19cc8f37e406bcc18b91
Summary:
This diff adds six new counters to count the number of fetch Tree/Blob/BlobMetadata which are successful or failed in the first try.
We already have counters for the success/failure of retry.
## Note
This doc explains all the SaplingBackingStore ODS counters/duration after this stack changes: https://docs.google.com/document/d/1o355e8JGvq3fBYFD738Jkx9tPSWzH4bmxcM8YRKMziU/edit?usp=sharing
Reviewed By: jdelliot, kmancini
Differential Revision: D56554534
fbshipit-source-id: 8082a5227b44ea3fc78c8f98bd6701fd94f0dbf4
Summary:
`SaplingBackingStoreStats::fetchTree` is the duration that is supposed to record the fetching time (in Microsecond) when a tree/root tree is fetched from BackingStore. The `SaplingBackingStore.cpp` went through multiple merging and refactoring. All those changes cause this duration to not work accurately. To find that this duration is not accurate first we have to look at how `getTree`, `getRootTree`, and `importManifestForRoot` works in `SaplingBackingStore.cpp`
## Fetching Tree flow
We have three functions in `SaplingBackingStore.cpp` that fetch Tree from BackingStore:
1- `SaplingBackingStore::getTree` : It first checks local, if it is not found prepare a request and enqueue it. Requests will be dequeued with `processTreeImportRequests` and send to BackingStore with `getTreeBatch` and for the failed one we call `retryGetTree` which it calls `retryGetTreeImpl`
processTreeImportRequests
↳ getTreeBatch
↳ retryGetTree
↳ retryGetTreeImpl
2- `getRootTree`: Based on the commitID StoreResult, it calls `importTreeManifest` or `importTreeManifestImpl`. Finaly in the `importTreeManifestImpl` the tree fetched from BackingStore, and if it is not found it will be retried by `retryGetTree` and then `retryGetTreeImpl`
getRootTree
↳importTreeManifest
↳importTreeManifestImpl
↳ retryGetTree
↳ retryGetTreeImpl
3- `importManifestForRoot`: It calls `importTreeManifestImpl` and if it is not found in the first try it goes to same direction for `retryGetTree` and then `retryGetTreeImpl`
importManifestForRoot
↳importTreeManifestImpl
↳ retryGetTree
↳ retryGetTreeImpl
## Current fetch_tree duration records
Currently, we record fetch_tree duration in `processTreeImportRequests`,
`importTreeManifestImpl`, and `retryGetTree`. This is not accurate. Because `processTreeImportRequests` already record the time in `retryGetTree`, and recording this duration again in `retryGetTree` is a duplicated data.
## This diff solution
To not duplicating fetch_tree duration records, we should remove records from `importTreeManifestImpl` and `retryGetTree`. On the other hand, to confirm that the tree fetches which are running through `getRootTree` or `importManifestForRoot` are recording correctly, this diff brings the records into these two functions.
This doc explains all the SaplingBackingStore ODS counters/duration after this stack changes: https://docs.google.com/document/d/1o355e8JGvq3fBYFD738Jkx9tPSWzH4bmxcM8YRKMziU/edit?usp=sharing
Reviewed By: jdelliot
Differential Revision: D56530480
fbshipit-source-id: 486e38c7d0fe4e9d1be77d2cf15f2752bad29657
Summary:
## Background
`SaplingBackingStore` is now sending `request` to Sapling with `RemoteOnly` or `LocalOnly`. Therefore `SaplingBackingStore` knows the source of fetching each request. We need to save this info in `ObjectFetchContext` then we can send them to `HgImportTraceEvent::finish` and `traceBus_` can publish this info in `trace hg` command.
## This diff
Now that context have the value of `FetchedSource`, we can populate it in the `HgImportTraceEvent::finish` event. It is obvious that `HgImportTraceEvent::start` and `HgImportTraceEvent::queue` events doesn't know the `FetchedSource` because it is not fetched yet.
Also, this diff modified `trace_stream.cpp` to show the FetchedSource by emoji in `trace hg` command when it runs with `--verbose` argument
```
static const auto kLocalFetchedEmoji =
reinterpret_cast<const char*>(u8"\U0001F4BB"); // 💻
static const auto kRemoteFetchedEmoji =
reinterpret_cast<const char*>(u8"\U0001F310"); // 🌐
static const auto kUnknownFetchedEmoji =
reinterpret_cast<const char*>(u8"\U0001F937"); // 🤷
```
Differential Revision: D56283112
fbshipit-source-id: 8b7d461ff283b6aba8d894f9ec809e6a974c2201
Summary:
## Background
`SaplingBackingStore` is now sending `request` to Sapling with `RemoteOnly` or `LocalOnly`. Therefore `SaplingBackingStore` knows the source of fetching each request. We need to save this info in `ObjectFetchContext` then we can send them to `HgImportTraceEvent::finish` and `traceBus_` can publish this info in `trace hg` command.
## Thid diff
Adding a new field to `ObjectFetchContext` to keep the source of fetch for each request. The source can be LOCAL, REMOTE, or UNKNOWN
Reviewed By: jdelliot
Differential Revision: D56332989
fbshipit-source-id: d684593c26f8b97276130bf3972479e6b18dabaf
Summary:
## Background
`SaplingBackingStore` is now sending `request` to Sapling with `RemoteOnly` or `LocalOnly`. Therefore `SaplingBackingStore` knows the source of fetching each request. We need to save this info in `ObjectFetchContext` then we can send them to `HgImportTraceEvent::finish` and `traceBus_` can publish this info in `trace hg` command.
## This diff
`ObjectFetchContext` is not attached to `request` and when the `request` is fulfilled we cannot update the context. This diff adds a field `context_` to the `SaplingImportRequest` class then each request can access its own context.
Reviewed By: genevievehelsel
Differential Revision: D56221059
fbshipit-source-id: 2c4f73bfa86912c3189204a3c8c01a528dab0715
Summary: Use Python-implemented `pp` command instead of external `jq` for pretty printing JSON string. It will improve performance and also support more platforms.
Reviewed By: quark-zju
Differential Revision: D52957989
fbshipit-source-id: 7c23a6034c25a829018cf43a8df877a5fe2a3a73
Summary:
there is a bunch of scripts that uses python3 to run, this means that they are at the mercy of whoever controll their path, in linux, this can be system python3, but can also be platform python (and probably it is both), and in windows in particular it abuses c:\tools\fb-python\python3.exe. fbpython is the universal way to run python at the company that chooses the right platform python, but also provides monitoring and observability.
this scripts generates jobs like
https://www.internalfb.com/sandcastle/job/18014399781484898/
that will stop working once we remove c:\tools\fb-python\python3*.exe
Reviewed By: fried
Differential Revision: D56896564
fbshipit-source-id: d911fdaf6750635adb05b096f0522603baf47bcc
Summary:
Make the tests debugruntest compatible, so we can use the Python-implemented stdlib tools, such as `prettyprint`.
Currently, it seems debugruntest has two issues:
* it does not work well with binary file
* `ls` command does not handle symlink.
Reviewed By: quark-zju
Differential Revision: D52908050
fbshipit-source-id: fd989c103e04276a1a1ba746be02299cc0ecd12a
Summary:
If you're in merge conflcits, you shouldn't do most commit actions, like uncommit / hide / split. You have to resolve your conflict first.
So the UI should hide these actions to reduce clutter and prevent taking actions that don't make sense.
Reviewed By: quark-zju
Differential Revision: D56896529
fbshipit-source-id: 4cc33a86014187e024046f5e42c7537bc9f7962e
Summary:
People have always been confused with the terminology used for the "sides" of a conflict:
- local vs incoming
- local vs other
- dest vs source
- theirs vs yours
- rebasing onto vs being rebased
~~If we make this configurable, then we can have people customize the names in the way they'd like.~~
~~This diff starts by using a local storage config to replace all these names in the UI.~~
~~In a separate diff, we'll make this state configurable.~~
~~The default is set to source and dest, but we may want to change this. Although I did find that having a very long name by default can make it a bit hard to parse.~~
~~There is one problem with this approach: this won't affect the conflict markers shown inside the file when you start resolving.
But I think this is controlled by an sl config (haven't really checked...), so we could set that config for our `rebase` operations and use whatever value you've set in the UI, so after you set it it would be used for any rebases triggered by ISL. That seems like a good way for it to work. Then it should still only impact ISL, while still listening to your preference.~~
Update:
Just consolidate where we define the labels so we can easily change them later (or make them configurable if we later decide it's the right thing to do).
update the labels to be:
```
"source - being rebased"
```
and
```
"dest - rebasing onto"
```
Reviewed By: quark-zju, zzl0
Differential Revision: D56862394
fbshipit-source-id: 7c49a3b64a4e8051eecb46b59239463c63d3c14a
Summary:
^
This is meant to simply be a pipe to the commit cloud service, I'm trying to have as little logic here as possible.
Reviewed By: liubov-dmitrieva
Differential Revision: D56737773
fbshipit-source-id: 621e27e39da6113e84efedfb24f9251a87b12fa4