Fix(filewatcher): handle removed directories #8800 #9406

krlvi · 2024-11-07T21:40:43Z

Description

When setting up manual recursive watching on folders (as is the case under Linux), the watching thread should not exit if a directory no longer exists. This is likely to occur within build output directories and it should not be the cause for interrupting the daemon.

Likely fixes #8800 and #8491

Testing Instructions

Unit tests suite

cargo test -p turborepo-filewatch

Test building of GitBulter

(Within a Fedora Linux VM)

Prerequisites

sudo dnf check-update
sudo dnf install webkit2gtk4.1-devel \
  openssl-devel \
  curl \
  wget \
  file \
  libappindicator-gtk3-devel \
  librsvg2-devel
sudo dnf group install "C Development Tools and Libraries"
curl --proto '=https' --tlsv1.2 https://sh.rustup.rs -sSf | sh

Obtain test project sources

git clone https://github.com/gitbutlerapp/gitbutler
cd gitbutler

Replace turbo with the turbo binary built from this branch, as an example:

diff --git a/package.json b/package.json
index 394d0f8e9..8f46a08e1 100644
--- a/package.json
+++ b/package.json
@@ -11,7 +11,7 @@
                "dev:ui": "pnpm --filter @gitbutler/ui storybook",
                "dev:web": "turbo watch --filter @gitbutler/web dev",
                "dev:desktop": "pnpm tauri dev",
-               "dev:internal-tauri": "turbo watch --filter @gitbutler/desktop dev",
+               "dev:internal-tauri": "/home/parallels/src/turborepo/target/debug/turbo watch --filter @gitbutler/desktop dev",
                "package": "turbo run package",
                "test": "turbo run test --no-daemon",
                "test:watch": "pnpm --filter @gitbutler/desktop run test:watch",

Build project

pnpm install
pnpm tauri dev

vercel · 2024-11-07T21:40:52Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

6 Skipped Deployments

Name	Status	Preview	Updated (UTC)
examples-basic-web	⬜️ Ignored (Inspect)	Visit Preview	Nov 8, 2024 5:18pm
examples-designsystem-docs	⬜️ Ignored (Inspect)	Visit Preview	Nov 8, 2024 5:18pm
examples-native-web	⬜️ Ignored (Inspect)	Visit Preview	Nov 8, 2024 5:18pm
examples-svelte-web	⬜️ Ignored (Inspect)	Visit Preview	Nov 8, 2024 5:18pm
examples-tailwind-web	⬜️ Ignored (Inspect)	Visit Preview	Nov 8, 2024 5:18pm
examples-vite-web	⬜️ Ignored (Inspect)	Visit Preview	Nov 8, 2024 5:18pm

vercel · 2024-11-07T21:40:54Z

@krlvi is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

When setting up manual recursive watching on folders (as is the case under Linux), the watching thread should not exit if a directory no longer exists. This is likely to occur within build output directories and it should not be the cause for interrupting the daemon.

Bumps buffers size for package change events from 1024 to 4096. With some build tools (e.g. cargo) the number of changes to the build direcotry can be quite large resulting in processing delay. It is also likely that during build operations the cpu resources are constrained further contributing to delays in event processing, thus increasing the buffer size seems appropriate

krlvi · 2024-11-07T23:57:03Z

Hi @NicholasLYang - I noticed your comment here #8765 (comment) and also your name in git history and thought ping you on this PR for some feedback.

Background

