Skip to content
This repository has been archived by the owner on Feb 3, 2023. It is now read-only.

WASM Container Skeleton - Build for Browser Light Clients #894

Merged
merged 20 commits into from May 17, 2019

Conversation

neonphog
Copy link
Contributor

@neonphog neonphog commented Jan 18, 2019

Baby steps to getting us a build target that will work for Holo light client. Currently just imports core_types and exposes a test function that we should remove later.

Mostly just want everyone's opinion on:

  • naming - conductor_wasm ok?
  • build-strategy - wasm-bindgen ok? also, --nodejs ok? our options are: browser-only or nodejs only... but there are tools (like rollup / webpack) to turn nodejs projects into web projects, so I started with that... it makes it easier to do unit tests too, since we don't have to spin up a web-server
  • not committing the generated wasm to git - we could totally also commit it
  • Makefile - are we still using the root Makefile? what about the one in Docker? Something else?

before we start working through the real problems.

Example real problem... I suspect we will run into a lot of wasm incompatibilities as we try to include all of the crates needed:

$ make build_container_wasm
error[E0433]: failed to resolve: use of undeclared type or module `imp`
   --> /home/neonphog/.cargo/registry/src/github.com-1ecc6299db9ec823/same-file-1.0.3/src/lib.rs:105:19
    |
105 | pub struct Handle(imp::Handle);
    |                   ^^^ use of undeclared type or module `imp`

$ cargo tree -p same-file -i
same-file v1.0.3                                                                
└── walkdir v2.2.5
    └── holochain_cas_implementations v0.0.3 (/home/neonphog/projects/holochain-rust/cas_implementations)
        ├── holochain_container_api v0.0.3 (/home/neonphog/projects/holochain-ru

Copy link
Collaborator

@lucksus lucksus left a comment

Choose a reason for hiding this comment

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

Cool!
I'm ok with all those decisions.

Concerning Makefile, Circle CI uses the nix-shell setup in shell.nix. @thedavidmeister, what is needed to add tests to the CI?

And yeah, holochain_cas_implementations won't compile for WASM because of the file implementation that depends on filesystem OS functions which are not there in WASM.. Which, currently, also blocks container_api from being built for WASM. Did you try core? That should actually work now..

Would this crate container_wasm be a surrogate for container_api or do we need that there as well? I guess there is code in container_api that we might want to use in WASM as well. We would need to split that out then, or only make container dependend on cas_implementations, but not container_api...

@neonphog
Copy link
Contributor Author

@lucksus

Did you try core? That should actually work now

Not yet, I'm excited to iterate on this once it's a priority!

Would this crate container_wasm be a surrogate for container_api or do we need that there as well? I guess there is code in container_api that we might want to use in WASM as well

Yeah, I was, at one point, thinking we'd want to just wrap core, but I don't think that's correct... with micro-service apps bridging / capabilities etc being managed at the container level, I'm pretty sure we'll need that in the browser as well.

@thedavidmeister
Copy link
Contributor

@neonphog @lucksus this is great

  • lets try using compiler flags to drop fs implementations at the compiler level
  • what i'd really like to see, that would help "sell" bindgen for me in the short term, is the ability to get console logs out of the wasm (especially on panics) and unit test wasm (e.g. headless firefox) https://rustwasm.github.io/book/game-of-life/testing.html
  • looks like the bindgen installation process is via cargo, so it would work on nix just like hc-install-tarpaulin in shell.nix

Copy link
Contributor

@thedavidmeister thedavidmeister left a comment

Choose a reason for hiding this comment

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

as per comment in main thread

@lucksus
Copy link
Collaborator

lucksus commented Feb 11, 2019

Can we have Compiler flags and bindgen as follow ups, @thedavidmeister?
Adding this build to the nix scripts thought is something we should add here since currently this crate isn’t even build in CI.

Copy link
Collaborator

@lucksus lucksus left a comment

Choose a reason for hiding this comment

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

Requesting to add crate to nix scripts.

@thedavidmeister
Copy link
Contributor

@lucksus yeah followup sounds fine, i think what we need is some space in the SoA to track all of this

container_wasm/src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@thedavidmeister thedavidmeister left a comment

Choose a reason for hiding this comment

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

i've updated the SoA tree to track the followups for this, so happy for now

@thedavidmeister
Copy link
Contributor

  • cleaned up @sphinxc0re 's comments
  • renamed container to conductor
  • resolved merge xflx

but am wondering if actually we want this in a dedicated repo, much like moving the node conductor to a different repo

Copy link
Contributor

@thedavidmeister thedavidmeister left a comment

Choose a reason for hiding this comment

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

lemme nixify

@thedavidmeister
Copy link
Contributor

nixified and added tests to ci

@thedavidmeister thedavidmeister merged commit 60c6aa4 into develop May 17, 2019
@zippy zippy deleted the wasm-build branch October 4, 2019 18:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants