hyperledger / iroha

Iroha - A simple, enterprise-grade decentralized ledger
https://wiki.hyperledger.org/display/iroha
Apache License 2.0
433 stars 277 forks source link

`iroha_wasm_builder`: remove `cargo` invocation #2152

Open Arjentix opened 2 years ago

Arjentix commented 2 years ago

Right now there is a problem -- we have to invoke cargo from our iroha_wasm_builder to be able to build Rust Smartcontract in WASM. That can cause problems with cargo recursion problem (cargo doesn't like to be invoked multiple times from it self).

There is a wasm-builder crate which can help us with that. But I'm not 100% sure it can do all what we need. We need to do a research and try to use it

Erigara commented 1 year ago

Maybe we can use cargo as library.

Arjentix commented 1 year ago

@Erigara , cool idea

Arjentix commented 1 year ago

Blocked by https://github.com/hyperledger/ursa/issues/232

DCNick3 commented 10 months ago

That can cause problems with cargo recursion problem (cargo doesn't like to be invoked multiple times from it self).

@Arjentix, What are some of the problems that can be encountered? I only seem to find https://github.com/rust-lang/cargo/issues/6412, and this one:

  1. is already worked around by setting CARGO_TARGET_DIR to OUT_DIR in wasm_builder
  2. wouldn't be solved by using cargo as a library, as the build dir lock is still there
Arjentix commented 10 months ago

@DCNick3 , the problem was found when we were using our cargo lints. Because of that in iroha_wasm_builder we unset CARGO_RECURSION env var. Probably now there's no problem, need to test it.

DCNick3 commented 10 months ago

I think it's actually called RUST_RECURSION_COUNT..

DCNick3 commented 10 months ago

I don't think that using cargo as a library is a good idea.

Mainly because its API is perma-unstable and is not gonna be stabilized soon.

Other problem I can see is ignoring the rustup config, but ig with us using a pinned nightly version for building smart contracts that's wouldn't be a problem.

And, well, it doesn't seem necessary IMO: the RUST_RECURSION_COUNT workaround is not necessary w/o cargo lints (I wasn't getting any problems when disabling it).

There's also CARGO_ENCODED_RUSTFLAGS workaround: (https://github.com/hyperledger/iroha/blob/f9f5ede6791a99ec788a05755867e1fcb497a9e2/wasm_builder/src/lib.rs#L402C1-L406C47). This one still seems to still be required, but I think it's better then chasing a perma-unstable API.