Normally I build our project on Mac where we experience no issues, however recently had the need to build it in a Linux VM and noticed the turbo filewatcher misbehaving.
Subsequently, I noticed that other people were also struggling to build GitButler (a turbo project) on Linux (gitbutlerapp/gitbutler#5039). With some more searching I discovered this issue #8491 from another turbo project, as well as these two #8800 #8765

I did some manual testing with the projects mentioned but of course I wanted to reach out to see if there might be something I have not considered here.

Byron · 2024-11-08T06:56:39Z

crates/turborepo-lib/src/daemon/server.rs

@@ -604,7 +604,7 @@ impl proto::turbod_server::Turbod for TurboGrpcServiceInner {
            .package_changes()
            .await;

-        let (tx, rx) = mpsc::channel(1024);
+        let (tx, rx) = mpsc::channel(4096);


I just wanted to surface the description for the commit-message as a motivation for this change, which should remove any element of surprise.

Bumps buffers size for package change events from 1024 to 4096.
With some build tools (e.g. cargo) the number of changes to the build
direcotry can be quite large resulting in processing delay.

It is also likely that during build operations the cpu resources are
constrained further contributing to delays in event processing,
thus increasing the buffer size seems appropriate

ndom91 · 2024-11-08T09:38:30Z

Can confirm, this seems to have allowed me to run turbo watch on my Nix box finally! 🥳

NicholasLYang

Thanks for the contribution! Looks good to me, I'd just add a log line

NicholasLYang · 2024-11-08T16:10:41Z

crates/turborepo-filewatch/src/lib.rs

-                                        break 'outer;
+                                        match err {
+                                            WatchError::WalkDir(_) => {
+                                                // Likely the path no longer exists


Can we add a debug log here for this scenario?

Thanks for taking a look! Added a dbg log statement here f9e08fc

Adds debug logging when setting up file watching fails due to the directory no longer existing

dBianchii · 2024-11-10T21:34:24Z

Hi, I am one of those affected by the issue described here

How do I test this patch on my monorepo? Sorry to ask, as I don't know how to apply these changes to my exact repo.

krlvi · 2024-11-11T08:15:31Z

Hi, I am one of those affected by the issue described here

How do I test this patch on my monorepo? Sorry to ask, as I don't know how to apply these changes to my exact repo.

Hey! If you have things set up you could build turborepo from source https://github.com/vercel/turborepo/blob/main/CONTRIBUTING.md using this branch and then in your on own project's build setup you can replace the references to turbo with the full path to the binary built from source.

Perhaps there is a way to create a binary with CI here, but as an external contributor I don't have permission to trigger CI

krlvi requested a review from a team as a code owner November 7, 2024 21:40

krlvi requested review from tknickman and dimitropoulos November 7, 2024 21:40

turbo-orchestrator bot added needs: triage New issues get this label. Remove it after triage owned-by: turborepo labels Nov 7, 2024

krlvi force-pushed the fix-filewatcher branch from 2192491 to 3d21e39 Compare November 7, 2024 22:46

This was referenced Nov 8, 2024

discovery failed: bad grpc status code: The operation was cancelled #8491

Open

server is unavailable: channel closed gitbutlerapp/gitbutler#5039

Open

Byron reviewed Nov 8, 2024

View reviewed changes

NicholasLYang approved these changes Nov 8, 2024

View reviewed changes

feat(filewatcher): Debug logging on removed dir

f9e08fc

Adds debug logging when setting up file watching fails due to the directory no longer existing

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix(filewatcher): handle removed directories #8800 #9406

Fix(filewatcher): handle removed directories #8800 #9406

krlvi commented Nov 7, 2024 •

edited

Loading

vercel bot commented Nov 7, 2024 •

edited

Loading

vercel bot commented Nov 7, 2024

krlvi commented Nov 7, 2024 •

edited

Loading

Byron Nov 8, 2024

ndom91 commented Nov 8, 2024

NicholasLYang left a comment

NicholasLYang Nov 8, 2024

krlvi Nov 8, 2024

dBianchii commented Nov 10, 2024

krlvi commented Nov 11, 2024

Fix(filewatcher): handle removed directories #8800 #9406

Are you sure you want to change the base?

Fix(filewatcher): handle removed directories #8800 #9406

Conversation

krlvi commented Nov 7, 2024 • edited Loading

Description

Testing Instructions

Unit tests suite

Test building of GitBulter

vercel bot commented Nov 7, 2024 • edited Loading

vercel bot commented Nov 7, 2024

krlvi commented Nov 7, 2024 • edited Loading

Background

Byron Nov 8, 2024

Choose a reason for hiding this comment

ndom91 commented Nov 8, 2024

NicholasLYang left a comment

Choose a reason for hiding this comment

NicholasLYang Nov 8, 2024

Choose a reason for hiding this comment

krlvi Nov 8, 2024

Choose a reason for hiding this comment

dBianchii commented Nov 10, 2024

krlvi commented Nov 11, 2024

krlvi commented Nov 7, 2024 •

edited

Loading

vercel bot commented Nov 7, 2024 •

edited

Loading

krlvi commented Nov 7, 2024 •

edited

Loading