diff --git a/Cargo.lock b/Cargo.lock index a52dae8..c39f7fa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -322,6 +322,12 @@ dependencies = [ "syn", ] +[[package]] +name = "dunce" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813" + [[package]] name = "eph" version = "0.0.0" @@ -330,6 +336,7 @@ dependencies = [ "bollard", "clap", "dirs", + "dunce", "futures-util", "globset", "hex", diff --git a/Cargo.toml b/Cargo.toml index 502cbff..d40e496 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,6 +20,12 @@ anyhow = "1" bollard = "0.21" clap = { version = "4", features = ["derive"] } dirs = "6" +# Canonicalizing a workspace path with std on Windows yields the extended-length +# `\\?\C:\...` form. Docker's Windows volume parser rejects that as a bind-mount +# source, and it also leaks into display output. dunce::canonicalize returns a +# plain `C:\...` path whenever one exists (the common case), keeping the verbatim +# prefix only for paths that genuinely require it. On Unix it is std::fs::canonicalize. +dunce = "1" futures-util = "0.3" # `eph dev --watch` compiles the user's globs into a matcher and watches the # workspace tree for changes: globset for gitignore-style glob matching, notify diff --git a/docs/user-guide/troubleshooting.md b/docs/user-guide/troubleshooting.md index 3ee20e1..4239326 100644 --- a/docs/user-guide/troubleshooting.md +++ b/docs/user-guide/troubleshooting.md @@ -168,6 +168,35 @@ When you run under WSL, `eph` is a Linux process, so its state directory is the `%LOCALAPPDATA%` path. The `%LOCALAPPDATA%` location applies only to a native Windows build. +### Relative bind mounts on Windows + +A relative host bind mount (`volume=./seed:/docker-entrypoint-initdb.d`) resolves +against the workspace root, which eph now stores as a plain `C:\...` path. Older +builds (up to v0.3.1) stored the canonical path in Windows' extended-length +`\\?\C:\...` form and forwarded it to Docker as the bind source; the daemon +rejects that form with a garbled error: + +``` +Error: failed to create container +Caused by: + Docker responded with status code 500: \?\C%!(EXTRA string=is not a valid Windows path) +``` + +If you hit this, update eph: relative binds now resolve to a source Docker +accepts. Two knock-on notes: + +- **Workspace IDs changed on Windows.** The ID is derived from the workspace + path, and dropping the `\\?\` prefix changes it. Containers, named volumes, and + the state directory from an older build live under the previous ID, so eph no + longer sees them. Run `docker ps -a` / `docker volume ls` and remove any + leftover `eph--*` containers and volumes by hand, then + `eph up` fresh. Named volumes did work before this fix, so this cleanup only + matters if you had services running under an older build. +- **Very long workspace paths.** If the workspace sits deep enough that it has no + ordinary `C:\...` representation, the path keeps the `\\?\` prefix and eph + rejects the bind mount up front with a clear message rather than passing an + unusable source to Docker. Move the workspace to a shorter path. + ## Getting more detail `eph -v ` enables debug logging (to stderr). For a service's own diff --git a/src/service.rs b/src/service.rs index 19a0810..b4ee234 100644 --- a/src/service.rs +++ b/src/service.rs @@ -497,32 +497,71 @@ fn parse_command_override( /// service (via [`Workspace::volume_name`]) so two workspaces, or two services, /// never collide on a shared volume. A spec without a `:` half /// is passed through unchanged, so Docker reports the malformed mount itself. -fn resolve_volume_spec(spec: &str, workspace: &Workspace, service_name: &str) -> String { +/// +/// # Errors +/// +/// Returns an error if the resolved host source is a Windows extended-length +/// (`\\?\`) path that Docker cannot use as a mount source (see +/// [`reject_verbatim_bind_source`]), or if a relative source resolves to a path +/// that is not valid UTF-8. +fn resolve_volume_spec(spec: &str, workspace: &Workspace, service_name: &str) -> Result { if spec.starts_with('/') || spec.starts_with('.') { // Host-path bind mount. let parts: Vec<&str> = spec.splitn(2, ':').collect(); if parts.len() == 2 { let host_path = if parts[0].starts_with('.') { - workspace.path.join(parts[0]).to_string_lossy().to_string() + let joined = workspace.path.join(parts[0]); + // to_str, not to_string_lossy: a lossy replacement in a bind + // source would silently mount the wrong host path. + joined + .to_str() + .with_context(|| { + format!("bind mount source {} is not valid UTF-8", joined.display()) + })? + .to_string() } else { parts[0].to_string() }; - format!("{}:{}", host_path, parts[1]) + reject_verbatim_bind_source(&host_path)?; + Ok(format!("{}:{}", host_path, parts[1])) } else { - spec.to_string() + Ok(spec.to_string()) } } else { // Named volume, namespaced to the workspace + service. let parts: Vec<&str> = spec.splitn(2, ':').collect(); if parts.len() == 2 { let volume_name = workspace.volume_name(service_name, parts[0]); - format!("{}:{}", volume_name, parts[1]) + Ok(format!("{}:{}", volume_name, parts[1])) } else { - spec.to_string() + Ok(spec.to_string()) } } } +/// Reject a Windows extended-length ("verbatim") bind-mount source. +/// +/// Docker's Windows volume parser rejects the `\\?\C:\...` and `\\?\UNC\...` +/// forms that `std`'s canonicalization emits, responding with a garbled +/// `\?\C%!(EXTRA string=is not a valid Windows path)` (the `%!(EXTRA ...)` is an +/// upstream moby `fmt` artifact). eph normalizes the workspace root away from +/// that form in [`Workspace::from_path`] via `dunce::canonicalize`, so this only +/// fires for a path long enough to have no ordinary Win32 representation: the +/// root keeps its `\\?\` prefix and a relative bind resolved against it inherits +/// it. Fail closed with an actionable message rather than forwarding a source the +/// daemon will only reject cryptically. +fn reject_verbatim_bind_source(source: &str) -> Result<()> { + if source.starts_with(r"\\?\") { + bail!( + "bind mount source `{source}` is a Windows extended-length (\\\\?\\) path, \ + which Docker cannot use as a mount source. This happens when the workspace \ + path is long enough to require that prefix; move the workspace to a shorter \ + path and run eph again." + ); + } + Ok(()) +} + /// Poll `probe` until it yields a result or `timeout_dur` elapses, sleeping /// `interval` between attempts. /// @@ -1152,7 +1191,7 @@ impl DockerClient { .volumes .iter() .map(|v| resolve_volume_spec(v, workspace, &service.name)) - .collect(); + .collect::>>()?; let host_config = HostConfig { port_bindings: Some(port_bindings), @@ -3145,7 +3184,7 @@ mod tests { // A bare name is namespaced to `eph---` so two // workspaces or services never share a volume. assert_eq!( - resolve_volume_spec("data:/var/lib/postgresql/data", &ws, "db"), + resolve_volume_spec("data:/var/lib/postgresql/data", &ws, "db").unwrap(), "eph-abcd1234-db-data:/var/lib/postgresql/data" ); } @@ -3155,7 +3194,7 @@ mod tests { let ws = test_workspace("/ws"); // An absolute host path is a bind mount used verbatim (not namespaced). assert_eq!( - resolve_volume_spec("/host/path:/in/container", &ws, "db"), + resolve_volume_spec("/host/path:/in/container", &ws, "db").unwrap(), "/host/path:/in/container" ); } @@ -3169,7 +3208,7 @@ mod tests { PathBuf::from("/ws").join("./data").to_string_lossy() ); assert_eq!( - resolve_volume_spec("./data:/in/container", &ws, "db"), + resolve_volume_spec("./data:/in/container", &ws, "db").unwrap(), expected ); } @@ -3179,9 +3218,49 @@ mod tests { let ws = test_workspace("/ws"); // No `:` half: passed through unchanged (Docker reports // the malformed mount). Holds for both the named and host-path branches. - assert_eq!(resolve_volume_spec("justaname", &ws, "db"), "justaname"); - assert_eq!(resolve_volume_spec("/abs/only", &ws, "db"), "/abs/only"); - assert_eq!(resolve_volume_spec("./rel/only", &ws, "db"), "./rel/only"); + assert_eq!( + resolve_volume_spec("justaname", &ws, "db").unwrap(), + "justaname" + ); + assert_eq!( + resolve_volume_spec("/abs/only", &ws, "db").unwrap(), + "/abs/only" + ); + assert_eq!( + resolve_volume_spec("./rel/only", &ws, "db").unwrap(), + "./rel/only" + ); + } + + #[test] + fn resolve_volume_spec_relative_bind_against_plain_windows_root_is_clean() { + // Regression for #44: with the workspace path normalized to a plain + // `C:\...` form (as dunce::canonicalize now yields), a relative bind + // resolves to a source Docker accepts, with no `\\?\` prefix. + let ws = test_workspace(r"C:\Users\me\project"); + let resolved = resolve_volume_spec("./seed:/docker-entrypoint-initdb.d", &ws, "postgres") + .expect("plain Windows root must resolve cleanly"); + assert!( + !resolved.starts_with(r"\\?\"), + "resolved source must not carry the extended-length prefix: {resolved}" + ); + assert!(resolved.ends_with(":/docker-entrypoint-initdb.d")); + } + + #[test] + fn resolve_volume_spec_rejects_verbatim_relative_source() { + // Regression for #44: if the workspace path could not be normalized (a + // genuine long path keeps the `\\?\` prefix), a relative bind that + // resolves onto it is rejected here with an actionable error rather than + // forwarded to Docker, which would reject it cryptically. + let ws = test_workspace(r"\\?\C:\Users\me\project"); + let err = resolve_volume_spec("./seed:/in/container", &ws, "db") + .expect_err("a verbatim source must be rejected"); + let msg = err.to_string(); + assert!( + msg.contains("extended-length"), + "error should explain the extended-length path: {msg}" + ); } #[test] diff --git a/src/watch.rs b/src/watch.rs index 8f17227..23f8452 100644 --- a/src/watch.rs +++ b/src/watch.rs @@ -67,8 +67,10 @@ impl Watch { // Match against the canonical root so `strip_prefix` succeeds regardless // of how the caller spelled the path or how the OS reports events. - let root = root - .canonicalize() + // dunce::canonicalize matches Workspace::from_path: on Windows it avoids + // the extended-length `\\?\` prefix, so the root here lines up with the + // stored workspace path and with the paths notify reports. + let root = dunce::canonicalize(root) .with_context(|| format!("resolving watch root {}", root.display()))?; let (tx, rx) = mpsc::unbounded_channel(); diff --git a/src/workspace.rs b/src/workspace.rs index 4fa2d2b..92f9bdc 100644 --- a/src/workspace.rs +++ b/src/workspace.rs @@ -30,7 +30,9 @@ impl Workspace { /// /// The path is canonicalized, so the resulting [`id`](Self::id) is stable /// regardless of how the directory was addressed (relative path, symlink, - /// etc.). + /// etc.). On Windows the canonical form omits the extended-length `\\?\` + /// prefix (via `dunce`) whenever a plain `C:\...` path exists, so the stored + /// path is one Docker and the display code can use directly. /// /// # Errors /// @@ -49,7 +51,11 @@ impl Workspace { /// # } /// ``` pub fn from_path(path: impl AsRef) -> Result { - let path = path.as_ref().canonicalize().with_context(|| { + // dunce::canonicalize, not std's: on Windows std returns the + // extended-length `\\?\C:\...` form, which Docker rejects as a bind-mount + // source and which leaks into display output. dunce yields a plain + // `C:\...` path whenever one exists. On Unix it is std::fs::canonicalize. + let path = dunce::canonicalize(path.as_ref()).with_context(|| { format!( "failed to resolve workspace path: {}", path.as_ref().display()