Windows: Fix issues with paths in extensions (#37811)
### Background Zed extensions use WASI to access the file-system. They only have read-write access to one specific folder called their work dir. But extensions do need to be able to *refer* to other arbitrary files on the user's machine. For instance, extensions need to be able to look up existing binaries on the user's `PATH`, and request that Zed invoke them as language servers. Similarly, extensions can create paths to files in the user's project, and use them as arguments in commands that Zed should run. For these reasons, we pass *real* paths back and forth between the host and extensions; we don't try to abstract over the file-system with some virtualization scheme. On Windows, this results in a bit of mismatch, because `wasi-libc` uses *unix-like* path conventions (and thus, so does the Rust standard library when compiling to WASI). ### Change 1 - Fixing `current_dir` In order to keep the extension API minimal, extensions use the standard library function`env::current_dir()` to query the location of their "work" directory. Previously, when initializing extensions, we used the `env::set_current_dir` function to set their work directory, but on Windows, where absolute paths typically begin with a drive letter, like `C:`, the [`wasi-libc` implementation of `chdir`](https://github.com/WebAssembly/wasi-libc/blob/d1793637d8afcdc730408e7b6a19a050c3336ce7/libc-bottom-half/sources/chdir.c#L21) was prepending an extra forward slash to the path, which caused `current_dir()` to return an invalid path. See https://github.com/bytecodealliance/wasmtime/issues/10415 In this PR, I've switched our extension initialization function to *bypass* wasi-libc's `chdir` function, and instead write directly to wasi-libc's private, internal state. This is a bit of a hack, but it causes the `current_dir()` function to do what we want on Windows without any changes to extensions' source code. ### Change 2 - Working around WASI's relative path handling Once `current_dir` was fixed (giving us correct absolute paths on Windows), @kubkon and I discovered that without the spurious leading `/` character, windows absolute paths were no longer accepted by Rust's `std::fs` APIs, because they were now recognized as relative paths, and were being appended to the working directory. We first tried to override the `__wasilibc_find_abspath` function in `wasi-libc` to make it recognize windows absolute paths as being absolute, but that functionality is difficult to override. Eventually @kubkon realized that we could prevent WASI-libc's CWD handling from being linked into the WASM file by overriding the `chdir` function. wasi-libc is designed so that if you don't use their `chdir` function, then all paths will be interpreted as relative to `/`. This makes absolute paths behave correctly. Then, in order to make *relative* paths work again, we simply add a preopen for `.`. Relative paths will match that. ### Next Steps This is a change to `zed-extension-api`, so we do need to update every Zed extension to use the new version, in order for them to work on windows. Release Notes: - N/A --------- Co-authored-by: Jakub Konka <kubkon@jakubkonka.com>
This commit is contained in:
co-authored by
Jakub Konka
parent
d5d30b5c44
commit
c4d75ea6d5
@@ -17,4 +17,9 @@ commit = "8432ffe32ccd360534837256747beb5b1c82fca1"
|
||||
[[capabilities]]
|
||||
kind = "process:exec"
|
||||
command = "echo"
|
||||
args = ["hello!"]
|
||||
args = ["hello from a child process!"]
|
||||
|
||||
[[capabilities]]
|
||||
kind = "process:exec"
|
||||
command = "cmd"
|
||||
args = ["/C", "echo", "hello from a child process!"]
|
||||
|
||||
@@ -14,9 +14,37 @@ impl TestExtension {
|
||||
language_server_id: &LanguageServerId,
|
||||
_worktree: &zed::Worktree,
|
||||
) -> Result<String> {
|
||||
let echo_output = Command::new("echo").arg("hello!").output()?;
|
||||
let (platform, arch) = zed::current_platform();
|
||||
|
||||
println!("{}", String::from_utf8_lossy(&echo_output.stdout));
|
||||
let current_dir = std::env::current_dir().unwrap();
|
||||
println!("current_dir: {}", current_dir.display());
|
||||
|
||||
fs::create_dir_all(current_dir.join("dir-created-with-abs-path")).unwrap();
|
||||
fs::create_dir_all("./dir-created-with-rel-path").unwrap();
|
||||
fs::write("file-created-with-rel-path", b"contents 1").unwrap();
|
||||
fs::write(
|
||||
current_dir.join("file-created-with-abs-path"),
|
||||
b"contents 2",
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
fs::read("file-created-with-rel-path").unwrap(),
|
||||
b"contents 1"
|
||||
);
|
||||
assert_eq!(
|
||||
fs::read("file-created-with-abs-path").unwrap(),
|
||||
b"contents 2"
|
||||
);
|
||||
|
||||
let command = match platform {
|
||||
zed::Os::Linux | zed::Os::Mac => Command::new("echo"),
|
||||
zed::Os::Windows => Command::new("cmd").args(["/C", "echo"]),
|
||||
};
|
||||
let output = command.arg("hello from a child process!").output()?;
|
||||
println!(
|
||||
"command output: {}",
|
||||
String::from_utf8_lossy(&output.stdout).trim()
|
||||
);
|
||||
|
||||
if let Some(path) = &self.cached_binary_path
|
||||
&& fs::metadata(path).is_ok_and(|stat| stat.is_file())
|
||||
@@ -36,9 +64,18 @@ impl TestExtension {
|
||||
},
|
||||
)?;
|
||||
|
||||
let (platform, arch) = zed::current_platform();
|
||||
let ext = "tar.gz";
|
||||
let download_type = zed::DownloadedFileType::GzipTar;
|
||||
|
||||
// Do this if you want to actually run this extension -
|
||||
// the actual asset is a .zip. But the integration test is simpler
|
||||
// if every platform uses .tar.gz.
|
||||
//
|
||||
// ext = "zip";
|
||||
// download_type = zed::DownloadedFileType::Zip;
|
||||
|
||||
let asset_name = format!(
|
||||
"gleam-{version}-{arch}-{os}.tar.gz",
|
||||
"gleam-{version}-{arch}-{os}.{ext}",
|
||||
version = release.version,
|
||||
arch = match arch {
|
||||
zed::Architecture::Aarch64 => "aarch64",
|
||||
@@ -67,18 +104,21 @@ impl TestExtension {
|
||||
&zed::LanguageServerInstallationStatus::Downloading,
|
||||
);
|
||||
|
||||
zed::download_file(
|
||||
&asset.download_url,
|
||||
&version_dir,
|
||||
zed::DownloadedFileType::GzipTar,
|
||||
)
|
||||
.map_err(|e| format!("failed to download file: {e}"))?;
|
||||
zed::download_file(&asset.download_url, &version_dir, download_type)
|
||||
.map_err(|e| format!("failed to download file: {e}"))?;
|
||||
|
||||
zed::set_language_server_installation_status(
|
||||
language_server_id,
|
||||
&zed::LanguageServerInstallationStatus::None,
|
||||
);
|
||||
|
||||
let entries =
|
||||
fs::read_dir(".").map_err(|e| format!("failed to list working directory {e}"))?;
|
||||
for entry in entries {
|
||||
let entry = entry.map_err(|e| format!("failed to load directory entry {e}"))?;
|
||||
if entry.file_name().to_str() != Some(&version_dir) {
|
||||
let filename = entry.file_name();
|
||||
let filename = filename.to_str().unwrap();
|
||||
if filename.starts_with("gleam-") && filename != version_dir {
|
||||
fs::remove_dir_all(entry.path()).ok();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user