Leading slash with env::current_dir() in Windows #20559

alvgaona · 2024-11-12T21:19:01Z

Check for existing issues

Completed

Describe the bug / provide steps to reproduce it

The issue is related to LSPs not spinning up due to a message like this one:

Language server error: astro-language-server

oneshot canceled
-- stderr--
node:internal/modules/cjs/loader:1051
  throw err;
  ^

Error: Cannot find module 'E:\C:\Users\alvga\AppData\Local\Zed\extensions\work\astro_1\node_modules\@astrojs\language-server\bin\nodeServer.js'
    at Module._resolveFilename (node:internal/modules/cjs/loader:1048:15)
    at Module._load (node:internal/modules/cjs/loader:901:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:83:12)
    at node:internal/main/run_main_module:23:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

Node.js v20.5.0

or this one:

Language server error: svelte-language-server

oneshot canceled
-- stderr--
node:internal/modules/cjs/loader:1051
  throw err;
  ^

Error: Cannot find module 'E:\C:\Users\alvga\AppData\Local\Zed\extensions\work\svelte\node_modules\svelte-language-server\bin\server.js'
    at Module._resolveFilename (node:internal/modules/cjs/loader:1048:15)
    at Module._load (node:internal/modules/cjs/loader:901:27)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:83:12)
    at node:internal/main/run_main_module:23:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}

Node.js v20.5.0

The problem is related to the WASM runtime in which the extensions run. The env::current_dir() is abstracted away from the base platform in which Zed is running. You can check this with env::constants::OS which returns an empty string.

env::current_dir() returns this path: "/C:\\Users\\alvga\\AppData\\Local\\Zed\\extensions\\work\\astro_1/node_modules/@astrojs/language-server/bin/nodeServer.js". You will see a leading forward slash which then is passed to the zed::Command and interprets it as some disk, and ultimately having the first presented log.

This may not only impact extensions but maybe other crates as well.

Environment

Zed: v0.162.0 (Zed Dev 4ca1837)
OS: Windows 10.0.22631
Memory: 47.9 GiB
Architecture: x86_64
GPU: NVIDIA GeForce RTX 3060 || NVIDIA || 565.90

If applicable, add mockups / screenshots to help explain present your vision of the feature

No response

If applicable, attach your Zed.log file to this issue.

No response

The text was updated successfully, but these errors were encountered:

alvgaona · 2024-11-12T21:19:42Z

Relates to

Astro LSP path leading slash in Windows #20552

SomeoneToIgnore · 2024-11-12T21:49:16Z

I have looked around and found one peculiarity:

zed/crates/extension/src/extension_builder.rs

Line 42 in 181c372

Some("wasi-sdk-21.0.m-mingw.tar.gz")

What if we try wasi-sdk-23.0-x86_64-windows.tar.gz there instead? Or the latest, v24?

alvgaona · 2024-11-12T23:02:29Z

I have looked around and found one peculiarity:

zed/crates/extension/src/extension_builder.rs

Line 42 in 181c372

Some("wasi-sdk-21.0.m-mingw.tar.gz")

What if we try wasi-sdk-23.0-x86_64-windows.tar.gz there instead? Or the latest, v24?

Nothing changes with v23 and windows instead of mingw.

SomeoneToIgnore · 2024-11-13T07:52:37Z

Thank you, good to know.
IIRC mingw used such path notation in git-bash, so odd that it's not it.

One other thing I was pointed at is

zed/crates/extension_host/src/wasm_host.rs

Lines 313 to 315 in 55cd99c

    
           fn work_dir(&self) -> PathBuf { 
        
               self.host.work_dir.join(self.manifest.id.as_ref()) 
        
           }

and this:

zed/crates/extension_host/src/wasm_host.rs

