Summary:
I need to write a custom backend for the Session middleware. Gotham in 0.6.0 used synchronous methods in the session `Backend` trait. In 0.7.0 they modified [the trait](https://github.com/gotham-rs/gotham/blob/main/gotham/src/middleware/session/backend/mod.rs#L29) to support async definitions for `persist_session`, `read_session`, and `drop_session`.
This commit also migrates a few callsites in mononoke (edenserver and lfs server). Unit tests are passing, so probably we're good?
Reviewed By: zertosh
Differential Revision:
D42745808
Privacy Context Container: L1153662
fbshipit-source-id: 8b04c8fd55c411eb003264a71136b55007e81517
Summary:
Remove `BlobRepo::get_blobstore`, which returns a clone of the repo blobstore, and replace it with the equivalent facet methods.
In some cases, the obtained clone of the blobstore was just immediately followed by a call to `.boxed()`, which constructs a new `Arc` around the clone of the blobstore. This is a double-indirection, and we can avoid this by using the `RepoBlobstoreArc` trait instead.
Differential Revision: D42747658
fbshipit-source-id: 4941068c7ccb3e4a627f4a26e0d2905e0b54d3a7
Summary: This method returns a reference to the `RepoBlobstore`. Replace with the corresponding facet trait.
Differential Revision: D42745917
fbshipit-source-id: 2202083cc2e4ee5ede9bf2de9d7d80ab926a3cff
Summary:
Teach Repo::file_store() and tree_store() to use the gitstore for git repos.
I also had to avoid a git store tree query for empty repos since git doesn't like our null ID.
Reviewed By: quark-zju
Differential Revision: D42711084
fbshipit-source-id: 399431d1478e9bcb1ffadb08e6bca0ed54f863a3
Summary:
The built-in git config file has an %include for /etc/mercurial/git_overrides.rc. Test were sensitive to this file, which is undesirable.
Fix in the easiest way possible by truncating the static config in tests starting from the first "%include".
I first tried to fix in a general way by refsuing to load any config files in tests unless they are under $TESTTMP. That didn't pan out due to various places that do exactly that.
Reviewed By: quark-zju
Differential Revision: D42711085
fbshipit-source-id: cd36fa439dd08d20ffb362c23a6296ff662b48dc
Summary:
The mononoke_hg_sync can't deal with commits will null manifests because it
tries to fetch them and they don't exist. Let's fix it by skipping over them
Reviewed By: yancouto
Differential Revision: D42753347
fbshipit-source-id: e6f3e05a7f1e0cf9253c22c6614fd941a7c8c202
Summary:
Sapling when trying to push a commit with null manifest tries to pack it first and fails miserably to obtain it. This diffs makes it skip null manifests as those don't need to be transferred.
NOTE: I've also included comment about more general problem that I've noticed. In `shallowbundle.py` we skip over all manifests that don't exist by catching exception but this exception had changed due to rust migration. **I didn't fix that problem.**
Reviewed By: muirdm
Differential Revision: D42753348
fbshipit-source-id: 9a1ae168d09d89bd0316991af88c6801e4c2f41b
Summary: This diff fixes up few places in Mononoke that fail when hitting null manifest. This fix is by no means complete but it's all that's needed to unblock pushes of commits with null manifests.
Differential Revision: D42753351
fbshipit-source-id: 7b0df1f78e5be66e3fd89b929197597331d38e98
Summary:
If the commit without parents is also one of the roots of the repo it has null hg manifest. Many things in our infra are struggling with such commits, most recently monononoke_hg_sync job. This diff shows that the issue exists but doesn't event get to syncing commits because it fails to push those commits in the first place.
I'll use this test to show the impact of next few diffs in this stack.
Reviewed By: yancouto
Differential Revision: D42753350
fbshipit-source-id: 2995fb95b28343b85c8d08a3b7c14b2fcfa56c47
Summary:
This stack adds an API for analytics tracking in ISL.
In open source builds, this will be completely disabled. It's only internally that we want to be able to track usage to understand how ISL is used.
Reviewed By: bolinfest
Differential Revision: D42466433
fbshipit-source-id: cd5e08f37be541b62b29603581880771b5af9c20
Summary:
On ProjectedFS, concurrent invalidation with placeholders being laid on disk is
racy, and the race is unfortunately not fixable from EdenFS. The way this would
manifest is that IO done on files being invalidated would randomly fail with
"Internal Error", or "File not found" while EdenFS would succeed all the IO.
The only way to prevent it is to make sure that GC always considers the atime
of a file prior to invalidating it, and since on Windows the atime is only
updated at most once an hour, files that have been accessed less than an hour
ago must never be invalidated.
Reviewed By: kmancini
Differential Revision: D42694759
fbshipit-source-id: 5ec880b2df8dc96f05b5a060a3e15950f296ffe3
Summary:
In some cases, we want to be able to express configs that must be constrained
between a lower and higher duration. This adds support for such configs.
Reviewed By: kmancini
Differential Revision: D42694760
fbshipit-source-id: b8c48a4e5cb1a09ce4a217194f3d3ef5fd8d4662
Summary:
The isSafeForInodeAccess is used to know when the root inode is initialized and
valid, but this is used in racy context. For instance, the getMountPoints
collects all the mounts whose root inode is initialized, but then releases the
lock on the mountPoints_ list, subsequently, the EdenMount's root Inode is
accessed without checking. If the EdenMount is shutdown prior to the root Inode
being copied, the root Inode might be a nullptr, causing a NULL-dereference.
This pattern can be found in all of the Thrift entry points that play with
mounts and thus are all potentially sources of EdenFS crashes. To solve this,
we need to guarantee 2 things:
- isSafeForInodeAccess must only change state while the mountPoints_ lock is
held, specifically at shutdown time,
- The root inode must be copied while the mountPoints_ lock is held.
This diff does both. For the second one in particular EdenServer::getMount is
modified to also return the root Inode. This is done instead of introducing a new
function to prevent future bugs from seeping in that may be missed at review time.
Reviewed By: chadaustin
Differential Revision: D42563083
fbshipit-source-id: c0267277a54c425f330bbd58d1dc86ec3746502d
Summary: Add a bug button which opens a dropdown with more options. This is where we'll have a file-a-bug experience for internal users. OSS users will be redirected to open an issue on GitHub or join the discord or view the documentation.
Reviewed By: bolinfest
Differential Revision: D42732037
fbshipit-source-id: 3eae6a5e3fc8a8f84082c79aeec482d4dc3b8f07
Summary: In the next diff, we'll add a help dropdown for filing a bug. We want to show the ISL version number here. We don't have that info yet on the client, so we need to send it along. This requires the typical request+response messages like we do for other atoms.
Reviewed By: bolinfest
Differential Revision: D42732036
fbshipit-source-id: 73c03872583fb88ff15ef9f0aa14dfa65b43d6bd
Summary:
Add a component that handles clicking to copy the text content. It shows a copy icon on hover. After clicking to copy, it will show a success tooltip for a moment then auto-dismiss.
Such a component will be useful to have a consistent experience to click-to-copy text.
I think it's nice when you can just click to copy. Of course, I don't want to get in the way of good ole' selecting text + cmd-c. So that should still work here.
I added a small animation when you click to give some more tactile feedback.
I tried adding a hover tooltip to say "click to copy", but it kind of got in the way of the confirmation tooltip. Might be nice to still add that later.
Reviewed By: bolinfest
Differential Revision: D42732039
fbshipit-source-id: 2dbbb61bb72bde62878f8388c32720d86c1ccced
Summary:
Breaking this task down into a few steps:
1. remove reference of config.py in `workspace.py`, `dirsync.py`, and `templater.py`.
2. remove `config.py`.
Reviewed By: muirdm
Differential Revision: D42731038
fbshipit-source-id: 6feb4db0808f5182696e1366042a9a94a59f743b
Summary:
Breaking this task down into a few steps:
1. remove reference of config.py in `workspace.py`, `dirsync.py`, and `templater.py`.
2. remove `config.py`.
Reviewed By: muirdm
Differential Revision: D42731393
fbshipit-source-id: c0de17d4a592fbb9aba511ae5012696f57691e53
Summary: I don't see any test for `cfg.names()`. Adding test in this diff.
Reviewed By: muirdm
Differential Revision: D42731690
fbshipit-source-id: 6d92759921d0fd1c735250a438b98f3c6db4a355
Summary:
[sl] ci: fix Windows OS CI due to path length
Currently our OSS CI on Windows is failing due to some paths being too long (e.g., `C:/Users/runneradmin/.cargo/git/checkouts/fbthrift-abf000ee5c7fcc50/c757ec4/thrift/website/src/json/ref/cpp/f/struct/special/structapache_1_1thrift_1_1op_1_1detail_1_1AnyOp_3_01type_1_1cpp__type_3_01T_00_01type_1_1map_3_01KTag_00_01VTag_01_4_01_4_01_4/assign.json`)
Since the length of that path is just above the Windows max length (263 vs. 260). This diff tries to alleviate that by setting the `core.longpaths` option for the git system config.
Pull Request resolved: https://github.com/facebook/sapling/pull/500
Test Plan:
Ran GitHub actions on a personal fork of the repo.
---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/sapling/pull/500).
* __->__ https://github.com/facebook/sapling/issues/500
Reviewed By: bolinfest
Differential Revision: D42727420
Pulled By: sggutier
fbshipit-source-id: 3cf908f8e9537f20d796dcaa97292512f1172919
Summary:
https://github.com/facebook/sapling/issues/477 points out that they were seeing `Error: Webview is disposed` in their vscode extension logs.
This is not actually the underlying problem of this issue, but rather it's a benign bug where closing & re-opening the ISL webview panel doesn't dispose its subscriptions, so old panels are still getting messages which doesn't work.
The fix is for us to more carefully add subscriptions we make to the list of disposables, so closing & re-opening ISL disposes all of its old subscriptions correctly.
The bug in https://github.com/facebook/sapling/issues/477 around unreliability is probably another lurking issue. I'll dig in to see if I can figure that one out too. I think I may have seen similar behavior.
Reviewed By: bolinfest
Differential Revision: D42723625
fbshipit-source-id: 3984b5e036bf61235543bed109ab9e8f1db2e6c4
Summary:
Slices were very noisy, so I made them log less.
Plus now you can specify the concurrency of the command via CLI args. I tweaked it and saw no improvements though :(
Reviewed By: markbt
Differential Revision: D42683915
fbshipit-source-id: 5930a44c6a24fd48560512dd83a630c283c9a351
Summary:
The `slice_repository` function is used in derived data backfiller, to remove the need from loading the whole commit graph to start deriving. This is very useful and could be used in other places, so this diff extracts that behaviour and makes it more generic.
Done:
- Move to a different lib
- Deblobrepoify it
- Make it more generic, so it's not derived data specific, and can be used for anything you can say "still needs to be done on this commit", and depends on its parents being done.
I will use this library in another place on the next commit.
Reviewed By: markbt
Differential Revision: D42640268
fbshipit-source-id: 66bf297c6790dce6412c7f0a283bc0379fc125c3
Summary:
This makes `Changesets::add_many` actually batch inserts in the SQL implementation.
To do that, we can write everything at once to the underlying tables, and do 3 queries instead of one per changeset.
(It was a bit easier than I thought, since only the `csparents` table depends on `ids` from `changesets` table, so we don't need the fixup logic like on D42227090 (4a820610cc).)
Reviewed By: markbt
Differential Revision: D42637792
fbshipit-source-id: d155b2e0bfb8641996f0d110e5412b666db46bf4
Summary:
This adds tests to verify `Changesets::add_many` works as expected.
It also improves the changesets unit tests by using `paste` to make the macro simpler and adding `pretty_assertion`.
On the following diff I'll modify `add_many` and this test will make sure it continues working. I added it on this diff to prove it works with the current implementation.
Reviewed By: markbt
Differential Revision: D42635815
fbshipit-source-id: ada3aad775b8aa8565732244270b1a1a4badc18a
Summary:
Historically, we avoided running the Black autoformatter on our
fork of ghstack so that the code would be easier to diff with
upstream. Now that Black is run on upstream ghstack:
e5eca89cb3
we have no reason to exclude it from our linter.
Going forward, we should update the GitHub CI to run `black --check`
and report back so that contributors get this signal as well.
Reviewed By: zsol
Differential Revision: D42494629
fbshipit-source-id: 7f09ec46f687e56662f4f6ac477fd2fd077709d6
Summary:
Upstream ghstack uses this for `github_fake.py`:
004d330948/ghstack/github_fake.py (L261)
Because our fork of ghstack talks to GitHub in a different
way and has its own test harness such that we deleted
`github_fake.py`, we can remove `github_schema.graphql`, as well.
This simplifies things because our internal linter wants to run
Prettier over it.
Reviewed By: evangrayk
Differential Revision: D42698061
fbshipit-source-id: 602346e4e5ee48331513c5e28eafbb39f1fac71a
Summary: One of our previous commits (D42611397 (5bb56fc6a2)) introduced the ability to pick between `sl` and `git` for the tag name, but currently this is breaking or CI on macOS. This diff fixes that issue.
Reviewed By: zzl0
Differential Revision: D42719150
fbshipit-source-id: 7e8f74322dc7bfe8d022e4f65b40d130221399bb
Summary: Small refactor used for analytics later in the stack. I split this out to make the analytics diffs as small as possible.
Reviewed By: bolinfest
Differential Revision: D42610256
fbshipit-source-id: 9abe8fe0f5adc3164169e73a475a8084ee10863f
Summary:
For analytics, we want to be able to represent code review systems with a short string. Here I add a common API so we can convert the repo to a string, like
```
github:github.com/facebook/sapling
```
Reviewed By: bolinfest
Differential Revision: D42610257
fbshipit-source-id: 509532aeb431ef4dea9ac53c12ac91636de6a45c
Summary:
The implementation details are not obvious (for example, the xxhash chunk start
does not match start). Add more docs about them.
Reviewed By: muirdm
Differential Revision: D42626680
fbshipit-source-id: 2294d1cb6af859fb75a68b3618796d26b2dadd1c
Summary:
`if cfg!(debug_assertions) { ... } else { ... }` is a better form since it
always checks that the code compiles and is sometimes shorter.
Reviewed By: muirdm
Differential Revision: D42626679
fbshipit-source-id: 1165b92b7b81d33704a5b4ee6ed36c61f4250c87
Summary:
This new mode allows the user to specify a single commit that should be used
instead of public bookmarks for determining what should be derived.
This should be useful for our big repo imports.
Reviewed By: markbt
Differential Revision: D42685259
fbshipit-source-id: 0ed67c076af66228ff0f68acf01d2b783b156359
Summary: Let's test how the disconnecting from megarepo work
Reviewed By: RajivTS
Differential Revision: D42536655
fbshipit-source-id: 011f7700c45cab6905ce52a2d0e25b93777b2627
Summary: This function is shared between 3 different tests and copied. Let's unify it.
Reviewed By: RajivTS
Differential Revision: D42536656
fbshipit-source-id: 89267f259409e2621b6f4cb3d73da079ea93951f
