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

bring cloud throws unsupported operation error #401

Closed
1 of 6 tasks
Chriscbr opened this issue Oct 31, 2022 · 10 comments
Closed
1 of 6 tasks

bring cloud throws unsupported operation error #401

Chriscbr opened this issue Oct 31, 2022 · 10 comments
Assignees
Labels
🐛 bug Something isn't working ⌨️ cli CLI 🛠️ compiler Compiler
Milestone

Comments

@Chriscbr
Copy link
Contributor

Chriscbr commented Oct 31, 2022

I tried this:

// hello.w
bring cloud;
wing compile hello.w

I expected to see this happen:

Some nice terraform generated

Instead, this happened:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { kind: Unsupported, message: "operation not supported on this platform" }', libs/wingc/src/parser/bring.rs:14:68

Component

Select one or more:

  • Language Design
  • Compiler
  • SDK
  • IDE Extension
  • Documentation
  • Development Environment

Environment

Platform: MacOS
Version (wing --version): 0.2.95

@Chriscbr Chriscbr added 🐛 bug Something isn't working ⌨️ cli CLI 🛠️ compiler Compiler labels Oct 31, 2022
@Chriscbr
Copy link
Contributor Author

also, I found running nx wing:test at the repo root now fails:

FAIL  src/commands/compile.test.ts
  ● compile command tests › should be able to compile the SDK capture test
    RuntimeError: unreachable

      at __rust_start_panic (wasm:/wasm/0666d6c2:1:2817416)
      at rust_panic (wasm:/wasm/0666d6c2:1:2218867)
      at std::panicking::rust_panic_with_hook::h5af4a166307aff48 (wasm:/wasm/0666d6c2:1:1331723)
      at std::panicking::begin_panic_handler::{{closure}}::h42a9871ead2de5e8 (wasm:/wasm/0666d6c2:1:2322047)
      at std::sys_common::backtrace::__rust_end_short_backtrace::h42a6bde96d4a4a1f (wasm:/wasm/0666d6c2:1:2815057)
      at rust_begin_unwind (wasm:/wasm/0666d6c2:1:2748846)
      at core::panicking::panic_fmt::h33d7d4c3033d60da (wasm:/wasm/0666d6c2:1:2761199)
      at core::result::unwrap_failed::h5d36bed23403009b (wasm:/wasm/0666d6c2:1:2388896)
      at core::result::Result<T,E>::unwrap::h8da22eb3bc561e8b (wasm:/wasm/0666d6c2:1:2133148)
      at wingc::parser::bring::bring::h8cd575998dea9c67 (wasm:/wasm/0666d6c2:1:713313)
      at wingc::compile::heae3d9811cfa14a6 (wasm:/wasm/0666d6c2:1:788434)
      at wingc::main::h19ad0e72b36becad (wasm:/wasm/0666d6c2:1:1652158)
      at core::ops::function::FnOnce::call_once::he5db9842b7b60ed9 (wasm:/wasm/0666d6c2:1:2761288)
      at std::sys_common::backtrace::__rust_begin_short_backtrace::hff79691fe0c4ead6 (wasm:/wasm/0666d6c2:1:2749027)
      at std::rt::lang_start::{{closure}}::h90529a8819e16d56 (wasm:/wasm/0666d6c2:1:2582095)
      at std::rt::lang_start_internal::hb6c9ed857072d0ac (wasm:/wasm/0666d6c2:1:1991636)
      at std::rt::lang_start::h6949d6a20d4a69ab (wasm:/wasm/0666d6c2:1:2373380)
      at __original_main (wasm:/wasm/0666d6c2:1:2813548)
      at _start (wasm:/wasm/0666d6c2:1:2814688)
      at _start.command_export (wasm:/wasm/0666d6c2:1:2817176)
      at src/commands/compile.ts:81:8
      at step (src/commands/compile.ts:46:23)
      at Object.next (src/commands/compile.ts:27:53)
      at fulfilled (src/commands/compile.ts:18:58)

Looks like this test isn't getting run in CI - I have a hunch this could be related to the other bug...

@3p3r
Copy link
Contributor

3p3r commented Nov 1, 2022

Investigation into this ticket turned out to be a much bigger issue than it seems. The bring implementation which got recently merged in #252 broke WASI builds. That PR added "context aware" bring syntax so nested relative bring statements would work correctly.

Well, that breaks the WASI cap-based filesystem since a WASI module needs to have the directories it needs access to "preopened" before execution starts (security, who needs that, I know!). See the following references:

Here's the problem in code form:

// main.w
bring "../local.w"