Lines 161 to 183 in 55cd99c

    
           async fn build_wasi_ctx(&self, manifest: &Arc<ExtensionManifest>) -> Result<wasi::WasiCtx> { 
        
               let extension_work_dir = self.work_dir.join(manifest.id.as_ref()); 
        
               self.fs 
        
                   .create_dir(&extension_work_dir) 
        
                   .await 
        
                   .context("failed to create extension work dir")?; 
        
               let file_perms = wasi::FilePerms::all(); 
        
               let dir_perms = wasi::DirPerms::all(); 
        
               Ok(wasi::WasiCtxBuilder::new() 
        
                   .inherit_stdio() 
        
                   .preopened_dir(&extension_work_dir, ".", dir_perms, file_perms)? 
        
                   .preopened_dir( 
        
                       &extension_work_dir, 
        
                       extension_work_dir.to_string_lossy(), 
        
                       dir_perms, 
        
                       file_perms, 
        
                   )? 
        
                   .env("PWD", extension_work_dir.to_string_lossy()) 
        
                   .env("RUST_BACKTRACE", "full") 
        
                   .build()) 
        
           }

could we debug and check what kind of paths dwell there?

Or I can check this tomorrow when I'm back to my Win machine.

SomeoneToIgnore · 2024-11-14T10:39:27Z

To answer my question, seems that we're doing fine here:

[crates\extension_host\src\wasm_host.rs:268:25] &extension_work_dir = "C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work\\html"
[crates\extension_host\src\wasm_host.rs:258:27] this.work_dir.clone() = "C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work"
[crates\extension_host\src\wasm_host.rs:268:25] &extension_work_dir = "C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work\\proto"
[crates\extension_host\src\wasm_host.rs:258:27] this.work_dir.clone() = "C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work"
[crates\extension_host\src\wasm_host.rs:268:25] &extension_work_dir = "C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work\\toml"
[crates\extension_host\src\wasm_host.rs:258:27] this.work_dir.clone() = "C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work"

for a patch

diff --git a/crates/extension_host/src/wasm_host.rs b/crates/extension_host/src/wasm_host.rs
index bd66a31a27..e28245d037 100644
--- a/crates/extension_host/src/wasm_host.rs
+++ b/crates/extension_host/src/wasm_host.rs
@@ -255,7 +255,7 @@ impl WasmHost {

             Ok(WasmExtension {
                 manifest: manifest.clone(),
-                work_dir: this.work_dir.clone().into(),
+                work_dir: dbg!(this.work_dir.clone()).into(),
                 tx,
                 zed_api_version,
             })
@@ -265,7 +265,7 @@ impl WasmHost {
     async fn build_wasi_ctx(&self, manifest: &Arc<ExtensionManifest>) -> Result<wasi::WasiCtx> {
         let extension_work_dir = self.work_dir.join(manifest.id.as_ref());
         self.fs
-            .create_dir(&extension_work_dir)
+            .create_dir(dbg!(&extension_work_dir))
             .await
             .context("failed to create extension work dir")?;

I still find it very suspicious that / -prefixed paths look exactly how my MINGW terminal does this:

Given that using the upgraded version of the SDK did not help and it was mingw-only version before, I lean to the fact that this is a bug in wasi.

The next step we'd better do then, is creating a minimal examle to repro this, and submitting this to their repo.

alvgaona · 2024-11-14T12:34:04Z

To answer my question, seems that we're doing fine here:

[crates\extension_host\src\wasm_host.rs:268:25] &extension_work_dir = "C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work\\html"
[crates\extension_host\src\wasm_host.rs:258:27] this.work_dir.clone() = "C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work"
[crates\extension_host\src\wasm_host.rs:268:25] &extension_work_dir = "C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work\\proto"
[crates\extension_host\src\wasm_host.rs:258:27] this.work_dir.clone() = "C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work"
[crates\extension_host\src\wasm_host.rs:268:25] &extension_work_dir = "C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work\\toml"
[crates\extension_host\src\wasm_host.rs:258:27] this.work_dir.clone() = "C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work"

for a patch

diff --git a/crates/extension_host/src/wasm_host.rs b/crates/extension_host/src/wasm_host.rs
index bd66a31a27..e28245d037 100644
--- a/crates/extension_host/src/wasm_host.rs
+++ b/crates/extension_host/src/wasm_host.rs
@@ -255,7 +255,7 @@ impl WasmHost {

             Ok(WasmExtension {
                 manifest: manifest.clone(),
-                work_dir: this.work_dir.clone().into(),
+                work_dir: dbg!(this.work_dir.clone()).into(),
                 tx,
                 zed_api_version,
             })
@@ -265,7 +265,7 @@ impl WasmHost {
     async fn build_wasi_ctx(&self, manifest: &Arc<ExtensionManifest>) -> Result<wasi::WasiCtx> {
         let extension_work_dir = self.work_dir.join(manifest.id.as_ref());
         self.fs
-            .create_dir(&extension_work_dir)
+            .create_dir(dbg!(&extension_work_dir))
             .await
             .context("failed to create extension work dir")?;

I still find it very suspicious that / -prefixed paths look exactly how my MINGW terminal does this:

Given that using the upgraded version of the SDK did not help and it was mingw-only version before, I lean to the fact that this is a bug in wasi.

The next step we'd better do then, is creating a minimal examle to repro this, and submitting this to their repo.

Would you like to test the windows v23 SDK yourself? In case I have done something wrong with it. Before we even file an issue to them.

SomeoneToIgnore · 2024-11-14T12:35:26Z

I did retest it by changing all SDK versions in the code and recompiling the html plugin in dev mode with it, so not sure we are missing anything.

someone120 · 2024-11-28T11:59:25Z

The working directory has been changed using the PWD environment variable here, but this variable cannot be accessed under Windows, which may be related to this issue.

zed/crates/extension_api/src/extension_api.rs

Line 169 in 6cba467

std::env::set_current_dir(std::env::var("PWD").unwrap()).unwrap();

SomeoneToIgnore · 2024-11-28T18:42:00Z

It's not that, apparently, if I change the extension as

diff --git a/crates/extension_api/src/extension_api.rs b/crates/extension_api/src/extension_api.rs
index 4bb1229538..7d7f17b1b3 100644
--- a/crates/extension_api/src/extension_api.rs
+++ b/crates/extension_api/src/extension_api.rs
@@ -166,6 +166,8 @@ macro_rules! register_extension {
     ($extension_type:ty) => {
         #[export_name = "init-extension"]
         pub extern "C" fn __init_extension() {
+            let a = std::env::var("PWD");
+            eprintln!("??????????????????? {:?}", a);
             std::env::set_current_dir(std::env::var("PWD").unwrap()).unwrap();
             zed_extension_api::register_extension(|| {
                 Box::new(<$extension_type as zed_extension_api::Extension>::new())
diff --git a/extensions/html/Cargo.toml b/extensions/html/Cargo.toml
index e8f8a741bd..e9ee29f203 100644
--- a/extensions/html/Cargo.toml
+++ b/extensions/html/Cargo.toml
@@ -13,4 +13,4 @@ path = "src/html.rs"
 crate-type = ["cdylib"]

 [dependencies]
-zed_extension_api = "0.1.0"
+zed_extension_api = { path = "../../crates/extension_api" }

and install the html extension as a dev one, I see

??????????????????? Ok("C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work\\html")

JunkuiZhang · 2024-11-30T14:23:22Z

It's not that, apparently, if I change the extension as

diff --git a/crates/extension_api/src/extension_api.rs b/crates/extension_api/src/extension_api.rs
index 4bb1229538..7d7f17b1b3 100644
--- a/crates/extension_api/src/extension_api.rs
+++ b/crates/extension_api/src/extension_api.rs
@@ -166,6 +166,8 @@ macro_rules! register_extension {
     ($extension_type:ty) => {
         #[export_name = "init-extension"]
         pub extern "C" fn __init_extension() {
+            let a = std::env::var("PWD");
+            eprintln!("??????????????????? {:?}", a);
             std::env::set_current_dir(std::env::var("PWD").unwrap()).unwrap();
             zed_extension_api::register_extension(|| {
                 Box::new(<$extension_type as zed_extension_api::Extension>::new())
diff --git a/extensions/html/Cargo.toml b/extensions/html/Cargo.toml
index e8f8a741bd..e9ee29f203 100644
--- a/extensions/html/Cargo.toml
+++ b/extensions/html/Cargo.toml
@@ -13,4 +13,4 @@ path = "src/html.rs"
 crate-type = ["cdylib"]

 [dependencies]
-zed_extension_api = "0.1.0"
+zed_extension_api = { path = "../../crates/extension_api" }

and install the html extension as a dev one, I see

??????????????????? Ok("C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work\\html")

I think the issue lies with std::env::current_dir(). When working on PR #14905, I conducted extensive tests and concluded that under WASI, the path returned by std::env::current_dir() will always start with /.

You can test this with your current modifications by making the following changes to your code:

macro_rules! register_extension {
    ($extension_type:ty) => {
        #[export_name = "init-extension"]
        pub extern "C" fn __init_extension() {
            let a = std::env::var("PWD");
            eprintln!("PWD: {:?}", a);
            std::env::set_current_dir(std::env::var("PWD").unwrap()).unwrap();
            eprintln!("Get: {:?}", std::env::current_dir());      // Add this line
            zed_extension_api::register_extension(|| {
                Box::new(<$extension_type as zed_extension_api::Extension>::new())

Which prints something like:

PWD: Ok("C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work\\html")
Get: Ok("/C:\\Users\\awpc\\AppData\\Local\\Zed\\extensions\\work\\html")

And under WASI, std::env::current_dir() internally calls libc::getcwd(), see here. Since we cannot modify the implementation of libc, I don’t think this issue can be easily resolved. Therefore, I think introducing an additional function, as done in PR #14905, might be the only viable solution.

someone120 · 2024-12-01T07:10:28Z

Is wasmtime's problem. We need turn problem to upstream

someone120 · 2024-12-01T08:49:28Z

Is wasmtime's problem. We need turn problem to upstream

Not an issue.wasmtime provides a sandbox environment that is similar to the Unix environment.That is why the first slash exists.

BERADQ · 2024-12-27T18:48:50Z

I believe the only thing we can do is to remove the leading slash from the string in the Windows environment. :(

someone120 · 2024-12-28T04:47:44Z

I think it is only way we can do.

…

________________________________ 发件人: Ninnana ***@***.***> 发送时间: 2024年12月28日 2:49 收件人: zed-industries/zed ***@***.***> 抄送: Yakira ***@***.***>; Comment ***@***.***> 主题: Re: [zed-industries/zed] Leading slash with env::current_dir() in Windows (Issue #20559) I believe the only thing we can do is to remove the leading slash from the string in the Windows environment. :( ― Reply to this email directly, view it on GitHub<#20559 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ALL6V3M6DLEB3MYSV5V26UL2HWOKTAVCNFSM6AAAAABRU7OCGOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNRTHE2DQOJUGE>. You are receiving this because you commented.Message ID: ***@***.***>

someone120 · 2024-12-29T10:19:58Z

Why did I go to check Wasmtime? It's because wasm runs within Wasmtime, and WASI is an interface that provides wasm with the ability to access files, but does not have the capability to run wasm.

someone120 · 2024-12-29T10:22:19Z

Now we have two options: one is to choose another wasm runtime, and the other is to simply remove the leading slash. However, the first option is too complex, involving too many modules. Therefore, we can only choose the second option.

someone120 · 2024-12-29T15:09:46Z

i create a pull request on zed-extensions/vue#9.

I used regex instead of cfg!(target_os = "windows") because latter doesn't run as expect. it doesn't compile the windows code on windows device.

BERADQ · 2024-12-29T16:10:11Z

i create a pull request on zed-extensions/vue#9.

I used regex instead of cfg!(target_os = "windows") because latter wouldn't run as expect. it wouldn't compile the windows code on windows device.

Because its compilation target is wasm instead of Windows.
It's not reasonable to write the same code for each extension or even use regular expressions. Instead, we should process strings in the host, so that cfg!(target_os = "windows") will work as expected.

someone120 · 2025-01-02T16:40:54Z

i create a pull request on zed-extensions/vue#9.

I used regex instead of cfg!(target_os = "windows") because latter wouldn't run as expect. it wouldn't compile the windows code on windows device.

Because its compilation target is wasm instead of Windows.

It's not reasonable to write the same code for each extension or even use regular expressions. Instead, we should process strings in the host, so that cfg!(target_os = "windows") will work as expected.

I'm working on it.

alvgaona added admin read Pending admin review bug [core label] triage Maintainer needs to classify the issue labels Nov 12, 2024

github-actions bot mentioned this issue Nov 16, 2024

Top-Ranking Issues (last 7 days) 📊 #6952

Open

This was referenced Nov 28, 2024

Windows: LSP doesn't work #19429

Open

[Windows] Astro LSP fails to launch #21199

Closed

alvgaona mentioned this issue Nov 30, 2024

[Windows] Failed to start language server, generated path seems invalid #17571

Closed

1 task

SomeoneToIgnore mentioned this issue Dec 7, 2024

Failing to Load vscode-html-language-server in windows machine #21677

Closed

1 task

zed-industries deleted a comment from dylansantwani Dec 10, 2024

notpeter mentioned this issue Dec 11, 2024

html-language-server not installing (Windows) #18519

Closed

1 task

github-actions bot mentioned this issue Dec 26, 2024

Top-Ranking Issues (All Time) 📊 #5393

Open

someone120 added a commit to someone120/zed that referenced this issue Jan 3, 2025

Fix leading slash on windows (zed-industries#20559)

5ccd2dc

someone120 mentioned this issue Jan 3, 2025

windows: Fix leading slash on windows #22600

Closed

someone120 added a commit to someone120/zed that referenced this issue Jan 3, 2025

Fix leading slash on windows (zed-industries#20559)

e6d08e4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Leading slash with env::current_dir() in Windows #20559

Leading slash with env::current_dir() in Windows #20559

alvgaona commented Nov 12, 2024

alvgaona commented Nov 12, 2024 •

edited by notpeter

Loading

SomeoneToIgnore commented Nov 12, 2024 •

edited

Loading

alvgaona commented Nov 12, 2024 •

edited

Loading

SomeoneToIgnore commented Nov 13, 2024 •

edited

Loading

SomeoneToIgnore commented Nov 14, 2024

alvgaona commented Nov 14, 2024

SomeoneToIgnore commented Nov 14, 2024 •

edited

Loading

someone120 commented Nov 28, 2024

SomeoneToIgnore commented Nov 28, 2024

JunkuiZhang commented Nov 30, 2024

someone120 commented Dec 1, 2024

someone120 commented Dec 1, 2024

BERADQ commented Dec 27, 2024

someone120 commented Dec 28, 2024 via email

someone120 commented Dec 29, 2024

someone120 commented Dec 29, 2024 •

edited

Loading

someone120 commented Dec 29, 2024 •

edited

Loading

BERADQ commented Dec 29, 2024

someone120 commented Jan 2, 2025

Leading slash with env::current_dir() in Windows #20559

Leading slash with env::current_dir() in Windows #20559

Comments

alvgaona commented Nov 12, 2024

Check for existing issues

Describe the bug / provide steps to reproduce it

Environment

If applicable, add mockups / screenshots to help explain present your vision of the feature

If applicable, attach your Zed.log file to this issue.

alvgaona commented Nov 12, 2024 • edited by notpeter Loading

SomeoneToIgnore commented Nov 12, 2024 • edited Loading

alvgaona commented Nov 12, 2024 • edited Loading

SomeoneToIgnore commented Nov 13, 2024 • edited Loading

SomeoneToIgnore commented Nov 14, 2024

alvgaona commented Nov 14, 2024

SomeoneToIgnore commented Nov 14, 2024 • edited Loading

someone120 commented Nov 28, 2024

SomeoneToIgnore commented Nov 28, 2024

JunkuiZhang commented Nov 30, 2024

someone120 commented Dec 1, 2024

someone120 commented Dec 1, 2024

BERADQ commented Dec 27, 2024

someone120 commented Dec 28, 2024 via email

someone120 commented Dec 29, 2024

someone120 commented Dec 29, 2024 • edited Loading

someone120 commented Dec 29, 2024 • edited Loading

BERADQ commented Dec 29, 2024

someone120 commented Jan 2, 2025

alvgaona commented Nov 12, 2024 •

edited by notpeter

Loading

SomeoneToIgnore commented Nov 12, 2024 •

edited

Loading

alvgaona commented Nov 12, 2024 •

edited

Loading

SomeoneToIgnore commented Nov 13, 2024 •

edited

Loading

SomeoneToIgnore commented Nov 14, 2024 •

edited

Loading

someone120 commented Dec 29, 2024 •

edited

Loading

someone120 commented Dec 29, 2024 •

edited

Loading