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

[interpreter]: Add depends to dune-project #1717

Merged
merged 2 commits into from
Jan 4, 2024
Merged

Conversation

muqiuhan
Copy link
Contributor

@muqiuhan muqiuhan commented Jan 4, 2024

Add depends to dune-project so that dependencies can be installed by calling opam install . --deps-only --with-doc --with-test and then build the interpreter from source :)

Copy link
Member

@rossberg rossberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good to me. Can you minimise the PR such that it avoids reformatting unrelated lines?

@muqiuhan
Copy link
Contributor Author

muqiuhan commented Jan 4, 2024

Thank you for your reply, I have removed the formatting related lines.

Copy link
Member

@rossberg rossberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@rossberg rossberg merged commit 44ed035 into WebAssembly:main Jan 4, 2024
1 check passed
@zapashcanon
Copy link
Contributor

I believe this is not correct and should be reverted.

The interpreter (wasm executable) does not depends on js_on_ocaml or js_of_ocaml-ppx, only the wast executable does. This executable is not part of the opam package and shouldn't impose its dependencies.

People won't be happy about having to install some ppx trash for this package...

One solution would be to mark them as development dependencies with e.g. (js_of_ocaml :dev), but I'd rather simply remove them as they're not really development dependencies...

rossberg added a commit that referenced this pull request Jan 4, 2024
@rossberg
Copy link
Member

rossberg commented Jan 4, 2024

Fair enough, I reverted the change.

dhil added a commit to wasmfx/specfx that referenced this pull request Jan 25, 2024
* [test] Adjust br_table length test to avoid wrong failure (WebAssembly#1715)

* [spec] Fix extmul & extadd execution (WebAssembly#1716)

* [interpreter] Add dune dependencies (WebAssembly#1717)

* Revert "[interpreter] Add dune dependencies (WebAssembly#1717)"

This reverts commit 44ed035.

* [interpreter/test] Fix inconsistent use of float values in `spectest` (WebAssembly#1718)

* [spec] Fix operand order in reduction of shape.replace_lane (WebAssembly#1721)

* [spec] Fix sx to be sx? in reduction of vcvtop_zero (WebAssembly#1720)

---------

Co-authored-by: Rod Vagg <[email protected]>
Co-authored-by: 702fbtngus <[email protected]>
Co-authored-by: Muqiu Han (韩暮秋) <[email protected]>
Co-authored-by: Andreas Rossberg <[email protected]>
Co-authored-by: Dongjun Youn / 윤동준 <[email protected]>
Co-authored-by: Hoseong Lee <[email protected]>
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