-
-
Notifications
You must be signed in to change notification settings - Fork 149
devserver: skip unlink when possible #2115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
devserver: skip unlink when possible #2115
Conversation
|
console.error(`Dst exists: ${exists}`) | ||
} | ||
if (exists) { | ||
await fs.promises.unlink(dst) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need to keep this when !grant_sandbox_write_permissions
?
Without this change we incur the cost of an extra unnecessary syscall on every file we need to sync. In my testing this didn't make a perceivable difference with small sets of changed files, however it could make a difference on large ones.
This change works just fine on Linux. However, on macOS you get a permission error if you try to copy over an existing file without
grant_sandbox_write_permissions = True
. So on macOS we only enable this optimization ifgrant_sandbox_write_permissions == True
, otherwise weunlink
the file like we did before.Changes are visible to end-users: no
Test plan
Existing tests
If it is possible to write a test that only runs on macOS, we could test that code path. However, I'm not sure how feasible that is.