Skip to content

Conversation

@bvisness
Copy link
Contributor

@bvisness bvisness commented Dec 8, 2025

The Compact Import Section proposal adds two new binary and text encodings to the import section that can express imports more concisely.

@bvisness bvisness changed the title Initial implementation of compact imports proposal Implement the compact imports proposal Dec 8, 2025
@bvisness
Copy link
Contributor Author

bvisness commented Dec 9, 2025

I see there are a gazillion CI failures that I don't get locally when running cargo test. I tried running cargo test --all and cargo test --locked --all (no idea what --locked does) but I just get:

   Compiling artifacts v0.1.0 (/home/bvisness/Developer/wasm-tools/crates/wit-dylib/test-programs/artifacts)
error: failed to run custom build command for `artifacts v0.1.0 (/home/bvisness/Developer/wasm-tools/crates/wit-dylib/test-programs/artifacts)`

Caused by:
  process didn't exit successfully: `/home/bvisness/Developer/wasm-tools/target/debug/build/artifacts-00f7e71926824b33/build-script-build` (exit status: 101)
  --- stderr

  thread 'main' (224713) panicked at crates/wit-dylib/test-programs/artifacts/build.rs:7:70:
  called `Result::unwrap()` on an `Err` value: NotPresent
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Would appreciate guidance on how to actually run tests here.

@alexcrichton
Copy link
Member

For testing cargo test --all should exercise almost all of CI yeah, it's mostly testing other packages in the workspace (cargo test only tests the top-level one). It's safe to ignore --locked as that's just a CI thing.

For the wit-dylib failure, that's a bad error message about the WASI_SDK_PATH env var being set -- I'll work on improving the error message there at least. In the meantime you can pass --exclude wit-dylib to Cargo to ignore that crate for now and it should get all the others building and such

@bvisness
Copy link
Contributor Author

bvisness commented Dec 9, 2025

I am making good progress on the text parser, I think, but also, I have found myself in an absolute lifetime disaster here. I would really appreciate any guidance on how to resolve the errors I'm seeing. I am completely stumped. Surely I don't have to annotate the entire program with 'a just to be able to iterate over imports in the AST?

@bvisness
Copy link
Contributor Author

As I mentioned on Zulip, I can't seem to use wasm-tools parse on the following file because I get an end-of-file error:

(module
  (import "foo" "bar" (func (param i32)))
  (import "foo" "baz" (func (param i32)))
  (import "foo"
    (item "beep" (func (param i64)))
    (item "boop" (func (param i64)))
  )
)
error: unexpected end-of-file (at offset 0x3f)

It seems to me like it's correctly parsing the text, but then blowing up when trying to do some kind of...validation run of the binary parser, except with no features enabled? Or maybe something else is wrong, since the compact encoding should trigger an "unexpected externtype" error.

@bvisness
Copy link
Contributor Author

Ok, I think it's all pretty much working, except that there are WIT linking issues that I don't know how to resolve. Would appreciate any guidance on that. I'll leave this in a draft state for a bit while I add spec tests to the proposal repo and use them to validate both this PR and our SpiderMonkey implementation.

@alexcrichton
Copy link
Member

Ah those are safe failures to ignore. That's from rustc nightly updates breaking things which I've resolved in #2402 by pinning nightly temporarily

@alexcrichton
Copy link
Member

Code-wise everything looks good to me modulo TODO-docs 👍

@bvisness bvisness marked this pull request as ready for review December 12, 2025 15:59
@bvisness bvisness requested a review from a team as a code owner December 12, 2025 15:59
@bvisness bvisness requested review from alexcrichton and removed request for a team December 12, 2025 15:59
@bvisness
Copy link
Contributor Author

No idea what those test failures are...I don't think I changed that file.

If you'd like, I can wait to add spec tests for compact imports before merging this? But I'm not sure what you do for proposal spec tests in this repo.

@alexcrichton
Copy link
Member

The failures here are related to Rust 1.92 being released, which should be fixed now with a rebase to pick up the changes from #2402.

Otherwise it's ok to defer the rigorous tests to the proposal testsuite, but it would be good to have at least a basic test in this repository for some of the textual forms and basic validation -- for example the binary format should be rejected if the feature is not enabled, and then accepted with the feature enabled.

This implements the Compact Import Section proposal for WebAssembly:
https://github.com/WebAssembly/compact-import-section

At the time of writing this proposal is stage 2. It adds two new binary
/ text encodings for the import section that allow for deduplication of
module names and externtypes.
@bvisness bvisness force-pushed the compact-import-section branch from 8164c28 to d861b6d Compare December 17, 2025 16:45
@bvisness
Copy link
Contributor Author

Well, I rebased, I added some tests, they work locally, and now I'm getting newly inscrutable build errors.

@alexcrichton
Copy link
Member

> #3 ERROR: failed to authorize: failed to fetch oauth token: unexpected status from POST request to https://auth.docker.io/token: 500 Internal Server Error: an unknown error occurred during the request

just network errors, I'll retry

@alexcrichton alexcrichton added this pull request to the merge queue Dec 19, 2025
Merged via the queue into bytecodealliance:main with commit 8697e5b Dec 19, 2025
62 of 66 checks passed
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.

2 participants