Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion crates/git/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ mod state;

pub use state::{StateHandle, StateOptions};
#[doc(hidden)]
pub use state::{debug_engine_refs, debug_status_recomputes, debug_worktree_watches};
pub use state::{
debug_engine_refs, debug_status_recomputes, debug_wait_worktree_watches, debug_worktree_watches,
};

/// The slash-separated Git path of `abs` relative to `workdir`, if any.
/// On Windows this converts backslashes so that ignore rules and index
Expand Down
29 changes: 23 additions & 6 deletions crates/git/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use std::collections::{BTreeMap, BTreeSet, HashMap};
use std::path::{Path, PathBuf};
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::mpsc::{Receiver, RecvTimeoutError, SyncSender};
use std::sync::{Arc, Mutex, OnceLock, Weak};
use std::sync::{Arc, Condvar, Mutex, OnceLock, Weak};
use std::time::{Duration, Instant};

use crate::model::{
Expand Down Expand Up @@ -223,11 +223,27 @@ fn status_recomputes() -> &'static Mutex<HashMap<PathBuf, u64>> {
/// arm. Test/diagnostic hook.
#[doc(hidden)]
pub fn debug_worktree_watches(gitdir: &Path) -> Option<Vec<PathBuf>> {
worktree_watch_sets().lock().unwrap().get(gitdir).cloned()
worktree_watch_sets().0.lock().unwrap().get(gitdir).cloned()
}

fn worktree_watch_sets() -> &'static Mutex<HashMap<PathBuf, Vec<PathBuf>>> {
static SETS: OnceLock<Mutex<HashMap<PathBuf, Vec<PathBuf>>>> = OnceLock::new();
/// Blocks until the engine publishes a watch set for `gitdir` that
/// satisfies `done`. Test/diagnostic hook.
#[doc(hidden)]
pub fn debug_wait_worktree_watches(gitdir: &Path, mut done: impl FnMut(&[PathBuf]) -> bool) {
let (sets, published) = worktree_watch_sets();
drop(
published
.wait_while(sets.lock().unwrap(), |sets| {
!done(sets.get(gitdir).map_or(&[], Vec::as_slice))
})
.unwrap(),
);
}

type WatchSets = (Mutex<HashMap<PathBuf, Vec<PathBuf>>>, Condvar);

fn worktree_watch_sets() -> &'static WatchSets {
static SETS: OnceLock<WatchSets> = OnceLock::new();
SETS.get_or_init(Default::default)
}

Expand Down Expand Up @@ -1241,10 +1257,11 @@ impl Engine {
}
arms.watch_set_changed = false;
let dirs = arms.worktree_dirs.iter().cloned().collect();
worktree_watch_sets()
.lock()
let (sets, published) = worktree_watch_sets();
sets.lock()
.unwrap()
.insert((*self.repo.gitdir).clone(), dirs);
published.notify_all();
}

/// Directories to arm for the ignore sources that live *outside* every
Expand Down
31 changes: 17 additions & 14 deletions crates/git/tests/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::collections::BTreeMap;
use std::path::PathBuf;
use std::process::Command;
use std::sync::atomic::{AtomicU64, Ordering};
use std::time::{Duration, Instant};
use std::time::Duration;

use yas_git::native::{
self, BlameRecord, DiffRecord, DiffRequest, Endpoint, LogRequest, PatchRecord, PatchRequest,
Expand Down Expand Up @@ -79,6 +79,19 @@ impl Repository {
.map(|path| std::fs::canonicalize(path).unwrap())
.collect()
}

/// Waits for the engine to arm or disarm `dir`. The status snapshot can
/// reflect an index written after its pass reconciled the watch set, so
/// the set catches up on the pass that index event triggers.
fn wait_for_watch(&self, dir: &str, armed: bool) {
let path = self.0.join(dir);
yas_git::debug_wait_worktree_watches(&self.0.join(".git"), |watches| {
watches
.iter()
.any(|watch| std::fs::canonicalize(watch).is_ok_and(|watch| watch == path))
== armed
});
Comment on lines +86 to +93

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚪ Nit · Watch-set waits can hang instead of failing

debug_wait_worktree_watches now uses wait_while with no timeout. If the engine never arms or disarms the directory, these tests block until the CI job's own timeout. That covers a real regression, and also a gitdir key that doesn't match repo.gitdir, such as an uncanonicalized path. Before this change, reattached_status_selection_observes_changes_made_while_pruned failed after 5 s with "ignored directory was not pruned", and the 6ef493c version kept that 5 s bound through wait_timeout_while. This is the same problem as #1, now also in the git engine tests.

Ultimator · issue #2 · reply @ultimator-ai fix 2 to have it fixed

}
}

