From 2e8cbee8ac2e523ebaba9bf66b514a2002c2b998 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Sat, 15 Aug 2026 19:19:33 -0700 Subject: [PATCH 1/2] update vhdl_ls workspace when new files are added --- vw-analyzer/src/server.rs | 17 +- vw-analyzer/src/vhdl_backend.rs | 300 +++++++++++++++++++++++++++++++- 2 files changed, 308 insertions(+), 9 deletions(-) diff --git a/vw-analyzer/src/server.rs b/vw-analyzer/src/server.rs index 0176f5a..9d8ff3c 100644 --- a/vw-analyzer/src/server.rs +++ b/vw-analyzer/src/server.rs @@ -273,7 +273,14 @@ impl LanguageServer for Analyzer { // registration lets each backend pick its own patterns — // today the VHDL backend needs `vw.toml`, `vw.lock`, and // `ip/**/*.htcl` to reflect `vw update` and IP-config - // edits back into the wrapped `vhdl_ls::VHDLServer`. + // edits back into the wrapped `vhdl_ls::VHDLServer`, plus + // `**/*.vhd{,l}` because that server's config is a concrete + // file list: a source added or removed on disk changes the + // library mapping, and until the config is re-rendered the + // new file resolves nothing (`No primary unit '' + // within library 'work'`). The VHDL backend also re-checks + // membership on open/save, so this registration failing + // degrades rather than breaks. // // Registration failures (client that doesn't advertise // dynamic registration, or refuses this specific one) are @@ -292,6 +299,14 @@ impl LanguageServer for Analyzer { glob_pattern: GlobPattern::String("**/ip/**/*.htcl".into()), kind: None, }, + FileSystemWatcher { + glob_pattern: GlobPattern::String("**/*.vhd".into()), + kind: None, + }, + FileSystemWatcher { + glob_pattern: GlobPattern::String("**/*.vhdl".into()), + kind: None, + }, ]; let registration = Registration { id: "vw-analyzer-watched-files".into(), diff --git a/vw-analyzer/src/vhdl_backend.rs b/vw-analyzer/src/vhdl_backend.rs index 91d02fa..48c7f06 100644 --- a/vw-analyzer/src/vhdl_backend.rs +++ b/vw-analyzer/src/vhdl_backend.rs @@ -23,7 +23,8 @@ use async_trait::async_trait; use camino::Utf8PathBuf; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; +use std::path::{Path, PathBuf}; use std::rc::Rc; use std::sync::{Arc, Mutex as StdMutex}; use tokio::sync::{oneshot, Mutex as TokioMutex}; @@ -31,11 +32,11 @@ use tower_lsp::lsp_types::{ ClientCapabilities, CompletionItem, Diagnostic, DidChangeTextDocumentParams, DidChangeWatchedFilesParams, DidCloseTextDocumentParams, DidOpenTextDocumentParams, DocumentSymbol, - DocumentSymbolParams, DocumentSymbolResponse, GotoDefinitionParams, - GotoDefinitionResponse, Hover, HoverParams, InitializeParams, Location, - LogMessageParams, MessageType, PartialResultParams, Position, - PublishDiagnosticsParams, ShowMessageParams, SignatureHelp, - SymbolInformation, TextDocumentIdentifier, TextDocumentItem, + DocumentSymbolParams, DocumentSymbolResponse, FileChangeType, + GotoDefinitionParams, GotoDefinitionResponse, Hover, HoverParams, + InitializeParams, Location, LogMessageParams, MessageType, + PartialResultParams, Position, PublishDiagnosticsParams, ShowMessageParams, + SignatureHelp, SymbolInformation, TextDocumentIdentifier, TextDocumentItem, TextDocumentPositionParams, Url, VersionedTextDocumentIdentifier, WorkDoneProgressParams, WorkspaceEdit, }; @@ -49,6 +50,21 @@ use crate::backend::LanguageBackend; /// `tokio::sync::oneshot`. struct WorkspaceHandle { tx: std::sync::mpsc::Sender, + /// Workspace root (the dir holding `vw.toml`) — kept so the + /// config can be re-rendered when the file set shifts. + root: Utf8PathBuf, + /// Every file the *currently installed* config maps into a + /// library, normalized the same way `vhdl_lang` normalizes + /// source paths. A `.vhd` that isn't in here is invisible to + /// the wrapped server's library mapping — it gets analyzed as + /// a standalone `Source::inline` with no `work` visibility, so + /// `use work.` fails to resolve. + project_files: StdMutex>, + /// Paths we already re-rendered for and *still* didn't find — + /// a scratch `.vhd` outside `hdl/`, or a buffer not yet written + /// to disk. Keeps every keystroke in such a file from walking + /// the workspace again. `did_save` re-checks regardless. + absent: StdMutex>, /// Held only for cleanup at drop; joining is best-effort. _thread: StdMutex>>, } @@ -62,6 +78,93 @@ impl Drop for WorkspaceHandle { } } +impl WorkspaceHandle { + /// Make sure `path` is part of the wrapped server's project + /// before its text arrives. + /// + /// The config is a *concrete file list* rendered from the + /// workspace's on-disk enumeration, so a `.vhd` created after + /// the worker spawned isn't in it. Re-render on first sight of + /// an unknown path and push the result down, which is what + /// puts a newly added `hdl/*.vhd` into `defaultlib` and lets + /// `use work.…` resolve. + /// + /// `force` bypasses the [`absent`](Self::absent) memo — used + /// from `did_save`, where the very event that matters is a + /// buffer becoming a file on disk (`file_names` only reports + /// files that exist). + fn ensure_in_project(&self, path: &Path, force: bool) { + let path = normalize(path); + if self.project_files.lock().unwrap().contains(&path) { + return; + } + if !force && !self.absent.lock().unwrap().insert(path.clone()) { + return; + } + let cfg = match vw_lib::render_vhdl_lang_config(&self.root, None) { + Ok(c) => c, + Err(e) => { + warn!( + "vhdl_backend: config re-render failed for {}: {e}", + self.root + ); + return; + } + }; + let files = config_file_set(&cfg); + let found = files.contains(&path); + if found { + self.absent.lock().unwrap().remove(&path); + } else { + self.absent.lock().unwrap().insert(path); + } + // Installing a config rebuilds the whole design root, so + // only do it when the enumeration actually moved. + if self.install_files(files) { + let _ = self.tx.send(Message::UpdateConfig(cfg)); + } + } + + /// Replace the cached project-file set. Returns true when it + /// differed from what was already cached — i.e. when the + /// wrapped server needs the new config. + fn install_files(&self, files: HashSet) -> bool { + let mut cached = self.project_files.lock().unwrap(); + if *cached == files { + return false; + } + // Anything previously written off as absent gets another + // chance against the new enumeration. + self.absent.lock().unwrap().retain(|p| !files.contains(p)); + *cached = files; + true + } +} + +/// Every file the config maps into a library, normalized for +/// comparison against a URI-derived path. +fn config_file_set(cfg: &vhdl_lang::Config) -> HashSet { + let mut messages = vhdl_lang::NullMessages; + cfg.iter_libraries() + .flat_map(|lib| lib.file_names(&mut messages)) + .map(|p| normalize(&p)) + .collect() +} + +fn is_vhdl(path: &Path) -> bool { + matches!( + path.extension().and_then(|e| e.to_str()), + Some("vhd") | Some("vhdl") + ) +} + +/// Absolute-but-not-canonical, matching `vhdl_lang::FilePath`: +/// symlinks are deliberately left unresolved there, so resolving +/// them here would produce paths that never compare equal. +fn normalize(path: &Path) -> PathBuf { + std::path::absolute(path).unwrap_or_else(|_| path.to_path_buf()) +} + #[allow(dead_code)] enum Message { DidOpen(DidOpenTextDocumentParams), @@ -153,6 +256,21 @@ impl VhdlBackend { map.insert(ws, handle.clone()); Some(handle) } + + /// Resolve `uri` to its workspace and make sure the file is in + /// that workspace's project before anything else touches it. + /// See [`WorkspaceHandle::ensure_in_project`] for why. + async fn workspace_for_file( + &self, + uri: &Url, + force_refresh: bool, + ) -> Option> { + let handle = self.ensure_workspace(uri).await?; + if let Ok(path) = uri.to_file_path() { + handle.ensure_in_project(&path, force_refresh); + } + Some(handle) + } } fn spawn_workspace_worker( @@ -163,9 +281,11 @@ fn spawn_workspace_worker( stdlib_libraries_path: Option, ) -> Arc { let (tx, rx) = std::sync::mpsc::channel::(); + let project_files = config_file_set(&initial_config); + let worker_root = root.clone(); let thread = std::thread::spawn(move || { workspace_thread( - root, + worker_root, client, runtime, initial_config, @@ -175,6 +295,9 @@ fn spawn_workspace_worker( }); Arc::new(WorkspaceHandle { tx, + root, + project_files: StdMutex::new(project_files), + absent: StdMutex::new(HashSet::new()), _thread: StdMutex::new(Some(thread)), }) } @@ -369,7 +492,10 @@ impl LanguageBackend for VhdlBackend { } async fn set_text(&self, uri: Url, text: String) { - let Some(ws) = self.ensure_workspace(&uri).await else { + // `force = false`: this runs on every keystroke, and the + // memo in `ensure_in_project` keeps all but the first sight + // of an unknown path free. + let Some(ws) = self.workspace_for_file(&uri, false).await else { return; }; // vhdl_ls doesn't distinguish "first open" from @@ -391,6 +517,14 @@ impl LanguageBackend for VhdlBackend { let _ = ws.tx.send(Message::DidOpen(params)); } + async fn save(&self, uri: &Url) { + // A brand-new buffer only becomes a project file once it + // exists on disk — `LibraryConfig::file_names` skips paths + // that don't. Force past the absent-memo so the first write + // of `hdl/new.vhd` pulls it into `defaultlib`. + self.workspace_for_file(uri, true).await; + } + async fn did_change_watched_files( &self, params: &DidChangeWatchedFilesParams, @@ -409,6 +543,15 @@ impl LanguageBackend for VhdlBackend { let Ok(path) = change.uri.to_file_path() else { continue; }; + // A VHDL source only moves the config when it appears + // or disappears — its *contents* reach the server + // through `did_change`, and re-rendering on every save + // would rebuild the whole design root for nothing. + // Vivado writing a few thousand generated `.vhd`s + // under `target/` is the case that makes this matter. + if is_vhdl(&path) && change.typ == FileChangeType::CHANGED { + continue; + } if let Some(ws) = crate::workspace::find_workspace_dir(&path) { affected.insert(ws); } @@ -426,6 +569,11 @@ impl LanguageBackend for VhdlBackend { }; match vw_lib::render_vhdl_lang_config(&ws, None) { Ok(cfg) => { + // Keep the membership cache in step with what + // the server is about to be told, so a file + // this event just added doesn't trigger a + // second re-render when it's opened. + handle.install_files(config_file_set(&cfg)); let _ = handle.tx.send(Message::UpdateConfig(cfg)); } Err(e) => { @@ -589,3 +737,139 @@ const _MSG_TYPES: &[MessageType] = &[ MessageType::INFO, MessageType::LOG, ]; + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::mpsc::Receiver; + + const VW_TOML: &str = "[workspace]\nname=\"t\"\nversion=\"0.1.0\"\n\ + [dependencies]\n[test-dependencies]\n"; + + /// A workspace handle with no worker thread behind it — + /// `Message`s pile up in the returned receiver instead, which + /// is exactly what these tests want to inspect. + fn handle_for(ws: &Utf8PathBuf) -> (WorkspaceHandle, Receiver) { + let (tx, rx) = std::sync::mpsc::channel(); + let cfg = vw_lib::render_vhdl_lang_config(ws, None).unwrap(); + let handle = WorkspaceHandle { + tx, + root: ws.clone(), + project_files: StdMutex::new(config_file_set(&cfg)), + absent: StdMutex::new(HashSet::new()), + _thread: StdMutex::new(None), + }; + (handle, rx) + } + + fn workspace() -> (tempfile::TempDir, Utf8PathBuf) { + let tmp = tempfile::tempdir().unwrap(); + let ws = Utf8PathBuf::from_path_buf(tmp.path().to_path_buf()).unwrap(); + std::fs::write(ws.join("vw.toml"), VW_TOML).unwrap(); + std::fs::create_dir_all(ws.join("hdl")).unwrap(); + std::fs::write(ws.join("hdl/existing.vhd"), "").unwrap(); + (tmp, ws) + } + + fn took_update_config(rx: &Receiver) -> bool { + matches!(rx.try_recv(), Ok(Message::UpdateConfig(_))) + } + + /// The bug: a `.vhd` created after the workspace's server + /// started isn't in its rendered config, so it lands outside + /// every library and `use work.` can't resolve. Opening + /// it has to re-render first. + #[test] + fn new_file_pulls_in_a_fresh_config() { + let (_tmp, ws) = workspace(); + let (handle, rx) = handle_for(&ws); + + let added = ws.join("hdl/added.vhd"); + std::fs::write(&added, "").unwrap(); + handle.ensure_in_project(added.as_std_path(), false); + + assert!( + took_update_config(&rx), + "opening a newly created source must push a re-rendered config" + ); + assert!(handle + .project_files + .lock() + .unwrap() + .contains(&normalize(added.as_std_path()))); + } + + /// A file already in the config is the common case — every + /// keystroke goes through here, so it must not re-render. + #[test] + fn known_file_does_not_re_render() { + let (_tmp, ws) = workspace(); + let (handle, rx) = handle_for(&ws); + + handle.ensure_in_project( + ws.join("hdl/existing.vhd").as_std_path(), + false, + ); + + assert!(rx.try_recv().is_err(), "no config churn for a known file"); + } + + /// An unsaved buffer isn't on disk, so no re-render can find + /// it (`file_names` only reports files that exist). The memo + /// keeps the miss cheap; `did_save`'s forced re-check is what + /// picks the file up once it lands. + #[test] + fn absent_file_is_memoized_until_saved() { + let (_tmp, ws) = workspace(); + let (handle, rx) = handle_for(&ws); + let pending = ws.join("hdl/pending.vhd"); + + handle.ensure_in_project(pending.as_std_path(), false); + assert!( + rx.try_recv().is_err(), + "nothing on disk, nothing to install" + ); + + // File appears — but an unforced check is memoized away. + std::fs::write(&pending, "").unwrap(); + handle.ensure_in_project(pending.as_std_path(), false); + assert!( + rx.try_recv().is_err(), + "unforced re-check must stay memoized" + ); + + // `did_save`'s forced path is the one that notices. + handle.ensure_in_project(pending.as_std_path(), true); + assert!(took_update_config(&rx), "save must re-check and install"); + assert!(handle + .project_files + .lock() + .unwrap() + .contains(&normalize(pending.as_std_path()))); + } + + /// `did_change_watched_files` installs the new enumeration + /// directly; a file it just added must not trigger a second + /// re-render when the editor opens it. + #[test] + fn install_files_clears_the_absent_memo() { + let (_tmp, ws) = workspace(); + let (handle, rx) = handle_for(&ws); + let late = ws.join("hdl/late.vhd"); + + handle.ensure_in_project(late.as_std_path(), false); + assert!(handle + .absent + .lock() + .unwrap() + .contains(&normalize(late.as_std_path()))); + + std::fs::write(&late, "").unwrap(); + let cfg = vw_lib::render_vhdl_lang_config(&ws, None).unwrap(); + assert!(handle.install_files(config_file_set(&cfg))); + + handle.ensure_in_project(late.as_std_path(), false); + assert!(rx.try_recv().is_err(), "already installed — no re-render"); + assert!(handle.absent.lock().unwrap().is_empty()); + } +} From 4c0e609740b2adeaf787ee1504889010a54a44b2 Mon Sep 17 00:00:00 2001 From: Ryan Goodfellow Date: Sat, 15 Aug 2026 19:33:25 -0700 Subject: [PATCH 2/2] fix ci on macos plus a few symlink bugs --- vw-analyzer/src/htcl_backend.rs | 118 ++++++++++++++++++++------------ vw-lib/src/lib.rs | 34 ++++++--- vw-repl/src/lower.rs | 7 ++ 3 files changed, 107 insertions(+), 52 deletions(-) diff --git a/vw-analyzer/src/htcl_backend.rs b/vw-analyzer/src/htcl_backend.rs index 9b9b209..f897b51 100644 --- a/vw-analyzer/src/htcl_backend.rs +++ b/vw-analyzer/src/htcl_backend.rs @@ -601,7 +601,10 @@ async fn reindex_importers_of( analysis .as_ref() .filter(|a| { - a.view.imports.iter().any(|i| &i.file_uri == changed) + a.view + .imports + .iter() + .any(|i| same_file(&i.file_uri, changed)) }) .map(|_| u.clone()) }) @@ -1511,7 +1514,7 @@ impl HtclBackend { }; // For the origin file, prefer the in-memory analysis // text so unsaved edits round-trip. - let text = if file_uri == *origin { + let text = if same_file(&file_uri, origin) { if let Some(analysis) = self.analysis_for(&file_uri).await { analysis.local_text.clone() } else { @@ -1585,6 +1588,36 @@ fn uri_under_roots(uri: &Url, roots: &[std::path::PathBuf]) -> bool { }) } +/// True when two file URIs name the same file on disk. +/// +/// Import URIs are built from resolver output, which canonicalizes; +/// document URIs come from the editor, which does not — Helix hands +/// back whatever path the user opened. Any path crossing a symlink +/// (`$TMPDIR` on macOS, a checkout under a symlinked home) gives the +/// two sides different spellings of one file, and a plain `==` then +/// answers "different file": the fan-out reindex stops firing, so an +/// edit to an imported file never reaches the open importer. Compare +/// canonical forms, falling back to the raw path when +/// canonicalization fails (deleted file, permission error). +fn same_file(a: &Url, b: &Url) -> bool { + if a == b { + return true; + } + let (Ok(pa), Ok(pb)) = (a.to_file_path(), b.to_file_path()) else { + return false; + }; + // Cheap prune before touching the filesystem: two spellings of + // one file always agree on the final component. The fan-out + // scan runs this against every import of every open doc — a + // vivado-cmd tree is ~900 of them — and nearly all differ right + // here, so this keeps the syscalls to the handful that could + // plausibly match. + if pa.file_name() != pb.file_name() { + return false; + } + pa.canonicalize().unwrap_or(pa) == pb.canonicalize().unwrap_or(pb) +} + fn walk_htcl_files(dir: &std::path::Path, out: &mut Vec) { let entries = match std::fs::read_dir(dir) { Ok(e) => e, @@ -2236,6 +2269,21 @@ mod tests { Url::parse("file:///tmp/x.htcl").unwrap() } + /// A temp dir plus the *canonical* form of its path. + /// + /// Everything the analyzer resolves through the loader comes + /// back canonicalized, so a test that builds its expected URIs + /// from the raw `TempDir::path()` is comparing two spellings of + /// one file. That only shows up where the temp root crosses a + /// symlink — which is exactly what macOS does, with `$TMPDIR` + /// under `/var` → `/private/var`, and what Linux's `/tmp` + /// happens not to do. Build test paths from this root instead. + fn temp_root() -> (tempfile::TempDir, std::path::PathBuf) { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().canonicalize().unwrap(); + (dir, root) + } + #[tokio::test] async fn handles_htcl_extension() { let backend = HtclBackend::new(); @@ -2955,15 +3003,15 @@ proc make_widget {} dict { return {} }\n"; Url, // main.htcl Url, // lib.htcl ) { - let dir = tempfile::tempdir().unwrap(); - let lib_path = dir.path().join("lib.htcl"); + let (dir, root) = temp_root(); + let lib_path = root.join("lib.htcl"); std::fs::write( &lib_path, "## Greet someone.\n\ proc greet {\n ## Who to greet.\n who\n} { puts \"hi $who\" }\n", ) .unwrap(); - let main_path = dir.path().join("main.htcl"); + let main_path = root.join("main.htcl"); let main_src = "src lib\ngreet -who world\n"; std::fs::write(&main_path, main_src).unwrap(); @@ -3327,8 +3375,8 @@ proc greet {\n ## Who to greet.\n who\n} { puts \"hi $who\" }\n", /// the real vivado-cmd tree still returned nothing. #[tokio::test] async fn goto_finds_sibling_workspace_dep_via_nested_src() { - let dir = tempfile::tempdir().unwrap(); - let amd = dir.path().join("amd"); + let (_dir, root) = temp_root(); + let amd = root.join("amd"); let cpm5 = amd.join("cpm5"); let vivado_cmd = amd.join("vivado-cmd"); let vivado_cmd_cmd = vivado_cmd.join("cmd"); @@ -3413,8 +3461,8 @@ proc greet {\n ## Who to greet.\n who\n} { puts \"hi $who\" }\n", /// I'm in a vw-tracked dependency." #[tokio::test] async fn goto_finds_sibling_workspace_dep() { - let dir = tempfile::tempdir().unwrap(); - let amd = dir.path().join("amd"); + let (_dir, root) = temp_root(); + let amd = root.join("amd"); let cpm5 = amd.join("cpm5"); let vivado_cmd = amd.join("vivado-cmd"); std::fs::create_dir_all(&cpm5).unwrap(); @@ -3611,10 +3659,10 @@ proc greet {\n ## Who to greet.\n who\n} { puts \"hi $who\" }\n", // error-free. workspace_diagnostics must report the lib's // diagnostic against the LIB's URI so the editor's // workspace picker points to the right file. - let dir = tempfile::tempdir().unwrap(); - let lib_path = dir.path().join("broken.htcl"); + let (_dir, root) = temp_root(); + let lib_path = root.join("broken.htcl"); std::fs::write(&lib_path, "proc broken {} { return 42 }\n").unwrap(); - let main_path = dir.path().join("main.htcl"); + let main_path = root.join("main.htcl"); let main_src = "src broken\n"; std::fs::write(&main_path, main_src).unwrap(); let backend = HtclBackend::new(); @@ -3622,9 +3670,7 @@ proc greet {\n ## Who to greet.\n who\n} { puts \"hi $who\" }\n", let lib_uri = Url::from_file_path(&lib_path).unwrap(); // Set the editor's workspace root to the temp dir so the // filter accepts the lib file (which lives inside it). - backend - .set_workspace_roots(vec![dir.path().to_path_buf()]) - .await; + backend.set_workspace_roots(vec![root.clone()]).await; backend .set_text_sync(main_uri.clone(), main_src.into()) .await; @@ -3652,18 +3698,16 @@ proc greet {\n ## Who to greet.\n who\n} { puts \"hi $who\" }\n", // That empty payload is what the editor overwrites its // cached "had errors" state with; without it, the // stale errors linger in `space-D` even after the fix. - let dir = tempfile::tempdir().unwrap(); - let lib_path = dir.path().join("lib.htcl"); + let (_dir, root) = temp_root(); + let lib_path = root.join("lib.htcl"); std::fs::write(&lib_path, "proc broken {} { return 42 }\n").unwrap(); - let main_path = dir.path().join("main.htcl"); + let main_path = root.join("main.htcl"); let main_src = "src lib\n"; std::fs::write(&main_path, main_src).unwrap(); let backend = HtclBackend::new(); let main_uri = Url::from_file_path(&main_path).unwrap(); let lib_uri = Url::from_file_path(&lib_path).unwrap(); - backend - .set_workspace_roots(vec![dir.path().to_path_buf()]) - .await; + backend.set_workspace_roots(vec![root.clone()]).await; backend .set_text_sync(main_uri.clone(), main_src.into()) .await; @@ -3855,14 +3899,11 @@ proc greet {\n ## Who to greet.\n who\n} { puts \"hi $who\" }\n", // entry-point set. Without this, Helix's space-D picker // would show nothing for warnings in files the user // hasn't visited. - let dir = tempfile::tempdir().unwrap(); + let (_dir, root) = temp_root(); // Minimal vw.toml to make this a valid workspace root // (workspace-discovery walks up looking for it). - std::fs::write( - dir.path().join("vw.toml"), - "[workspace]\nname = \"t\"\n", - ) - .unwrap(); + std::fs::write(root.join("vw.toml"), "[workspace]\nname = \"t\"\n") + .unwrap(); // design.htcl carries a stub proc with a `@default(0)` // arg; the redundant-default warning fires on the call // site below. @@ -3870,19 +3911,17 @@ proc greet {\n ## Who to greet.\n who\n} { puts \"hi $who\" }\n", proc use_it { @default(0) count } unit { puts $count } use_it -count 0 "; - let design_path = dir.path().join("design.htcl"); + let design_path = root.join("design.htcl"); std::fs::write(&design_path, design_src).unwrap(); let design_uri = Url::from_file_path(&design_path).unwrap(); // Open a DIFFERENT file — `other.htcl` — that does NOT // src design.htcl. Without preload, design.htcl wouldn't // appear in the docs map at all. - let other_path = dir.path().join("other.htcl"); + let other_path = root.join("other.htcl"); std::fs::write(&other_path, "puts hi\n").unwrap(); let other_uri = Url::from_file_path(&other_path).unwrap(); let backend = HtclBackend::new(); - backend - .set_workspace_roots(vec![dir.path().to_path_buf()]) - .await; + backend.set_workspace_roots(vec![root.clone()]).await; backend .set_text_sync(other_uri.clone(), "puts hi\n".into()) .await; @@ -3922,25 +3961,20 @@ use_it -count 0 // hasn't committed at that instant; the assertion is // that `workspace_diagnostics` still returns the warning // (having awaited the commit internally). - let dir = tempfile::tempdir().unwrap(); - std::fs::write( - dir.path().join("vw.toml"), - "[workspace]\nname = \"t\"\n", - ) - .unwrap(); + let (_dir, root) = temp_root(); + std::fs::write(root.join("vw.toml"), "[workspace]\nname = \"t\"\n") + .unwrap(); let warn_src = "\ proc use_it { @default(0) count } unit { puts $count } use_it -count 0 "; - let warn_path = dir.path().join("design.htcl"); + let warn_path = root.join("design.htcl"); std::fs::write(&warn_path, warn_src).unwrap(); let warn_uri = Url::from_file_path(&warn_path).unwrap(); let backend = HtclBackend::new(); // set_workspace_roots kicks off the preload but returns // BEFORE any preload indexer commits. - backend - .set_workspace_roots(vec![dir.path().to_path_buf()]) - .await; + backend.set_workspace_roots(vec![root.clone()]).await; // Straight to workspace_diagnostics — no // wait_until_analysis_present. This is the racey path. let ws: std::collections::HashMap> = diff --git a/vw-lib/src/lib.rs b/vw-lib/src/lib.rs index c3591cb..7664155 100644 --- a/vw-lib/src/lib.rs +++ b/vw-lib/src/lib.rs @@ -5859,6 +5859,20 @@ mod locked_resolution_tests { mod dependency_source_tests { use super::*; + /// A temp dir plus the *canonical* form of its path. + /// + /// Dep resolution canonicalizes the paths it hands back (see + /// `transitive_dep_cache_paths`), so a test that builds its + /// expected paths from the raw `TempDir::path()` compares two + /// spellings of one directory. Only bites where the temp root + /// crosses a symlink — macOS puts `$TMPDIR` under + /// `/var` → `/private/var`; Linux's `/tmp` is a real directory. + fn temp_root() -> (tempfile::TempDir, PathBuf) { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path().canonicalize().unwrap(); + (dir, root) + } + #[test] fn manifest_path_dep_wins_over_stale_git_lock_entry() { let tmp = tempfile::tempdir().unwrap(); @@ -6016,10 +6030,10 @@ your_instance_name : primary_clock\n\ // metroid → cips → vivado-cmd. Asking for metroid's deps // transitively should return cips AND vivado-cmd, even though // metroid only declares cips. - let dir = tempfile::tempdir().unwrap(); - let metroid = dir.path().join("metroid"); - let cips = dir.path().join("cips"); - let vivado_cmd = dir.path().join("vivado-cmd"); + let (_dir, root) = temp_root(); + let metroid = root.join("metroid"); + let cips = root.join("cips"); + let vivado_cmd = root.join("vivado-cmd"); std::fs::create_dir_all(&metroid).unwrap(); std::fs::create_dir_all(&cips).unwrap(); std::fs::create_dir_all(&vivado_cmd).unwrap(); @@ -6062,12 +6076,12 @@ your_instance_name : primary_clock\n\ // `shared` is whichever was inserted first; entry itself // doesn't declare `shared`, so the test just asserts we got // *one* deterministic answer rather than a panic / duplicate. - let dir = tempfile::tempdir().unwrap(); - let entry = dir.path().join("entry"); - let a = dir.path().join("a"); - let b = dir.path().join("b"); - let shared_v1 = dir.path().join("shared-v1"); - let shared_v2 = dir.path().join("shared-v2"); + let (_dir, root) = temp_root(); + let entry = root.join("entry"); + let a = root.join("a"); + let b = root.join("b"); + let shared_v1 = root.join("shared-v1"); + let shared_v2 = root.join("shared-v2"); for d in [&entry, &a, &b, &shared_v1, &shared_v2] { std::fs::create_dir_all(d).unwrap(); } diff --git a/vw-repl/src/lower.rs b/vw-repl/src/lower.rs index b01a7ef..b91089a 100644 --- a/vw-repl/src/lower.rs +++ b/vw-repl/src/lower.rs @@ -884,6 +884,13 @@ impl ScratchFile { let path = dir.join(name); let mut f = std::fs::File::create(&path)?; f.write_all(contents.as_bytes())?; + // The loader canonicalizes every path it records, so hold the + // canonical form: `file.path == scratch_path` is what marks a + // frame as "the line you just typed" rather than a file, and + // under a workspace path that crosses a symlink the two + // spellings never match — every REPL error would then cite + // `.vw-repl-input-.htcl` as its source file. + let path = path.canonicalize().unwrap_or(path); Ok(Self { path }) } }