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

Processing custom cargo environment #89

Closed
SteMak opened this issue Aug 24, 2022 · 20 comments · Fixed by #124
Closed

Processing custom cargo environment #89

SteMak opened this issue Aug 24, 2022 · 20 comments · Fixed by #124

Comments

@SteMak
Copy link

SteMak commented Aug 24, 2022

Problem

Provide some special cargo env variables into the docker container crashes the script.

Background

I use custom CARGO_TARGET_DIR and CARGO_BUILD_TARGET_DIR variables that are responsible for target folder name that is target by default.
It is important to provide the variables to docker container as the project folder in mounted as volume and cargo shouldn't create additional target directory.

Solution

Use corresponding env variables when constructing paths to files.
Here optimize.sh#L42 change is needed: replace target with "$CARGO_TARGET_DIR".

@SteMak
Copy link
Author

SteMak commented Aug 24, 2022

Another possible solution and I think it is more conscious is setting CARGO_TARGET_DIR and CARGO_BUILD_TARGET_DIR variables in the container to outside the /code directory and pushing only final artifacts to the mounted folder.

@webmaster128
Copy link
Member

I think the container should not write intermediate build results to the host. Let’s try to work in that direction.

Sent with GitHawk

@SteMak
Copy link
Author

SteMak commented Aug 25, 2022

I think it could copy src folder and Cargo.(toml | lock) files to internal build directory and then push artifacts back.

For now, I'm doing the same but outside the container, it may be a temporal solution for other rangers:

# removing dirs
rm -rf $(DIR)/.optimize_cache
rm -rf $(DIR)/artifacts

# create cache dir and copy sources
mkdir -p $(DIR)/.optimize_cache
cp $(DIR)/Cargo.toml $(DIR)/.optimize_cache
cp $(DIR)/Cargo.lock $(DIR)/.optimize_cache
cp -r $(DIR)/src $(DIR)/.optimize_cache/src

# run docker there
docker run --rm \
	-e CARGO_TERM_COLOR=always \
	--volume $(DIR)/.optimize_cache:/code \
	--volume $(CONTAINER)_cache:/code/target \
	--volume registry_cache:/usr/local/cargo/registry \
	cosmwasm/rust-optimizer:0.12.6

# move built artifacts
mv $(DIR)/.optimize_cache/artifacts $(DIR)

# removing cache dir
rm -rf $(DIR)/.optimize_cache

@webmaster128
Copy link
Member

Okay, so on the happy path this is prevented by using --mount type=volume,source="$(basename "$(pwd)")_cache",target=/code/target. This way the build results are written to a caching volume instead of the host.

How are you setting CARGO_TARGET_DIR/CARGO_BUILD_TARGET_DIR manually? Is this somewhere in your source code?

It is important to provide the variables to docker container as the project folder in mounted as volume and cargo shouldn't create additional target directory.

What's the use case for this?

@ewoolsey
Copy link

ewoolsey commented Jun 8, 2023

I'd like to piggy back off of this for my use case. I'm the maintainer of wasm-deploy and I would like to implement a way for users to have a deterministic build via rust-optimizer. wasm-deploy allows for any arbitrary workspace layout. Currently, rust-optimizer assumes the location of the cargo target dir, and when building a single contract inside a workspace, it incorrectly assumes where the target dir is. It would be great if we could pass an argument into the docker command, something like --target-dir="my/target/dir"

Here is an example of the error I'm getting