Can't do that ^ because .. isn't preopened for WASI. Only . is preopened and we do not know .. needs to be preopened until we parse user's code.

We can do two things:

  • Expose a path.resolve + read_source functions from JS side to Rust and call it in Rust (this'll break native builds entirely)
  • Doing pre-parsing at JS level using tree-sitter's WASM builds and figuring out all directories in a pre-pre-processor stage

Suggestions welcome.

Tagging for visibility @yoav-steinberg / @MarkMcCulloh / @staycoolcall911

@3p3r
Copy link
Contributor

3p3r commented Nov 1, 2022

Another option would be adding a "dry run" command to the compiler in Rust bin.rs so it only prints what it needs from its host to preopen and then the actual compile command will have that available. JS side will have to call the WASM module twice. Once to get the dry run results and once to actually compile. This by itself does not work unless JS keeps calling WASM recursively per local wing file until it exits 1 which signals no more brings. Hacky.

@eladb
Copy link
Contributor

eladb commented Nov 1, 2022

I think delegating file actions to the host (wing cli in our case) actually makes sense. I can see how for example in a browser context we would want a completely different way of locating and loading files.

Instead of leaking out node apis into the compiler I'd define something like ICompilerHost which will expose a bunch of facilities for the compiler, amongst them file reading apis.

Does that make sense?

@3p3r
Copy link
Contributor

3p3r commented Nov 1, 2022

That makes sense. Compilers having a separate host is a common design. For reference:

While at it, we can also enable DWARF in WASI builds to help debugging. As well as providing a base host implementation for native builds (like how V8 offers a PlatformBase).

@3p3r 3p3r self-assigned this Nov 1, 2022
@3p3r 3p3r added this to the MVP-1 milestone Nov 1, 2022
@3p3r
Copy link
Contributor

3p3r commented Nov 1, 2022

Work is currently in progress in my branch: main...sep/compiler-host
I abstracted all platform calls into its own trait and conditionally included the platform impl based on compilation target.
It currently compiles just fine but wasm-bindgen's CLI is failing to generate bindings for JS. I am investigating that atm.

Conditional inclusion: https://github.com/monadahq/winglang/blob/sep/compiler-host/libs/wingc/src/platform.rs
Two available platforms:

I kept native compatibility because removing it breaks so many other things.

@3p3r
Copy link
Contributor

3p3r commented Nov 2, 2022

The rabbit hole just got deeper. So wasm-bindgen does not fully support WASI targets and some times it compiles, some times it does not and fails to generate bindings. Our compiler needs WASI to compile due to its dependency on tree-sitter. wasm-bindgen is needed to import/export stuff from/to the host.

Since these two technologies do not mix (wasm-bindgen and WASI), we basically need to recreate wasm-bindgen's low level access in Rust through low level extern "C" calls, a lot like how Emscripten does it in their SDK.

This is not hard, our platform use is pretty limited, but it's time consuming.

@eladb
Copy link
Contributor

eladb commented Nov 2, 2022

Work is currently in progress in my branch: main...sep/compiler-host

@3p3r would you be able to create a draft pull request with this branch so I can provide some early feedback?

@3p3r
Copy link
Contributor

3p3r commented Nov 2, 2022

bet. #414

3p3r added a commit that referenced this issue Nov 2, 2022
This reverts commit 05fb820.

Context and tracking ticket for this issue:
#401

Reverting the PR in the meantime while I am working on a fix, so
compiler can be invoked again.
@staycoolcall911 staycoolcall911 moved this to 🆕 New - not properly defined in Wing Nov 3, 2022
@staycoolcall911 staycoolcall911 moved this from 🆕 New - not properly defined to 🤝 Backlog - handoff to owners in Wing Nov 3, 2022
@ekeren ekeren added the p3 label Nov 13, 2022
@staycoolcall911 staycoolcall911 moved this from 🤝 Backlog - handoff to owners to 🔖 Ready - owned and understood in Wing Nov 13, 2022
@staycoolcall911
Copy link
Contributor

Fixes #476

@3p3r 3p3r moved this from 🔖 Ready - owned and understood to 🏗 In progress in Wing Nov 15, 2022
@3p3r 3p3r moved this from 🏗 In progress to 🔖 Ready - owned and understood in Wing Nov 15, 2022
@github-project-automation github-project-automation bot moved this from 🔖 Ready - owned and understood to ✅ Done in Wing Jan 3, 2023
skyrpex added a commit that referenced this issue Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working ⌨️ cli CLI 🛠️ compiler Compiler
Projects
Archived in project
Development

No branches or pull requests

5 participants