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

Fix build with wasmtime 27.0.0 #1493

Merged
merged 1 commit into from
Nov 26, 2024
Merged

Fix build with wasmtime 27.0.0 #1493

merged 1 commit into from
Nov 26, 2024

Conversation

osokin
Copy link
Contributor

@osokin osokin commented Nov 22, 2024

No description provided.

@thresheek
Copy link
Member

Does that still work with e.g. wasmtime 24?

@thresheek
Copy link
Member

Does that still work with e.g. wasmtime 24?

Ah, it looks like CI tells us it does not: https://github.com/nginx/unit/actions/runs/11979528350/job/33401907630?pr=1493

@osokin
Copy link
Contributor Author

osokin commented Nov 22, 2024

Does that still work with e.g. wasmtime 24?

Ah, it looks like CI tells us it does not: https://github.com/nginx/unit/actions/runs/11979528350/job/33401907630?pr=1493

Thanks for the notice, I've just updated the patch.

@ac000 ac000 self-requested a review November 23, 2024 02:51
@ac000
Copy link
Member

ac000 commented Nov 23, 2024

I guess we also want write perms to match current behaviour...

@osokin
Copy link
Contributor Author

osokin commented Nov 23, 2024

I guess we also want write perms to match current behaviour...

Updated.

Wasmtime 27.0.0 adjusted the C API to start flowing through the
directory and file permission bits of the underlying rust
wasi_config_preopen_dir() implementation.

The directory permissions control whether a directory is read-only or
whether you can create/modify files within.

You always need at least WASMTIME_WASI_DIR_PERMS_READ.

The file permissions control whether you can read or read/write files.

WASMTIME_WASI_FILE_PERMS_WRITE seems to imply
WASMTIME_WASI_FILE_PERMS_READ (but we add both just to make it clear
what we want)

[ Permissions tweak and commit message - Andrew ]
Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
@ac000
Copy link
Member

ac000 commented Nov 26, 2024

  • Tweaked the permissions to provide full read/write access.
  • Added a commit message.
$ git range-diff d89eed2c...6cc4d706
1:  d89eed2c ! 1:  6cc4d706 Fix build with wasmtime 27.0.0
    @@ Metadata
     Author: Sergey A. Osokin <sergey.osokin@nginx.com>
     
      ## Commit message ##
    -    Fix build with wasmtime 27.0.0
    +    wasm: Fix build with wasmtime 27.0.0
    +
    +    Wasmtime 27.0.0 adjusted the C API to start flowing through the
    +    directory and file permission bits of the underlying rust
    +    wasi_config_preopen_dir() implementation.
    +
    +    The directory permissions control whether a directory is read-only or
    +    whether you can create/modify files within.
    +
    +    You always need at least WASMTIME_WASI_DIR_PERMS_READ.
    +
    +    The file permissions control whether you can read or read/write files.
    +
    +    WASMTIME_WASI_FILE_PERMS_WRITE seems to imply
    +    WASMTIME_WASI_FILE_PERMS_READ (but we add both just to make it clear
    +    what we want)
    +
    +    [ Permissions tweak and commit message - Andrew ]
    +    Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
     
      ## src/wasm/nxt_rt_wasmtime.c ##
     @@ src/wasm/nxt_rt_wasmtime.c: nxt_wasmtime_wasi_init(const nxt_wasm_ctx_t *ctx)
    @@ src/wasm/nxt_rt_wasmtime.c: nxt_wasmtime_wasi_init(const nxt_wasm_ctx_t *ctx)
          for (dir = ctx->dirs; dir != NULL && *dir != NULL; dir++) {
     +#if defined(WASMTIME_VERSION_MAJOR) && (WASMTIME_VERSION_MAJOR >= 27)
     +        wasi_config_preopen_dir(wasi_config, *dir, *dir,
    -+                                 WASMTIME_WASI_DIR_PERMS_WRITE,
    -+                                 WASMTIME_WASI_FILE_PERMS_WRITE);
    ++                WASMTIME_WASI_DIR_PERMS_READ|WASMTIME_WASI_DIR_PERMS_WRITE,
    ++                WASMTIME_WASI_FILE_PERMS_READ|WASMTIME_WASI_FILE_PERMS_WRITE);
     +#else
              wasi_config_preopen_dir(wasi_config, *dir, *dir);
     +#endif
```

@ac000 ac000 merged commit 6cc4d70 into nginx:master Nov 26, 2024
25 checks passed
@ac000
Copy link
Member

ac000 commented Nov 26, 2024

Merged. Thanks Sergey!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants