`Extensions` support issue on Winodws

JunkuiZhang commented 3 months ago

Check for existing issues

[X] Completed

Describe the feature

Support for Extensions on Windows requires considerable effort to achieve.

At the moment, there are the following issues:

current_dir() reports an incorrect path (see #14897).
Extensions cannot run using powershell (see #13891). The approach in PR #13891 is currently not feasible in WASM extension system.

Taking the purescript-language-server as an example, the server_path for this extension on macOS is node_modules/.bin/purescript-language-server, but on Windows, it should be node_modules/.bin/purescript-language-server.ps1 and should be executed with powershell instead of node. Additionally, the PATH environment variable must include node.exe.

Therefore, to run this extension successfully, the final code (on Windows) is as follows:

// extensions\purescript\src\purescript.rs
fn language_server_command(
        &mut self,
        config: &zed::LanguageServerId,
        _worktree: &zed::Worktree,
    ) -> Result<zed::Command> {
        let server_path = self.server_script_path(config)?;
        let command = "powershell.exe".into();
        let env = vec![("PATH".to_string(), zed::node_environment_path()?)];
        Ok(zed::Command {
            command,
            args: vec![
                zed::current_dir()
                    .unwrap()
                    .join(&server_path)
                    .to_string_lossy()
                    .to_string(),
                "--stdio".to_string(),
            ],
            env,
        })
    }

For the second issue, I modified zed_extension_api to provide a new function node_environment_path(), which returns the PATH environment variable required to run the extension.

Additionally, extension_lsp_adapter also needs modification. Currently, we always resolve the path for the Command returned by the extension's language_server_command() function:

// crates\extension\src\extension_lsp_adapter.rs
let path = self
                .host
                .path_from_extension(&self.extension.manifest.id, command.command.as_ref());

This causes powershell.exe to become C:/some/dir/powershell.exe, leading to the extension failing to run. Therefore, special handling is required. I used the following code:

// crates\extension\src\extension_lsp_adapter.rs
let path = if command.command.as_str() == "powershell.exe" {
                command.command.into()
            } else {
                self.host
                    .path_from_extension(&self.extension.manifest.id, command.command.as_ref())
            };

A better approach would be for Command to return a type other than String, such as the following, which would allow more elegant handling:

enum CommandType {
    Shell(String),
    Other(String),
}

let path = match command.command {
    CommandType::Shell(shell) => shell.into(),
    CommandType::Other(path) => self.host
                    .path_from_extension(&self.extension.manifest.id, command.command.as_ref()),
};

However, I encountered an issue when trying to modify crates\extension_api\wit\since_v0.0.7\extension.wit and related files to attempt this change. The compilation failed with errors indicating that since_v0.0.6 and other versions could not compile. So, I just use a simple String comparation here.

Edit: After I apply the changes to crates\extension\src\wasm_host\wit\since_v0_0_4.rs, it seems that everything is ok. I know little about wit, so this might be wrong.

For the first issus, I created a new function current_dir() to retrieve the correct path.

With this change, purescript successfully runs on Windows, see:

Screenshot 2024-07-23 010755

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

No response

JunkuiZhang commented 3 months ago

Any suggestions are welcome. If my explanation isn't clear enough (sorry for my limited English), I can create a draft PR to better illustrate my point.

@maxdeviant Could you please take a look at this issue? Your expertise would be really helpful.

SomeoneToIgnore commented 2 days ago

I've experienced two more fundamental issues with the extensions:

Inability to uninstall them using the extensions view (seems to be tracked in https://github.com/zed-industries/zed/issues/18153)

But this can be overridden by the manual removal of the extension directory, so not that bad.

Worse, inability to install any dev extension: when I try to do that, it seems to compile successfully, but fails to install:

2024-11-06T01:25:52.06935+02:00 [INFO] compiled grammar html for extension C:\work\zed\extensions\html
2024-11-06T01:25:52.0694026+02:00 [INFO] finished compiling extension C:\work\zed\extensions\html
2024-11-06T01:25:52.069989+02:00 [ERROR] A required privilege is not held by the client. (os error 1314)
2024-11-06T01:25:52.292568+02:00 [INFO] rebuilt extension index in 10.5836ms

@JunkuiZhang do you have any idea what does this error mean and how are we supposed to address it?

JunkuiZhang commented 2 days ago

Inability to uninstall them using the extensions view (seems to be tracked in Cannot uninstall extension on windows #18153)

But this can be overridden by the manual removal of the extension directory, so not that bad.

I've created a PR to address this #18467. But it requires a new feature: stop_running_server, and I know nothing about LSP, so I stopped there. But with the PR you can uninstall extensions already.

Worse, inability to install any dev extension: when I try to do that, it seems to compile successfully, but fails to install:
2024-11-06T01:25:52.06935+02:00 [INFO] compiled grammar html for extension C:\work\zed\extensions\html
2024-11-06T01:25:52.0694026+02:00 [INFO] finished compiling extension C:\work\zed\extensions\html
2024-11-06T01:25:52.069989+02:00 [ERROR] A required privilege is not held by the client. (os error 1314)
2024-11-06T01:25:52.292568+02:00 [INFO] rebuilt extension index in 10.5836ms
@JunkuiZhang do you have any idea what does this error mean and how are we supposed to address it?

@SomeoneToIgnore I guess you need to run zed.exe with Administrator privilege. During the installation of the dev extension, certain files will be symlinked, and creating symlinks on Windows requires administrator privileges.

SomeoneToIgnore commented 2 days ago

Thank you for the context, the more I learn about this system's internals the more I'm confused.

Speaking more of the symlinks, I've found two things:

there's a way to enable a "Developer mode" that should permit this? (https://stackoverflow.com/questions/58038683/allow-mklink-for-a-non-admin-user)
seems that hard links can be created without any issues?

In your opinion, would either of this options make sense and which is more "normal" on that OS?

JunkuiZhang commented 2 days ago

Thank you for the context, the more I learn about this system's internals the more I'm confused.

Welcome to Windows world :)

Speaking more of the symlinks, I've found two things:

there's a way to enable a "Developer mode" that should permit this? (https://stackoverflow.com/questions/58038683/allow-mklink-for-a-non-admin-user)

Yes, on Windows, if you enable "Developer Mode," you can create symlinks without requiring Administrator privileges. However, I think it makes no sense to ask users to enable Developer Mode when using Zed, so it seems more reasonable to let them run Zed as Administrator when they need to develop plugins.

seems that hard links can be created without any issues?

As far as I know, there are three types of links in Windows: symlink, hard link, and junction. Hard link and junction have some limitations. For example, hard links can only link to files on the same logical volume, while junctions can only link to directories.

And I guess the Hard link and junction might be some legacy options in Windows, see the answer here:

This is a pretty advanced topic, and is mostly used in Windows world for compatibility.

You can also see the release timeline for these link types. If I’m not mistaken, hard links and junctions were available since Windows XP, while symlinks were introduced much later, with Windows Vista.

In your opinion, would either of this options make sense and which is more "normal" on that OS?

Given this, I would recommend using symlinks.

zed-industries / zed