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

Adapt rust optimizer for cosmwasm-plus #18

Closed
ethanfrey opened this issue Aug 11, 2020 · 5 comments · Fixed by #19
Closed

Adapt rust optimizer for cosmwasm-plus #18

ethanfrey opened this issue Aug 11, 2020 · 5 comments · Fixed by #19

Comments

@ethanfrey
Copy link
Member

We currently support two modes:

  • The simple mode is when the repo has exactly one contract in it, no arguments provided.
  • The multi-repo mode was designed for cosmwasm-examples, where each contract is not in the workspace

However, it fails when we try to build cosmwasm-plus:

  • Multi-repo with all contracts in the same workspace (needed as we import from other packages)

There are two key differences here that cause the build error:

  1. (Stack trace below) target where the code goes is not in the directory with the contract's Cargo.toml, but in the repo root
  2. (May cause an issue) multiple contracts will all compile to the same target dir. This means *.wasm will not be unique, but we need the name of the contract we compile.

Output of a failed build (see bottom):

$ docker run --rm -v "$(pwd)":/code \
>   --mount type=volume,source="cosmwasm_plus_cache",target=/code/target \
>   --mount type=volume,source=registry_cache,target=/usr/local/cargo/registry \
>   cosmwasm/rust-optimizer:0.9.0 ./contracts/cw1-subkeys
Building contract in /code/contracts/cw1-subkeys
    Updating crates.io index
 Downloading crates ...
   ....
   Compiling cw20 v0.1.0 (/code/packages/cw20)
   Compiling cw1-whitelist v0.1.0 (/code/contracts/cw1-whitelist)
   Compiling cw1-subkeys v0.1.0 (/code/contracts/cw1-subkeys)
    Finished release [optimized] target(s) in 1m 08s
Failed opening 'target/wasm32-unknown-unknown/release/*.wasm'

That failed as it looked for /code/contracts/cw1-subkeys/target not /code/target

@maurolacy
Copy link
Contributor

maurolacy commented Aug 11, 2020

Yes, I've ran into that issue. If I recall correctly, a possible way to solve it is to add a cd - near the end of the cosmwasm-optimizer shell script.

That way you return to the previous directory (which has no effect when there's no previous dir) and the cp command will succeed.

Update: this only has to be done when the contract is in a workspace. So, a way to detect that (or better, the location of the target directory) would be needed.

@webmaster128
Copy link
Member

webmaster128 commented Aug 11, 2020

The multi-repo mode was designed for cosmwasm-examples, where each contract is not in the workspace

This mode is for the development contracts in the cosmwasm repo. Here we have the case that we parts of the source code live in ./../.. but otherwise this is like the first case. in cosmwasm-examples it is sufficient to treat the directories as completely independent (what they are).

That failed as it looked for /code/contracts/cw1-subkeys/target not /code/target

I see, this line is the problem: https://github.com/CosmWasm/rust-optimizer/blob/v0.9.0/optimize.sh#L18.

a way to detect that (or better, the location of the target directory)

Yeah, we can either pass the target dir as a parameter. The other way would be to use find to search the entire /code for *.wasm files. That would be more general but could lead to unintended results somehow.

@maurolacy
Copy link
Contributor

maurolacy commented Aug 11, 2020

It occurs to me that a way that is simple (though a little bit limited) would be to grep for [workspace] inside the base Cargo.toml file (i.e. before doing cd "$contractdir") and do a cd - before the wasm-opt if found.

Better and more robust than looking for *.wasm IMO.

@ethanfrey
Copy link
Member Author

I added another script that does exactly what I want for the cosmwasm-plus case (and may be useful for other monorepos).
Another key difference, is you want to provide names on the output, not just contract.wasm. And that there will be many different *.wasm files in the same target dir at the end (which breaks the wasm-opt script in optimize.sh)

Can you take a look at #19?

@ethanfrey
Copy link
Member Author

I started on some unified script, but figured better not to break existing code and 3 cases seemed a bit much to handle in a bash script.

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 a pull request may close this issue.

3 participants