-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(cli): suppress watch-mode errors for paths under .git #11305
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?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@biomejs/biome": patch | ||
| --- | ||
|
|
||
| Fixed [#11110](https://github.com/biomejs/biome/issues/11110): `biome lint --watch` and other watch-mode commands no longer emit `internalError/io` diagnostics for file events under `.git/` (such as `.git/index.lock`), and no longer surface transient filesystem errors reported by `notify` for paths that the watcher has no interest in. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,10 @@ | ||
| use std::sync::mpsc::{Receiver, channel}; | ||
|
|
||
| use crate::runner::diagnostics::WatcherDiagnostic; | ||
| use crate::runner::watcher::{Watcher, WatcherEvent}; | ||
| use biome_diagnostics::{Error, NotifyError}; | ||
| use camino::Utf8PathBuf; | ||
| use camino::{Utf8Path, Utf8PathBuf}; | ||
| use tracing::warn; | ||
| use notify::event::{CreateKind, ModifyKind, RemoveKind}; | ||
| use notify::{Event, EventKind, RecursiveMode, Result, recommended_watcher}; | ||
| use tracing::warn; | ||
|
|
||
| pub(crate) struct DefaultWatcher { | ||
| rx: Receiver<Result<Event>>, | ||
|
|
@@ -23,6 +21,20 @@ impl DefaultWatcher { | |
| } | ||
| } | ||
|
|
||
| /// Returns `true` if the path lives inside a directory that Biome should not | ||
| /// surface to the user even when it appears in a watcher event. | ||
| /// | ||
| /// Today this only covers the `.git` directory. It exists so that transient | ||
| /// VCS bookkeeping (such as `.git/index.lock` updates) does not produce | ||
| /// `internalError/io` diagnostics in `--watch` mode, even when the project's | ||
| /// `.gitignore` does not exclude `.git` or VCS integration is disabled. | ||
| fn is_internal_vcs_path(path: &Utf8Path) -> bool { | ||
| path.components().any(|c| match c { | ||
| camino::Utf8Component::Normal(part) => part == ".git", | ||
| _ => false, | ||
| }) | ||
| } | ||
|
|
||
| impl Watcher for DefaultWatcher { | ||
| fn watch(&mut self, paths: Vec<Utf8PathBuf>) { | ||
| let mut watched_paths = self.watcher.paths_mut(); | ||
|
|
@@ -40,9 +52,17 @@ impl Watcher for DefaultWatcher { | |
| fn poll(&mut self) -> Option<WatcherEvent> { | ||
| self.rx.iter().find_map(|event| { | ||
| match event { | ||
| Err(err) => Some(WatcherEvent::Error(WatcherDiagnostic { | ||
| source: Some(Error::from(NotifyError::from(err))), | ||
| })), | ||
| Err(err) => { | ||
| // `notify` surfaces filesystem errors that occur on the | ||
| // watched paths (for example, transient permission | ||
| // failures while reading a file inside `.git`). Those | ||
| // events should never reach Biome's error reporter, since | ||
| // they do not reflect problems with the user's code or | ||
| // configuration. Drop the event so the watcher keeps | ||
| // running. | ||
| warn!("Watcher event error (suppressed): {err}"); | ||
| None | ||
| } | ||
| Ok(event) => { | ||
| // Modifying folder or metadata is ignored as it can unlikely affect the results. | ||
| // Any event types are necessary for some platforms to catch events. | ||
|
|
@@ -55,13 +75,23 @@ impl Watcher for DefaultWatcher { | |
| | EventKind::Remove(RemoveKind::File | RemoveKind::Any) | ||
| | EventKind::Any | ||
| ) { | ||
| Some(WatcherEvent::Changed( | ||
| event | ||
| .paths | ||
| .into_iter() | ||
| .filter_map(|path| Utf8PathBuf::from_path_buf(path).ok()) | ||
| .collect(), | ||
| )) | ||
| let paths: Vec<Utf8PathBuf> = event | ||
| .paths | ||
| .into_iter() | ||
| .filter_map(|path| Utf8PathBuf::from_path_buf(path).ok()) | ||
| // `.git/` paths come from VCS bookkeeping (e.g. | ||
| // `index.lock` churn) and should not trigger a | ||
| // re-crawl. Filtering at the watcher level keeps | ||
| // the workspace scanner's ignore contract intact | ||
| // while removing the spurious diagnostics that | ||
| // otherwise surface in watch mode. | ||
| .filter(|path| !is_internal_vcs_path(path)) | ||
| .collect(); | ||
| if paths.is_empty() { | ||
| None | ||
| } else { | ||
| Some(WatcherEvent::Changed(paths)) | ||
| } | ||
| } else { | ||
| None | ||
| } | ||
|
|
@@ -70,3 +100,30 @@ impl Watcher for DefaultWatcher { | |
| }) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::is_internal_vcs_path; | ||
|
Comment on lines
+104
to
+106
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 3 'mod tests|use super::is_internal_vcs_path|use camino::Utf8Path|Utf8Path::new' \
crates/biome_cli/src/runner/impls/watchers/default.rsRepository: biomejs/biome Length of output: 1025 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- imports and tests ---'
sed -n '1,145p' crates/biome_cli/src/runner/impls/watchers/default.rs
printf '%s\n' '--- related watcher symbols and call sites ---'
rg -n -C 4 'fn poll|notify::Error|Utf8PathBuf|is_internal_vcs_path|changed_paths|poll\(' \
crates/biome_cli/src/runner/impls/watchers/default.rs crates/biome_cli/src/runner/impls/watchers
printf '%s\n' '--- standalone Rust name-resolution probe ---'
if command -v rustc >/dev/null 2>&1; then
tmp="$(mktemp --suffix=.rs)"
trap 'rm -f "$tmp" "${tmp%.rs}"' EXIT
cat >"$tmp" <<'RS'
mod parent {
use std::path::Path;
mod child {
fn uses_parent_import() {
let _ = Path::new(".");
}
}
}
fn main() {}
RS
if rustc "$tmp" -o "${tmp%.rs}" 2>&1; then
echo 'child can use the parent use binding unqualified'
else
echo 'child cannot use the parent use binding unqualified'
fi
else
echo 'rustc unavailable'
fiRepository: biomejs/biome Length of output: 23059 🏁 Script executed: #!/bin/bash
set -euo pipefail
tmp="$(mktemp /tmp/rust-scope-XXXXXX.rs)"
bin="${tmp%.rs}"
trap 'rm -f "$tmp" "$bin"' EXIT
cat >"$tmp" <<'RS'
mod parent {
use std::path::Path;
mod child {
fn uses_parent_import() {
let _ = Path::new(".");
}
}
}
fn main() {}
RS
if rustc --crate-name rust_scope_probe "$tmp" -o "$bin" 2>/tmp/rust-scope-probe.err; then
echo 'child can use the parent use binding unqualified'
else
echo 'child cannot use the parent use binding unqualified'
cat /tmp/rust-scope-probe.err
fiRepository: biomejs/biome Length of output: 852 Import The parent module’s import is not inherited by the child module, so 🤖 Prompt for AI Agents |
||
|
|
||
| #[test] | ||
| fn detects_path_inside_dot_git_directory() { | ||
| assert!(is_internal_vcs_path(Utf8Path::new( | ||
| "/repo/.git/index.lock" | ||
| ))); | ||
| assert!(is_internal_vcs_path(Utf8Path::new("repo/.git/HEAD"))); | ||
| assert!(is_internal_vcs_path(Utf8Path::new( | ||
| "/repo/sub/.git/index.lock" | ||
| ))); | ||
| } | ||
|
|
||
| #[test] | ||
| fn leaves_non_git_paths_alone() { | ||
| assert!(!is_internal_vcs_path(Utf8Path::new("/repo/src/index.js"))); | ||
| assert!(!is_internal_vcs_path(Utf8Path::new( | ||
| "/repo/.github/workflows/ci.yml" | ||
| ))); | ||
| assert!(!is_internal_vcs_path(Utf8Path::new( | ||
| "/repo/some/path/git-notes.txt" | ||
| ))); | ||
| } | ||
| } | ||
|
Comment on lines
+104
to
+129
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C 8 'DefaultWatcher|fn poll|WatcherEvent::Changed|WatcherEvent::Error|notify::Error' \
crates/biome_cli/src/runner/impls/watchersRepository: biomejs/biome Length of output: 7546 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- default watcher ---'
cat -n crates/biome_cli/src/runner/impls/watchers/default.rs
printf '%s\n' '--- watcher trait and consumers ---'
rg -n -C 10 'pub trait Watcher|enum WatcherEvent|WatcherEvent::Error|poll\(\)' \
crates/biome_cli/src/runner crates/biome_cli/src | head -n 260
printf '%s\n' '--- watcher module files and test imports ---'
git ls-files crates/biome_cli/src/runner/impls/watchers
rg -n -C 5 'Utf8Path|DefaultWatcher|WatcherEvent::Changed|WatcherEvent::Error' \
crates/biome_cli/src/runner/impls/watchersRepository: biomejs/biome Length of output: 25580 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- targeted change history for the reviewed file ---'
git diff --unified=25 -- crates/biome_cli/src/runner/impls/watchers/default.rs | head -n 240
printf '%s\n' '--- watcher diagnostics and error construction ---'
rg -n -C 12 'struct WatcherDiagnostic|enum WatcherDiagnostic|WatcherDiagnostic|WatcherEvent::Error' \
crates/biome_cli/src/runner crates/biome_cli/src | head -n 260
printf '%s\n' '--- deterministic checks for the test module and poll branches ---'
python3 - <<'PY'
from pathlib import Path
import re
p = Path("crates/biome_cli/src/runner/impls/watchers/default.rs")
text = p.read_text()
tests = text[text.index("#[cfg(test)]"):]
print("test module imports Utf8Path:",
bool(re.search(r'^\s*use\s+[^;]*\bUtf8Path\b', tests, re.M)))
print("test module references Utf8Path:",
bool(re.search(r'\bUtf8Path::new\s*\(', tests)))
print("poll suppresses Err branches:",
bool(re.search(r'Err\(err\)\s*=>\s*\{.*?None', text, re.S)))
print("poll emits Changed only after non-empty paths:",
bool(re.search(r'if paths\.is_empty\(\)\s*\{\s*None\s*\}\s*else\s*\{\s*Some\(WatcherEvent::Changed\(paths\)\)', text, re.S)))
print("poll filters .git paths:",
bool(re.search(r'\.filter\(\|path\| !is_internal_vcs_path\(path\)\)', text)))
PYRepository: biomejs/biome Length of output: 8939 Add the missing import and The test module uses 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
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.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: biomejs/biome
Length of output: 25778
🏁 Script executed:
Repository: biomejs/biome
Length of output: 28535
🏁 Script executed:
Repository: biomejs/biome
Length of output: 50370
🏁 Script executed:
Repository: biomejs/biome
Length of output: 7111
🌐 Web query:
notify 8.2.0 Error type paths ErrorKind documentation💡 Result:
In the notify crate version 8.2.0, errors are represented by the notify::Error struct, which contains two primary fields: kind and paths [1][2]. The kind field is an instance of the notify::ErrorKind enum, which categorizes the type of error that occurred [3][1]. The paths field is a Vec that contains any filesystem paths relevant to the error, if applicable [1][2]. The ErrorKind enum includes the following variants: - Generic(String): Used for platform-specific mappings or opaque internal errors [3][4]. - Io(std::io::Error): Used for standard I/O errors [3][4]. - PathNotFound: Indicates that a specified path does not exist [3][4]. - WatchNotFound: Used when attempting to remove a watch that does not exist [3][4]. - InvalidConfig(Config): Indicates an invalid value was provided in the runtime configuration [3][4]. - MaxFilesWatch: Indicates the limit on the total number of inotify watches has been reached [3][4]. The Error struct provides methods to manage these paths, such as add_path(PathBuf) and set_paths(Vec), allowing users to associate specific filesystem targets with a given error instance [2]. Errors can be generated either during the creation of a Watcher or during the event stream [1].
Citations:
🏁 Script executed:
Repository: biomejs/biome
Length of output: 20442
🏁 Script executed:
Repository: biomejs/biome
Length of output: 32747
Preserve non-VCS watcher errors.
If a
notify::Errorhas no paths, includes a non-VCS path, or is not demonstrably transient, returnWatcherEvent::Error. Suppress only transient errors limited to internal VCS paths. Add regression tests and importcamino::Utf8Pathin the test module so the tests compile.🤖 Prompt for AI Agents