-
Notifications
You must be signed in to change notification settings - Fork 61
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
Fix how cosmwasm-plus compiles #21
Comments
Hi Ethan, Is this because compilation artifacts are being re-used between contracts (even when feature flags change)? This looks like a What about running Regards, |
We run In an earlier version, I looped over However, we want this to work without a command-line argument, so we want to do the looping version, but need to auto-detect if we need to loop over packages. Do you want to take this on? |
Basically 5d60546#diff-5c2979e23e9d5a4e3646e200224774beR15-R23 was fine. But we cannot use |
Sure (if it's not too urgent). Looks like an easy fix; I'll try to do it during the week. |
I can reproduce this behaviour as follows: No features here ⬇️
Building workspace adds --cfg 'feature="library"' to the cw1_whitelist build ⬇️
This is pretty much what is discussed in rust-lang/cargo#5015 and rust-lang/cargo#5364. In order to avoid relying on problematic or unstable behaviour, I agree we should build the contracts individually.
A quite flexible variation of this would be to pass a single foder as a compile all contracts in here argument and let cosmwasm-plus
cosmwasm-examples (what we do now)
cosmwasm-examples (what we could do instead)
cosmwasm (what we do now)
cosmwasm (what we could do instead)
|
That sounds nice. I thought we were trying to avoid arguments for the verification (and the spec of how to run this docker image with no args). But we are updating metadata soon, so maybe we can just add this as a new field? |
Damn, I forgot about that. Well, it works as long as you compile contrats individually. What would even be the source folder of a contract from this repo? Every contracts gets the full repo as its source? |
Each contract has a crate We need the same compile process as with cosmwasm (mount root, compile in each one). All deps are in Cargo.toml |
Urg, this is a problem. The build process needs to be the same as the verification proces otherwise all this gets super unrelyable (Cargo.lock changes at least). We should not build contracts with
cosmwasm is a not helping for the problems we have here because we don't publish source codes there and have no verification process. |
As I was pretty unhappy with my result in #22, which is overly complicated and insufficient at the same time, I made a writeup on the challenges that come in specifically with workspaces: https://hackmd.io/@webmaster128/Sy6VCqiMw The way forward a see now includes:
|
Okay, let's go with this... single and multiple versions. For the simple version, we can use the compile logic from 0.9.0 and the output logic from 0.10.1. I would add a second one with another Dockerfile here, rust-optimizer-plus, which goes and compiles all files in the contract dir one by one, tailored just to the cosmwasm plus use case. Other monorepos can follow this pattern or make their own more complex one with python scripts and all |
Can you make this second image, or should I? |
I'm on it right now and can present a solid base later today |
Closed by #23 |
We made a great simplification by just running the compile step one time on the top level folder even with multple workspaces: https://github.com/CosmWasm/rust-optimizer/blob/v0.10.1/optimize.sh#L25
However, when we run this on
cosmwasm-plus
we get cw1_whitelist with a size of 13kb. That is due to the 'library' feature being enabled when imported by subkeys, so no exports and almost everything gets optimized out.Features are the union of all crates referencing it. In order to allow these crates to be imported as libraries and also compiled alone, we need to run the
cargo build
script once per contract. We also decided we cannot pass argument on the command-line, so I propose something like the following:[package]name
entry then compile there[package]members
entry, then go into each directory listed there and run the build script in each oneThis should work the same for cosmwasm and cosmwasm-examples and cosmwasm-template. The only change would be cosmwasm-plus, where we get the previous behavior (listing
.contracts/*
) without passing that explicitly. We may need a tool to parse the toml file beyondgrep
. https://github.com/freshautomations/stoml is interesting but not sure if we canapt install
it.The text was updated successfully, but these errors were encountered: