Summary:
`nullptr` is typesafe. `0` and `NULL` are not. In the future, only `nullptr` will be allowed.
This diff helps us embrace the future _now_ in service of enabling `-Wzero-as-null-pointer-constant`.
Reviewed By: meyering
Differential Revision: D54163067
fbshipit-source-id: 3473ba42193c8dd3cd61b8f170d20c7279f88521
Summary:
To support better telemetry and logging in watchman we want to use Eden's components. Lets migrate and detangle the needed pieces.
This change extracts DynamicEvent from LogEvent so that it can be moved to edencommon in a later diff.
Reviewed By: kmancini
Differential Revision: D54046152
fbshipit-source-id: b50bae2b155f8640a8b319f9b0353e7f44110100
Summary:
To support better telemetry and logging in watchman we want to use Eden's components. Lets migrate and detangle the needed pieces.
This change moves Throw.h and it's related tests from eden to edencommon.
Reviewed By: genevievehelsel
Differential Revision: D54046153
fbshipit-source-id: 669d702c13e70536d9c0b58ff8ff17f826237851
Summary:
Don't include commit title in PR body
Seems redundant to include the commit title in both the PR body and title, so this strips out the title.
Had some trouble building to test this out (ran into `error[E0658]: use of unstable library feature 'stdsimd'`)
Pull Request resolved: https://github.com/facebook/sapling/pull/822
Reviewed By: zzl0
Differential Revision: D52938280
Pulled By: quark-zju
fbshipit-source-id: ffe70aa9c35125f09de5d2e0e63243af96e61236
Summary:
Fix commit message template
Fixing an issue where the commit template from `sl debugcommitmessage` would be overridden due to `Internal.getCustomDefaultCommitTemplate`. This is because the check `customTemplate?.trim() !== ''` would return `true` for undefined, leading to the template being overwritten with `undefined`.
After this fix, a template correctly loads into ISL with "Summary" and "Test Plan" by default.
Also updated the "ignore line" regex to include the trailing linebreak (optionally), to prevent a bunch of blank lines in templates.
Pull Request resolved: https://github.com/facebook/sapling/pull/821
Test Plan:
Confirmed after this fix, the default template of:
```
<Replace this line with a title. Use 1 line only, 67 chars or less>
SL: Enter commit message. Lines beginning with 'SL:' are removed.
SL: Leave message empty to abort commit.
SL: --
SL: user: Alex Coleman <alex@statsig.com>
SL: changed addons/isl-server/src/Repository.ts | 5 +++--
SL: changed addons/isl-server/src/ServerToClientAPI.ts | 2 +-
SL: 2 files changed, 4 insertions(+), 3 deletions(-)
```
was now transformed to:
```
```
And then correctly appeared in ISL:
Reviewed By: mitrandir77
Differential Revision: D52937981
fbshipit-source-id: aac4f333bc79645fb8f18e6f550006337c1addc1
Summary: The correct format of the .hg/sparse file is `%include {filter}`. This didn't show up in tests because hg only complains if you try to change the filter.
Reviewed By: fanzeyi
Differential Revision: D54134450
fbshipit-source-id: 5e3b784f5aaa7b6f232f3fed6b932fb9d1e570cb
Summary:
Default to showing blame
Blame feature has been out for a while and seems functional generally. I often get questions about how to enable this.
Seems like it should be good to just enable by default, but if that conflicts with an internal blame or something I could try and ensure we set this setting in all our repos instead
Pull Request resolved: https://github.com/facebook/sapling/pull/817
Reviewed By: evangrayk
Differential Revision: D52877062
Pulled By: quark-zju
fbshipit-source-id: a53f909d167ddb256d0514f14a8709a6e74dea2a
Summary:
Configurable branch name template for pull requests
Addressing issue:
* https://github.com/facebook/sapling/issues/642
This change adds the config option `github.pr.branch-name-template` to configure the name of the branch created when submitting pull requests, using standard Sapling templates.
The default config value is `pr{github_pull_request_number}`, to match the existing branch naming scheme.
More complex templates are possible, e.g.:
```
sl config --user github.pr.branch-name-template="prs/{sub(r'<[^>]*>|\W', '',
strip(author))}/{sub(r'\W+', '-', lower(strip(firstline(desc))))}_pr{github_pull_request_number}"
```
Then when submitting a commit with title "This changes stuff!" authored by "John Doe <jd@example.com>" as a PR, the new branch for the PR will be named: `prs/JohnDoe/this-changes-stuff_pr1234`.
Alternatively, you can choose the branch name at the time of submission with e.g.:
```
sl pr submit --config github.pr.branch-name-template=this-is-the-branch-name
```
Pull Request resolved: https://github.com/facebook/sapling/pull/772
Test Plan:
* Created a stack of simple commits in a test repo.
* Submitted stack to Github with `sapling pr submit`.
* Observed that the PR branches are of the form `pr1`, `pr2` etc. (as before).
* Ran `sl pr unlink -r .` on each commit.
* Ran the `sl config ...` line above to adjust the branch template.
* Submitted the stack again to Github, as new PRs.
* Observed that the PR branches are of the form `prs/RichardCollins/test-commit_pr3` etc..
(I also tested resubmitting/updating an existing PR with a different template, to ensure that existing branches are kept under their original name.)
Reviewed By: quark-zju
Differential Revision: D51242274
fbshipit-source-id: 0fa9cbefeffebc13ca1a9d6e8ddf00266f749350
Summary:
This was triggering frequently for me, auto-closing my ISL after I closed the sidebar and reopened it later.
This case seems irrelevant and likely indicative that the frame just isnt loaded yet, so just skipping the autoclose.
https://github.com/facebook/sapling/issues/768
Pull Request resolved: https://github.com/facebook/sapling/pull/827
Reviewed By: evangrayk
Differential Revision: D53138052
fbshipit-source-id: 1a548019652da481effeec0f054dab6947855dc4
Summary:
the intent is to test conversion to rust structured annotations for a subset of fbcode (have chosen 'fbcode/configerator/structs/scm/mononoke')
this diff relies on D53674463 (should land in conjunction with this one) where .thrift files are updated so that rust unstructured annotations are replaced by their structured equivalents.
Reviewed By: dtolnay
Differential Revision: D53676407
fbshipit-source-id: a3f71bd94354f2cf4f6400cf773c71f08b73966e
Summary:
The dependabot PRs for Javascript and Rust cannot be used as-is because our
monorepo requires "offline mirrors" for dependencies that dependabot cannot
directly update. So let's just limit the dependabot PR to 0. I guess this
does not disable security alerts which are shown separately
(https://github.com/facebook/sapling/security/dependabot).
Reviewed By: zzl0
Differential Revision: D54129988
fbshipit-source-id: b98cd903786b4a0245a17c0343733b22a1f000c6
Summary:
The backtick was accidentally including the aside, now it only encompasses the command.
Pull Request resolved: https://github.com/facebook/sapling/pull/829
Reviewed By: quark-zju
Differential Revision: D53185153
fbshipit-source-id: af8c932101d612a07dd2fdbecf716d9b86f94107
Summary: I accidentally ran Pyre for this file. Might as well add the annotations to the test functions.
Reviewed By: genevievehelsel
Differential Revision: D53840666
fbshipit-source-id: 45aa1a07521b31b51b3f598a15ee648ef3c9fd48
Summary:
Include changed on the ISL side:
- Drop monaco-editor dep.
- Migrate off `--experimental-specifier-resolution=node` which is incompatible
with the latest node.
except for the require -> import change since reviewstack is not using ESM.
Reviewed By: evangrayk
Differential Revision: D54127707
fbshipit-source-id: 1c961bd554204740d766098498fcebe63d11c3be
Summary: When adding mutations received from the server, we should add them in raw form, and not add implicit links like we would when adding them locally.
Reviewed By: quark-zju
Differential Revision: D54121327
fbshipit-source-id: fcaa1b3f3f302eb1d0541a5e98276b417039c327
Summary: As aprt of https://fburl.com/gsd/nviep5e1, migrating hooks from hardcoded and ugly config to json based configerator backed config. Also includes some additional cleanup.
Reviewed By: gustavoavena
Differential Revision: D54118068
fbshipit-source-id: fbd4cb9801c47282217b6ff764f5ec8f0814bd11
Summary:
node is dropping the `--experimental-specifier-resolution=node` support.
The new pattern requires relative imports to use `.js` suffix. Update imports
to do that. `tsc` is smart enough to look for `.ts` instead of `.js`.
Reviewed By: evangrayk
Differential Revision: D54104072
fbshipit-source-id: e0e30569dbc80b6b8ae4db17e359d068e7ff7d3d
Summary: This pattern is likely to be used in all hooks, so make it available as a method on `HookConfig`.
Reviewed By: liubov-dmitrieva
Differential Revision: D54114602
fbshipit-source-id: 3da89175a6dff28bcbf9e1726cdb5b7564b0ac2b
Summary: When adding an empty file we currently return that one new line was added because we calculate it based on the number of new lines. We also return Some(1) as the first added line instead of None. This diff special cases empty added files to properly handle this
Reviewed By: RajivTS, singhsrb
Differential Revision: D54113155
fbshipit-source-id: 7cd32f160ef2a767c6ceb29bd48365a807c5806e
Summary:
## What
Implementation of submodule changes expansion for forward syncing (i.e. small to large repo).
## Why
For more details, see T174902563 or [this doc](https://fburl.com/gdoc/xik9okw8).
## Notes
- Apologies for the big diff, but breaking this up was hard to do without straining the reviewer to go back and forth many diffs. I also synced with a couple of people offline about this and they preferred to review it this way as long as they could see the E2E flow and good integration tests.
- I tried to add lots of comments, but this is an extensive feature that can be inherently confusing, so **please ask many questions** if something sounds very confusing.
- **I'm happy to update/extend the documentation**, but at this point I need some feedback to do so.
- The integration test is probably the best way to view the feature working E2E, so I might add some diff comments there to facilitate review.
## What's next
- Debug and fix deletion in submodules and of submodules. Meaning:
- When submodule is deinitialized and deleted, its expanded directory in the large repo should be deleted as well.
- Deleting files in a submodule commits and updating the source repo to point to that commit should also delete the files in the expanded directory.
- Go over the test / edge cases in T174902563 to make sure important cases are covered (add/extend integration tests if they aren't).
- Start implementation to generate the submodules metadata file that will be needed to backsync submodule changes.
- Import whatsapp/iphone in the test repo and check that everything worked as expectedly.
Reviewed By: mitrandir77
Differential Revision: D54006211
fbshipit-source-id: 6938c428f3e92d0a1436af4a9aa10f893b975166
Summary:
disable transfer speed timeout for upload changesets
Currently, server sends the "upload_changesets" response once it is fully completed,
disable min speed transfer check to avoid premature termination of requests.
https://www.internalfb.com/code/fbsource/[b8c0216665249f8d691eaa155572372313bf2b7d]/fbcode/eden/mononoke/edenapi_service/src/handlers/commit.rs?lines=342
The timeout is more relevant to transferring data endpoints. This endpoint is not fast.
This is to fix the
```
error.HttpError: [28] Timeout was reached (Operation too slow. Less than 1500 bytes/sec transferred the last 60 seconds)
```
in S397015
We still have another timeout mentioned here: D50969586
Currently, 15 minutes, so we should manage to upload larger stacks.
Reviewed By: quark-zju
Differential Revision: D54085068
fbshipit-source-id: b9bfbcf104ceb21b70b32dc47cf65ff5d9fb3635
Summary:
All future reads to these files will error until the file metadata caches
are cleared. This diff triggers a delayed invalidation after a file is
read with the incorrect size. This makes the file accessible again.
It's possible (though it doesn't always happen) that EdenFS's internal state
could be inocorrect for the file. a future `eden doctor` run will be able
to fix it up.
Reviewed By: MichaelCuevas
Differential Revision: D52278628
fbshipit-source-id: 580746bb91d21c79f87d16d5e442843617ac779c
Summary:
Currently, EdenFS crashes if ProjFS requests to read more bytes than we have
available for a file. I am able to repro this, by blocking the read so that
it starts before an `hg checkout` that shortens file contents and completes
after.
I am making eden return an error when we don't have enough data available.
Unfortunately, it seems there is no error code that indicates to projfs it's
view is stale and make it flush it's caches and retry. All error codes seem to
be propagated to the user. These ones are the most promising to make PrjFS do
something we want: https://learn.microsoft.com/en-us/windows/win32/projfs/provider-overview#callback-return-codes
But all those still result in errors to the user. I tried a few more common
HRESULT return codes: https://learn.microsoft.com/en-us/windows/win32/seccrypto/common-hresult-values
But all errors still get propagated to the user. Invalid argument seems to be
the most accurate reflection of the issue, so I'm going with that for now.
It would be better if we could just return the amount of data that we have, and
not have to error. However, PrjFS will still think the file is the longer
length and will add null bytes. This will leave the file in an inconsistent
state and leave `hg status` dirty. This seems worse than erroring because it
causes EdenFS to corrupt the file.
Ideally we would fix this by preventing the length from getting out of sync
from the contents we should read the data from. As far as I see there are two
options, but they are both more involved:
- Failing the checkout when we try to invalidate the contents and
it fails. This would be a bit disruptive to the user as background reads
can interfere with a checkout. but it matches the what a native filesystem would
do.
- Keeping the version of the file to read in the placeholder so the version and
length are never out of sync. The file contents would then be the old contents.
This would leave the file modified and make status dirty, which is still not a
great user experience, but at least the file would be in a consistent state.
I've reached out to ProjFS about other solutions -- maybe there is or could be
a particular error that eden can send that will make ProjFS retry and
gracefully deal.
For now error is better than crash
Reviewed By: mshroyer, MichaelCuevas
Differential Revision: D50193962
fbshipit-source-id: 6ca05a4195c29996a0cd255bfd461a98f7dd5fa6
Summary:
I I wanna use some of this in an EdenHgTestCase, I can't inherit from this class
because EdenHgTestCase and EdenRepoTest are incompatible. I need to move these
methods down some of the layers, might as well put them on the widely used
EdenTestCase so that we can use them in all eden integration tests.
Reviewed By: genevievehelsel
Differential Revision: D50193866
fbshipit-source-id: da4bb8296b335939fec29f77d1abe5bf4e3ce76d
Summary:
Now that we have the ability to kill processes from the previous diff, this one adds the logic to retry if the process kill is successful.
(note: Replacing a previous diff that was giving me some rebase pain)
Reviewed By: MichaelCuevas
Differential Revision: D54070610
fbshipit-source-id: 7a085c7eccba2bf1544de55c3b5dca7a53533ef3
Summary:
```
eden/scm/saplingnative/bindings/modules/pysptui/src/lib.rs:217:49
|
217 | let mods = Modifiers::from_bits(mods).ok_or_else(|| {
| -------------------- ^^^^ expected `u16`, found `u8`
| |
| arguments to this function are incorrect
|
help: you can convert a `u8` to a `u16`
|
217 | let mods = Modifiers::from_bits(mods.into()).ok_or_else(|| {
| +++++++
```
Reviewed By: dtolnay
Differential Revision: D53616859
fbshipit-source-id: 9fec2d93015936fa20541812e985260d097cd530
Summary: replace manual parcing of response stream with fetch_single that does the same
Reviewed By: quark-zju
Differential Revision: D54064065
fbshipit-source-id: f9ac63a210e816bcf08cf8f829f2a8a33c7c4705