sha256sum: can't open './contracts/market/target/wasm32-unknown-unknown/release/*.wasm': No such file or directory

The actual target dir is located at `./target/

@webmaster128
Copy link
Member

@ewoolsey rust-optimizer does not support workspaces at all and does not try to do that. workspace-optimizer compiles all contracts in ./contracts/* and then looks for the Wasm in ./target

@ewoolsey
Copy link

ewoolsey commented Jun 8, 2023

@webmaster128 right, but workspace optimizer is not configurable enough for my use case. wasm-deploy supports any arbitrary workspace structure including alternate targets dirs. My thinking was that using rust-optimizer individually for each contract would work for me if I could specify a target dir. Do you see a better way to accomplish this?

@webmaster128
Copy link
Member

I guess the use case is our of scope for now. What we want to achieve here is minimal instructions as input in order to make reproducible builds easy. This in some places requires conventions, such as all contracts being a sub-folder of contracts. What it gives you is that source code + builder (docker image+tag) are sufficient for reproducible builds. No configs, no flags, no env variables. Just the two inputs and you can verify the build process. This makes manual verifications in gov proposals easy and tools like https://aurascan.io/contracts streamline this process.

That being said, I am happy to discuss what can be improved that does not require adding additional parameters to the verification step.

@ewoolsey
Copy link

ewoolsey commented Jun 8, 2023

@webmaster128 Part of the reason I piggy backed off of this issue is because the solution proposed by @SteMak

Here optimize.sh#L42 change is needed: replace target with "$CARGO_TARGET_DIR".

Seems like it could work for me as well. Although I am also unsure how that var is correctly set.

@webmaster128
Copy link
Member

Seems like it could work for me as well. Although I am also unsure how that var is correctly set.

But where would $CARGO_TARGET_DIR be set? My understanding is that the request is to accept the caller's $CARGO_TARGET_DIR as an input to workspace-optimizer. And this is what I want to avoid.

Regarding the shell in question (for WASM in "$CONTRACTDIR"/target/wasm32-unknown-unknown/release/*.wasm; do): yes, this is a poor solution making too many assumptions on how the target folder is organized internally. But it's the best way to find the .wasm file that I am aware of. What I am waiting for is the stabilization of --out-dir.

@webmaster128
Copy link
Member

I mean we could have a custom --target-dir for each contract in this loop. But then you'd have to re-compile all dependencies for each contract which is probably not what most devs want.

@ewoolsey
Copy link

ewoolsey commented Jun 8, 2023

--out-dir looks like it would be the perfect solution. Too bad it hasn't been stabilized. I've just done some googling to see if there is a reliable way of finding the target-dir and I couldn't seem to find anything unfortunately. Specifying --target-dir is a potential solutions. Although it does take that configurability away from the devs which is unfortunate.

@webmaster128
Copy link
Member

Although it does take that configurability away from the devs which is unfortunate.

That's not a problem here since we do not support custom settings anyways. This tools should exist early when it detects a CARGO_* env variable set by the caller.

@ewoolsey
Copy link

ewoolsey commented Jun 9, 2023

Why are custom envs a problem? If they are set in the repo, say via a toml, then it should be the same for when someone else goes to verify the signature? I guess I don't understand why that can't be accommodated.

@webmaster128
Copy link
Member

Why are custom envs a problem? If they are set in the repo, say via a toml, then it should be the same for when someone else goes to verify the signature? I guess I don't understand why that can't be accommodated.

I just realized that there is also build.target-dir as a Cargo.toml. This is indeed a good question. I was referring to real env vatriables that you need to pass via -e from host into container.

@webmaster128
Copy link
Member

But then again, https://doc.rust-lang.org/cargo/reference/config.html#buildtarget-dir is not set in the project, right?

Why would the project itself have a say where the target folder should be? This should be defined by the build environment (here the docker guest)

@webmaster128
Copy link
Member

If they are set in the repo, say via a toml

What exactly are you referring to here? How can a toml config file change environment variables?

@ewoolsey
Copy link

ewoolsey commented Jun 9, 2023

Ah my mistake I thought the target-dir could be set via a toml. Surely someone has written a tool to extract these variables though.

@webmaster128
Copy link
Member

webmaster128 commented Jun 12, 2023

Coming back to this solution:

I think the container should not write intermediate build results to the host. Let’s try to work in that direction.

With the change in #124 we make sure the target folder is set by the building system (as it should be). Instead of having the mount cache or override host behaviour which was never intended, it creates a mount cache or recompile behaviour. This should be a safe default and does not change conceptually how the builders work.

Providing environment variables to influence the build is discouraged since it will hurt reproducible builds. Maybe we disallow them explicitely at some point.

If you don't care about reproducible builds and just want optimized Wasm, it's easier and faster to use your local toolchain with approaches like this script or community tools like cw-optimizoor.

@webmaster128
Copy link
Member

0.13.0 was released. Now the builder never writes into the host environment, event if you don't set the cache volume. This makes the cache optional. If you don't use it, nothing bad happens.

See also the diff in the CHANGELOG for how to call it: https://github.com/CosmWasm/rust-optimizer/blob/main/CHANGELOG.md#0130---2023-06-20.

For follow-up discussions, please open a new specific issue. Thanks again for bringing this problem up!

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