impl Drop for Repository {
Expand Down Expand Up @@ -466,13 +479,13 @@ fn index_changes_reconcile_tracked_exceptions_to_ignore_pruning() {

repository.git(&["add", "-f", "generated/nested/tracked"]);
assert_eq!(state.status()["generated/nested/tracked"], (b'A', b' '));
assert!(watches().contains(&repository.0.join("generated/nested")));
repository.wait_for_watch("generated/nested", true);
repository.write("generated/nested/tracked", b"modified after force-add\n");
assert_eq!(state.status()["generated/nested/tracked"], (b'A', b'M'));

repository.git(&["rm", "--cached", "-f", "generated/nested/tracked"]);
assert!(state.status().is_empty());
assert!(!watches().contains(&repository.0.join("generated")));
repository.wait_for_watch("generated", false);
}

#[test]
Expand Down Expand Up @@ -594,17 +607,7 @@ fn reattached_status_selection_observes_changes_made_while_pruned() {

// Wait for the asynchronous detach to disarm this directory. Writes
// inside it then cannot invalidate a cached ignored-status segment.
let deadline = Instant::now() + Duration::from_secs(5);
while repository
.worktree_watches()
.contains(&repository.0.join("ignored"))
{
assert!(
Instant::now() < deadline,
"ignored directory was not pruned"
);
std::thread::sleep(Duration::from_millis(5));
}
repository.wait_for_watch("ignored", false);
repository.write("ignored/second", b"new ignored file\n");
let broader = StateStream::new(&handle, status_options(true, true));
assert_eq!(broader.status().len(), 2);
Expand Down
8 changes: 2 additions & 6 deletions crates/server/src/yas_process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2704,11 +2704,7 @@ mod windows_tests {
) -> T {
let mut output = String::new();
loop {
match tokio::time::timeout(Duration::from_secs(20), attachment.next())
.await
.expect("output in time")
.expect("the attachment is open")
{
match attachment.next().await.expect("the attachment is open") {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚪ Nit · read_until can now hang the job instead of failing

With the timeout gone, a test whose process never prints the line it waits for blocks until the CI job's own timeout. The run then shows a cancelled job instead of a failure that names the test and the output it got. This also drops the 20 s bound from the ping test, which does not start PowerShell. The previous version (a within parameter, 120 s for PowerShell, and a panic that showed the output so far) fixed the cold start without losing either.

Ultimator · issue #1 · reply @ultimator-ai fix 1 to have it fixed

Event::Output { stream, data, .. } => {
*acked += data.len() as u64;
attachment.acknowledge_output(stream, *acked).unwrap();
Expand Down Expand Up @@ -2955,7 +2951,6 @@ mod windows_tests {
let server = Server::new(false, true);
let runtime = Runtime::new(server.clone());
let session = runtime.session([34; 16], None).unwrap();
let started = std::time::Instant::now();
let mut attachment = session
.spawn(
&with_residue_grace(
Expand All @@ -2975,6 +2970,7 @@ mod windows_tests {
.unwrap();
let mut acked = 0;
let pid = read_until(&mut attachment, &mut acked, residue_pid).await;
let started = std::time::Instant::now();
let exit = exit_of(&mut attachment, &mut acked, Duration::from_secs(20)).await;
assert_eq!(
(exit.kind, exit.code),
Expand Down
Loading