Summary: Implement a naive submodule merge logic that can be used by rebase.
Reviewed By: bolinfest
Differential Revision: D42184287
fbshipit-source-id: fdb8e83733aa8a295405085ac1e5ace526c3421b
Summary: This function will be used in other places.
Reviewed By: bolinfest
Differential Revision: D42185597
fbshipit-source-id: e464ab83893bfe3f3476003295c4cf4bb50ed9bc
Summary: This function will be used in other places.
Reviewed By: bolinfest
Differential Revision: D42185598
fbshipit-source-id: 7805f1fe1b86862c49af9c81805b01285a9df782
Summary: as mentioned in D42078459 (ef9830b244), we in favor of `edenscm.Result` since it's more generic and feature complete.
Reviewed By: bolinfest
Differential Revision: D42122290
fbshipit-source-id: 6750818aa8bbd3c81fd9a8e40c43f999848db949
Summary:
Currently we enabled `prmarker` extension in sapling.rs config, and also added a post pull hook for calling the command of `prmarker` extension, but in some corner cases, the hook triggers other `sl` binary, and will cause "no debugprmarker" warning message.
```
post-pull.prmarker=sl debugprmarker
```
This diff is to enable `prmarker` for all git repos. BTW: it's safe to enable this extension for non-GitHub repo, it will just exit if the repo is not a GitHub repo.
Reviewed By: quark-zju
Differential Revision: D42181509
fbshipit-source-id: 976009a570bb476194f69b99feabf7256467f320
Summary:
When there is no response for a request, the test should
print out the other requests that were known to `MockGitHubServer`.
The test debugging experience still needs to be improved,
but at least this is a start.
Reviewed By: muirdm
Differential Revision: D42143989
fbshipit-source-id: 919ce741c6ebddc0025b67aad6e25f569cddbf18
Summary:
Previously, in our Sapling port of ghstack, we were interpreting `HEAD`
from Git as the result of:
```
sl log -r 'max(descendants(.))' -T '{node}'
```
which was forcing ghstack to always operate from the tip of your feature
branch rather than where you are. This updates the code so that `HEAD`
is interpreted as `sl whereami` so you can submit only your current commit
and the commits *below* it in the stack as pull requests.
Fixes https://github.com/facebook/sapling/issues/283.
Reviewed By: muirdm
Differential Revision: D42121060
fbshipit-source-id: 9ea54c200b9d7938225a858ef2421bc5cf4b78e5
Summary:
GitHub's `gh` CLI has a number of useful subcommands under `gh pr`.
When run in a Git repo, `gh` can automatically figure out the appropriate
value to use with the `--repo` flag, though that is not the case in Sapling.
In this case, we do a small amount of plumbing to implement
`sl pr list [flags]` so that it execs the analogous `gh pr list [flags]` call.
This pattern should work well for supporting more `gh pr` commands.
Reviewed By: muirdm
Differential Revision: D42120917
fbshipit-source-id: 51bd51f46cd15fc9e4bbde631adfe24646b0efb7
Summary:
This adds a key subcommand, `sl pr pull`, that facilitates
importing a stack created by `sl pr submit` in another working copy.
This is the `sl pr` analogue to `sl ghstack checkout`.
Reviewed By: muirdm
Differential Revision: D42116136
fbshipit-source-id: 94423f6bd15ab07771bdb671994912a1c0354137
Summary: This makes it possible to amend submodule changes to fix mistakes.
Reviewed By: bolinfest
Differential Revision: D42182504
fbshipit-source-id: 2fc416a1741d427c7b8fbed2c0514c10102e4e11
Summary: As the title. This verifies the next diff actually fixes the bug.
Reviewed By: bolinfest
Differential Revision: D42182508
fbshipit-source-id: 412c7d1559a8e8c9b50479171d84ec8511e58c91
Summary: Revert related tests will be expanded.
Reviewed By: bolinfest
Differential Revision: D42182506
fbshipit-source-id: 519be2b7f5c9c99b347520a794a2656e992b6ad7
Summary:
Make drawdag flexible so it can be used to create commits with submodule
changes.
Reviewed By: bolinfest
Differential Revision: D42182507
fbshipit-source-id: 1f68daaf4c5f17ae11b231a0b6afe9748bc4a82d
Summary:
In hgmain, set the environment variable NoDefaultCurrentDirectoryInExePath to disable the default Windows behavior of including the current working directory in PATH.
This avoids security issues where sl could execute a binary from the CWD (i.e. untrusted repo's working copy) instead of the proper system binary. For example, if the repo contains a binary named "watchman", code such as `util.popen4("watchman debug-status")` would prefer the repo's "watchman" to the system watchman.
Setting NoDefaultCurrentDirectoryInExePath assumes there is no legitimate use of this behavior when running other programs from within sl. I wouldn't be surprised if _something_ is depending on this behavior, but it doesn't seem like anything _should_ depend on it.
I like this approach since it should prevent all occurrences of this issue (i.e. including Python, Rust, ISL, etc). Another approach that isn't totally mutually exclusive with this approach is to add runtime checks to make sure we aren't accidentally running programs from the working copy (but still allow the CurrentDirectoryInExePath behavior). That seems harder to implement and verify/maintain.
Reviewed By: quark-zju
Differential Revision: D42154134
fbshipit-source-id: f625bb51e470c320ac96f803b55cb2c4ab4e44ec
Summary: Since Python 3, this was a no-op on all platforms.
Reviewed By: quark-zju
Differential Revision: D42158008
fbshipit-source-id: a25065037079cb136a6e54579a5373a9791cb3f0
Summary:
In D42154134 we want to disable the default Windows CWD-is-in-$PATH behavior. However, that also disables all relative path invocation (e.g. `system("foo/bar", cwd=repo.root)` would still work on Posix systems, but fail on Windows because Windows would no longer search ".").
I looked through our subprocess usage and this was the only example I could find.
I tried to also add a tripwire warning message to help discover more/future uses that might not work with D42154134.
Reviewed By: quark-zju
Differential Revision: D42158007
fbshipit-source-id: df94502244937c01ea2f1e025833d9699574523d
Summary: I don't think we use this, and it includes a "shell:" gateway to invoke arbitrary commands.
Reviewed By: quark-zju
Differential Revision: D42158009
fbshipit-source-id: 514da6fdf9b1644db98d48fe4a9aaca4c6096515
Summary:
Before this change, the rebase command would error out on public commits when using the `-b` option and rebase backwards. For instance, if one had the following graph:
```
Y
|
X B
\ /
A
```
and both `A` and `B` were public commits, while `X` and `Y` weren't, `rebase -b Y -d X` would try to move `B` on top of `X` and complain about `B` being a public commit.
This diff makes rebasing with `-b` slightly smarter, by trying to find the appropriate bottom-most non-public commits to be rebased.
Reviewed By: quark-zju
Differential Revision: D42087621
fbshipit-source-id: bd5b5e7b6b1716f72929b405beb53906d92614f1
Summary:
Implicitly panicking methods like `TimeZone::timestamp` are deprecated as of chrono 0.4.23.
```
warning: use of deprecated associated function `chrono::TimeZone::timestamp`: use `timestamp_opt()` instead
--> buck2_common/src/cas_digest.rs:330:13
|
330 | Utc.timestamp(self.inner.expires.load(Ordering::Relaxed), 0)
| ^^^^^^^^^
warning: use of deprecated associated function `chrono::TimeZone::timestamp`: use `timestamp_opt()` instead
--> buck2_build_api/src/actions/impls/cas_artifact.rs:234:60
|
234 | let dir = re_tree_to_directory(&tree, &Utc.timestamp(0, 0))
| ^^^^^^^^^
warning: use of deprecated associated function `chrono::TimeZone::timestamp`: use `timestamp_opt()` instead
--> buck2_build_api/src/interpreter/rule_defs/context.rs:925:43
|
925 | let expires_after_timestamp = Utc.timestamp(expires_after_timestamp, 0);
| ^^^^^^^^^
```
Reviewed By: zertosh
Differential Revision: D42177451
fbshipit-source-id: fd0ba0a37b02316e3fcd67737049a53dffb94ed2
Summary:
For native "status" (and "config") commands, we now respect various flags and configs to determine if we should enable pager. Previously the only thing we checked was stdout.is_tty().
We don't support _everything_ Python does (e.g. don't support pager.ignore list of commands), but this is probably good enough for now.
Reviewed By: quark-zju
Differential Revision: D42088976
fbshipit-source-id: 171588f11f1b5c5f7f86ce55298ee9a1377610f7
Summary:
This at least indicates to the user what they are waiting for.
We should port the recrawl progress bar to Rust as well, but that is a bit more work.
Reviewed By: quark-zju
Differential Revision: D42088974
fbshipit-source-id: 2fc5e1ac2b69d8416ca6382e7131db131e7bdead
Summary:
Log the number of files returned and whether it was a fresh instance. Also, instrument the watchman query so it shows up in perftrace, and populates the watchmanquery_time field.
Note that I didn't go through the hgmetrics stuff and instead logged straight to tracing. hgmetrics currently only makes it to scuba for Python commands.
Reviewed By: quark-zju
Differential Revision: D42088975
fbshipit-source-id: 38a2a647f9e3ce9b612d8f203fc3777c259698b9
Summary:
Send sync_timeout in Rust to match what we do in Python. Not sure if this matters, but I'm trying investigate/fix p90+ Rust status being slower than Python in some situations.
I observed that Rust is faster most of the time except for some (big) spikes where it isn't, so I looked at timeouts. Note that the Rust watchman client doesn't seem to have a configurable client side timeout (which Python does have).
Reviewed By: quark-zju
Differential Revision: D42088977
fbshipit-source-id: da12dc4d2bb5df96ce9855b835aa5fa8a8509012
Summary:
8910d18fe8
updated the format of the pull request body generated
by `sl pr submit`, so this updated ReviewStack to recognize it.
Reviewed By: quark-zju
Differential Revision: D42172351
fbshipit-source-id: a1099bba468190bae640fc523ee94b16b1f7afe4
Summary:
The new logged-out view of ReviewStack
now includes a screencast, giving users a
better idea of what to expect upon logging
in to ReviewStack.
Reviewed By: quark-zju
Differential Revision: D42162604
fbshipit-source-id: b060c9f3701191dcb3dae4bbcc965b05f4f774cd
Summary:
This makes it possible to use reviewstack.dev with a
GitHub Enterprise account. Unfortunately, it does not
appear that our existing Netlify OAuth flow can be used for
GHE accounts. I believe for that to work, the GHE
admin would have to create a separate instance of
ReviewStack with its own OAuth secret and hosted
on a different domain.
Fortunately, GHE users can still try reviewstack.dev:
it's just that they have to enter a little more information manually.
It turns out that if you have already bothered to authenticate
with your GHE account via the `gh` CLI, it is straightforward
to dump the PAT you need to try out ReviewStack.
Fixes https://github.com/facebook/sapling/issues/188
Reviewed By: quark-zju
Differential Revision: D42161860
fbshipit-source-id: 2982592152d46a8bc63267489a68e36353c0d24f
Summary:
On the home page, we have a header and footer
whose size is dynamic. They should get their full
height while the remaining space should be allocated
to the main content, offering a vertical scrollbar, if necessary.
This does something more like:
```
<div class="container">
<header>Header</header>
<main>
<p>Content goes here</p>
<p>Content goes here</p>
<p>Content goes here</p>
</main>
<footer>Footer</footer>
</div>
```
with:
```
.container {
display: flex;
flex-direction: column;
min-height: 100vh;
}
header,
footer {
flex: 0 0 auto;
}
main {
flex: 1 1 auto;
overflow-y: auto;
}
```
Reviewed By: quark-zju
Differential Revision: D42161859
fbshipit-source-id: 37a3510fb96beeb917f9d937bb0848ff45720353
Summary:
As noted in the comments in the code, we cannot always load
the image for a GHE avatar because the browser may block it with CORB.
Some suggest using a proxy to fetch the avatar, but that is a non-starter
for ReviewStack. Instead, we introduce a rudimentary fallback
mechanism that uses the first letter of the username as an avatar.
Reviewed By: quark-zju
Differential Revision: D42158617
fbshipit-source-id: bf0eb63d190820c58de17cf4c3578b09bdbf65f6
Summary:
Amends the developer dialog page (i.e., the one you get when
running `yarn start` locally, which is different than what is in
prod, which is `NetlifyLoginDialog.tsx`) to make it possible
to specify the hostname for the GitHub instance you want to
work with, which defaults to github.com.
This value is stored in `localStorage` as the value
associated with `"github.hostname"`.
Note it is already the case that `localStorage.clear()` is called
when the user logs out, so we don't have to worry about that
case.
The trickier thing is ensuring that the property is persisted
correctly in the first place because we also call `localStorage.clear()`
before storing the user's PAT upon login.
This diff extends `gitHubTokenPersistence` to get things working
for now, though this is a bit racy since `github.hostname` is written
first and then `github.token` is written afterwards in the callback
to a `Promise`. Ideally, these values would be written atomically,
but that requires changing the contract of `gitHubTokenPersistence`,
which will be done in a subsequent diff.
Reviewed By: quark-zju
Differential Revision: D42156925
fbshipit-source-id: a008f948c836d94703fd0cb226ba2aea545c6b32
Summary:
API requests for consumer GitHub are done against the following endpoints:
* GraphQL `https://api.github.com/graphql`
* REST `https://api.github.com/PATH`
According to GitHub's docs, a GitHub Enterprise (GHE) instance uses
slightly different ones:
* GraphQL `https://api.HOSTNAME/graphql`
* REST `https://HOSTNAME/api/v3/PATH`
though in the one GHE instance I have access to, both URL formats
appear to work.
In order for ReviewStack to support GitHub Enterprise, we have
to ensure the hostname is parameterized appropriately throughout the app.
I ran the following to audit the `reviewstack/` folder for
hardcoded references to `github.com`, excluding references in comments or `.test.ts` files:
```
addons$ rg github.com reviewstack/src | grep -v -E '^.*\.tsx?:\s*/?\*' | grep -v -E '^.*\.tsx?:\s*//' | grep -v test.ts
reviewstack/src/PullRequestSignals.tsx: {/* https://github.com/primer/react/issues/2146 */}
reviewstack/src/GitHubProjectPage.tsx: <Link href={`https://github.com${URLFor.project(props)}`}>
reviewstack/src/github/queryGraphQL.ts:const GITHUB_GRAPHQL_ENDPOINT = 'https://api.github.com/graphql';
reviewstack/src/github/GraphQLGitHubClient.ts: const url = `https://api.github.com/repos/${encodeURIComponent(
reviewstack/src/github/GraphQLGitHubClient.ts: const url = `https://api.github.com/repos/${encodeURIComponent(
```
Fortunately, this was a very short list, so it required very few
code changes to wire up GHE support end-to-end.
Though note that this diff does not make it easy for an
end-user to change the hostname param: that will be addressed
in subsequent diffs.
Reviewed By: muirdm
Differential Revision: D42153438
fbshipit-source-id: fc68e19ea7f07c26ec686d021413e6635b298281
Summary: raw-api allows access to the underlying DashMap shards, which I need.
Reviewed By: davidbarsky
Differential Revision: D42132672
fbshipit-source-id: 22b753f3ec95c85526f96a9e26d28379b2b4179b
Summary:
Point the user to "sl configfile" instead of trying to list the config file locations statically.
Also, get rid of blurbs about per-installation config file and built in config files because they don't really apply anymore.
Reviewed By: quark-zju
Differential Revision: D42156409
fbshipit-source-id: 9fed8723d87c95f818a4fcfd323a310c4dd65060
Summary:
The BSSM optimisation is currently just used for queries to `commit_find_files` that specify basenames, not suffixes. That is because the logic for suffixes was added after I had already added but not yet landed BSSM.
This diff makes BSSM be used for that optimisation as well, as was intended for BSSM.
To do that, we traverse the sharded map to find all (key, value) pairs in which `key` has the given prefix (and that's why we store the basenames reversed). The logic for fetching keys by prefix is added on D42071060. From there we continue the search as normal.
The order of files will not be lexicographical anymore, but will be consistent between requests, so it can still be used to page requests.
Reviewed By: mitrandir77
Differential Revision: D42098314
fbshipit-source-id: 45f16be358e62be09c2542cb53e5af01a687af8e
Summary:
Add a feature that manifests can list subentries by prefix.
This will later be used in BSSM to be able to list all files with a given suffix.
Reviewed By: mitrandir77
Differential Revision: D42071060
fbshipit-source-id: 8247f818554c2a34cf523dd4faa0bb4cfffc6135
Summary:
Also make public `reqwest-middleware`, needed to build custom reqwest client (such as supporting fwdproxy) and `task-local-extensions`, used to enable reqwest middleware to troubleshoot queries.
This also need a fixup for both hyperx and opentelemetry libraries.
Reviewed By: zertosh
Differential Revision: D42113375
fbshipit-source-id: ffd3d4b1c802ba95b2edd36a82188dbc9189baed
Summary:
We call it "eden rage" everywhere else, so searching hg rages for
"edenfs rage" is always unintuitive to me. Let's rename the header to
"eden rage".
Reviewed By: quark-zju
Differential Revision: D42152264
fbshipit-source-id: 32ecb8f935aaf386c5148586fb9cb8f9e4a4089f
