Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wasmer cli requires admin privilege on Windows due to cache #2454

Closed
rajsite opened this issue Jul 7, 2021 · 4 comments · Fixed by #2474
Closed

Wasmer cli requires admin privilege on Windows due to cache #2454

rajsite opened this issue Jul 7, 2021 · 4 comments · Fixed by #2474
Assignees
Labels
bug Something isn't working 📦 lib-cache About wasmer-cache 🖼️ platform-windows This issue happens on Windows
Milestone

Comments

@rajsite
Copy link

rajsite commented Jul 7, 2021

Describe the bug

Trying to run a wasm module with the wasmer cli in a normal user level cli results in:

> wasmer exorbitant.wasm
error: failed to run `exorbitant.wasm`
│   1: module instantiation failed (engine: universal, compiler: cranelift)
╰─> 2: Access is denied. (os error 5)

Windows 10 using Wasmer 2.0 installed from the Windows Installer in the release.

Steps to reproduce

From a user-privilege cli run: wasmer <some wasm file>

Expected behavior

I expect the cli to be able to run and cache build results in user accessible folders. I do not expect to need admin privileges to run wasmer.

Additional context

Workarounds:

  1. In a user-privilege cli disable the cache:

    wasmer run --disable-cache exorbitant.wasm
  2. Run from a cmd with admin privledges:

    1. Press start and type cmd
    2. Right click on the "Command Prompt" option and choose "Run as administrator" (approve the UAC prompt as needed)
    3. Run wasmer in the admin prompt

Edit: The wasm file used: https://wapm.io/package/rajsite/exorbitant

@rajsite rajsite added the bug Something isn't working label Jul 7, 2021
@chenyukang
Copy link
Contributor

I will have a try on this issue since most of you are using MacOs ...

@chenyukang
Copy link
Contributor

Confirmed the bug.
This is introduced by this PR

If we use:

Root: HKCU; Subkey: "Environment"; ValueType:string; ValueName: "WASMER_CACHE_DIR"; \
    ValueData: "{app}\cache"; Flags: preservestringtype

The WASMER_CACHE_DIR will be: "C:\\Program Files (x86)\\Wasmer\\cache\\2.0.0"
And we can not create directory in it: "C:\\Program Files (x86)\\Wasmer\\cache\\2.0.0\\cranelift"

This line will trigger an error.

@syrusakbary , why did we replace {%USERPROFILE} with app?

@syrusakbary
Copy link
Member

@chenyukang I did it that way because a user may have different versions of Wasmer installed in different folders and I wanted to associate the configuration with the installation. This was required for testing choco if I remember properly.

However, I did not know that would trigger an error though.

We can roll back partially that commit, or think on other solutions?

@chenyukang
Copy link
Contributor

chenyukang commented Jul 16, 2021

WASMER_CACHE_DIR is only used for caching Store, so I think it's better not to set a register default value for it on Windows. We can keep the consistent usage style with Unix/Linux OS.

If users want to specify a cache directory, they can set an environment variable WASMER_CACHE_DIR, otherwise, env::temp_dir() will be used and it's safe for caching files.

/// Get the cache dir
pub fn get_cache_dir() -> PathBuf {
    match env::var("WASMER_CACHE_DIR") {
        Ok(dir) => {
            let mut path = PathBuf::from(dir);
            path.push(VERSION);
            path
        }
        Err(_) => {
            // We use a temporal directory for saving cache files
            let mut temp_dir = env::temp_dir();
            temp_dir.push("wasmer");
            temp_dir.push(VERSION);
            temp_dir
        }
    }
}

What do you think?

@Hywan Hywan added 📦 lib-cache About wasmer-cache 🖼️ platform-windows This issue happens on Windows labels Jul 17, 2021
@Hywan Hywan self-assigned this Jul 17, 2021
bors bot added a commit that referenced this issue Jul 20, 2021
2474: Fix windows cache permission r=syrusakbary a=chenyukang

# Description
Fix issue #2454 , default `WASMER_CACHE_DIR` is not right for Windows.
Add more detailed messages for creating cache directory failure.

close #2454.



Co-authored-by: chenyukang <[email protected]>
Co-authored-by: yukang <[email protected]>
@bors bors bot closed this as completed in c1ad592 Jul 21, 2021
@syrusakbary syrusakbary added this to the v2.1 milestone Nov 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 📦 lib-cache About wasmer-cache 🖼️ platform-windows This issue happens on Windows
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants