Closes https://github.com/zed-industries/zed/issues/42845
Repro steps:
https://github.com/zed-industries/zed/issues/42845#issuecomment-3687413958
Initial investigation and Zed memory trace:
https://github.com/zed-industries/zed/issues/42845#issuecomment-3687877977
The PR consists of 2 commits:
*
[first](https://github.com/zed-industries/zed/pull/45669/changes/732d308c8d7e9af3649ac71ea65a9c029af820fc)
adds cosmetic fixes to remove backtraces from logs yet again and print
paths in quotes, as file descriptors may return empty paths.
It also stubs the cause if OOM in project panel: that one traversed all
worktrees in `for worktree_snapshot in visible_worktrees` and "accepted"
the one with empty paths + never called `entry_iter.advance();` in "no
file name found for the worktree" case, thus looping endlessly and
bloating the memory quite fast.
*
[second](https://github.com/zed-industries/zed/pull/45669/changes/7ebfe5da2fc6d32f3fa2d71c761f8b2ec26d945b)
adds something that resembles a fix: `fn current_path` on macOS used the
file handler to re-fetch the worktree root file path on worktree root
canonicalization failure.
What's odd, is that `libc::fcntl` returns `0` in the case when external
volume is not mounted, thus resulting in the `""` path string that is
propagated all the way up.
*
[third](https://github.com/zed-industries/zed/pull/45669/changes/1a7560cef3e9fac604124c19f46b1f9c7b91815f)
moves the fix down to the platform-related FS implementations
The "fix" now checks the only usage of this method inside `async fn
process_events` for an empty path and bails if that is the case.
I am not sure what is a better fix, but this stops any memory leaks and
given how bad the situation now, seems ok to merge for now with the
`TODO` comment for more clever people to fix properly later.
----------------
Now, when I disconnect the SMB share and reconnect it again, Zed stops
displaying any files in the project tree but the ones opened as editors.
As before, at first, when the share is unmounted, Zed fails to save any
changes because of the timeouts.
Later, when the share is re-connected, macOS Finder hangs still but Zed
starts to react on saves yet still only shows the files that are open as
editors.
The files can be edited and saved from now on.
Later, when Finder finally stops hanging and indicates that the share is
mounted fully, the rest of the file structure reappear in the project
panel, and all file saves are propagated, hence can be observed in the
share in Finder.
It feels that one good improvement to add on top is some "disconnected"
indicator that clearly shows that the file is not properly handles in
the OS.
This requires much more changes and thinking as nothing like that exists
in Zed yet, hence not done.
Release Notes:
- Fixed Zed OOM-ing when macOS file descriptors become invalid
We've been considering removing workspace-hack for a couple reasons:
- Lukas ran into a situation where its build script seemed to be causing
spurious rebuilds. This seems more likely to be a cargo bug than an
issue with workspace-hack itself (given that it has an empty build
script), but we don't necessarily want to take the time to hunt that
down right now.
- Marshall mentioned hakari interacts poorly with automated crate
updates (in our case provided by rennovate) because you'd need to have
`cargo hakari generate && cargo hakari manage-deps` after their changes
and we prefer to not have actions that make commits.
Currently removing workspace-hack causes our workspace to grow from
~1700 to ~2000 crates being built (depending on platform), which is
mainly a problem when you're building the whole workspace or running
tests across the the normal and remote binaries (which is where
feature-unification nets us the most sharing). It doesn't impact
incremental times noticeably when you're just iterating on `-p zed`, and
we'll hopefully get these savings back in the future when
rust-lang/cargo#14774 (which re-implements the functionality of hakari)
is finished.
Release Notes:
- N/A
This adds a "workspace-hack" crate, see
[mozilla's](https://hg.mozilla.org/mozilla-central/file/3a265fdc9f33e5946f0ca0a04af73acd7e6d1a39/build/workspace-hack/Cargo.toml#l7)
for a concise explanation of why this is useful. For us in practice this
means that if I were to run all the tests (`cargo nextest r
--workspace`) and then `cargo r`, all the deps from the previous cargo
command will be reused. Before this PR it would rebuild many deps due to
resolving different sets of features for them. For me this frequently
caused long rebuilds when things "should" already be cached.
To avoid manually maintaining our workspace-hack crate, we will use
[cargo hakari](https://docs.rs/cargo-hakari) to update the build files
when there's a necessary change. I've added a step to CI that checks
whether the workspace-hack crate is up to date, and instructs you to
re-run `script/update-workspace-hack` when it fails.
Finally, to make sure that people can still depend on crates in our
workspace without pulling in all the workspace deps, we use a `[patch]`
section following [hakari's
instructions](https://docs.rs/cargo-hakari/0.9.36/cargo_hakari/patch_directive/index.html)
One possible followup task would be making guppy use our
`rust-toolchain.toml` instead of having to duplicate that list in its
config, I opened an issue for that upstream: guppy-rs/guppy#481.
TODO:
- [x] Fix the extension test failure
- [x] Ensure the dev dependencies aren't being unified by Hakari into
the main dependencies
- [x] Ensure that the remote-server binary continues to not depend on
LibSSL
Release Notes:
- N/A
---------
Co-authored-by: Mikayla <mikayla@zed.dev>
Co-authored-by: Mikayla Maki <mikayla.c.maki@gmail.com>
This PR simplifies the Zed file system abstraction and implements
`Fs::watch` for linux and windows.
TODO:
- [x] Figure out why this fails to initialize the file watchers when we
have to initialize the config directory paths, but succeeds on
subsequent runs.
- [x] Fix macOS dependencies on old fsevents::Event crate
Release Notes:
- N/A
This PR moves the Clippy configuration up to the workspace level.
We're using the [`lints`
table](https://doc.rust-lang.org/cargo/reference/workspaces.html#the-lints-table)
to configure the Clippy ruleset in the workspace's `Cargo.toml`.
Each crate in the workspace now has the following in their own
`Cargo.toml` to inherit the lints from the workspace:
```toml
[lints]
workspace = true
```
This allows for configuring rust-analyzer to show Clippy lints in the
editor by using the following configuration in your Zed `settings.json`:
```json
{
"lsp": {
"rust-analyzer": {
"initialization_options": {
"check": {
"command": "clippy"
}
}
}
}
```
Release Notes:
- N/A
This PR upgrades our [`bitflags`](https://crates.io/crates/bitflags)
dependency to v2.4.2.
This also fixes an error that was seen when running `clippy`:
```
error: &-masking with zero
--> crates/fsevent/src/fsevent.rs:19:1
|
19 | / bitflags! {
20 | | #[repr(C)]
21 | | pub struct StreamFlags: u32 {
22 | | const NONE = 0x00000000;
... |
46 | | }
47 | | }
| |_^
|
= help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
= note: `#[deny(clippy::bad_bit_mask)]` on by default
= note: this error originates in the macro `__impl_bitflags` which comes from the expansion of the macro `bitflags` (in Nightly builds, run with -Z macro-backtrace for more info)
```
Fixes#8681.
Release Notes:
- N/A
This isn't exactly a great solution, but it's a step in the right
direction, and it's simple allowing us to quickly unblock linux. Without
this (or an equivalent) PR linux builds are broken.
I spent a bunch of time investigating using notify on macos, and have a
branch with that working and FakeFs updated to use notify events.
unfortunately I think this would come with some drawbacks. Primarily
that files that don't yet exist yet aren't handled as well as with using
events directly leading to some less than ideal tradeoffs.
This PR is very much a placeholder for a better cross platform solution.
Most problematically, it only fills in the portion of fsevent::Event
that is currently used, despite there being a lot more information in
the ones collected from macos. At the very least a followup PR should
hide those implementation details behind a cross platform Event type so
that if people try and access data that hasn't been translated, they
find out about it.
Release Notes:
- N/A
- [x] Fill in GPL license text.
- [x] live_kit_client depends on live_kit_server as non-dev dependency,
even though it seems to only be used for tests. Is that an issue?
Release Notes:
- N/A
