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.

Sign up for GitHub

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

Jump to bottom

feat: Add --release flag to wasm_builder #5209

Open
wants to merge 68 commits into
base: main
Choose a base branch
from
Open

feat: Add --release flag to wasm_builder #5209

wants to merge 68 commits into from
+1,064 −554

Conversation

aoyako
Copy link
Contributor

@aoyako aoyako commented Oct 30, 2024

Context

Resolves #5202

Problem

The current version of iroha_wasm_builder has a --release flag set by default for its output.
The input --optimize flag can be set that will apply -O -Os.
Still, it cannot produce a build in debug mode.

Solution

Changes in iroha_wasm_builder

This PR introduces a single flag --release that:

  • When set, apply --release -O -Os to the build.
  • When not set, produce a debug build.

Changes in build_wasm.sh

Now, build_wasm.sh produces a debug build by default. Flag --release can be added to build in optimized mode.

Checklist

  • I've read CONTRIBUTING.md.
  • (optional) I've written unit tests for the code changes.
  • All review comments have been resolved.
  • All CI checks pass.

@aoyako aoyako changed the title Feat: Add --release flag to wasm_builder feat: Add --release flag to wasm_builder Oct 30, 2024
@aoyako aoyako marked this pull request as draft October 30, 2024 15:46
aoyako added 2 commits October 31, 2024 01:19
@aoyako
fix: use correct binary directory for debug
986a835 
Signed-off-by: Lohachov Mykhailo <[email protected]>
@aoyako
chore: cargo fmt
7a5b957 
Signed-off-by: Lohachov Mykhailo <[email protected]>
@aoyako aoyako marked this pull request as ready for review October 30, 2024 16:28
s8sato
s8sato reviewed Oct 30, 2024
View reviewed changes
scripts/build_wasm.sh Show resolved Hide resolved
crates/iroha_wasm_builder/src/lib.rs Outdated Show resolved Hide resolved
@s8sato s8sato assigned s8sato and aoyako and unassigned s8sato Oct 30, 2024
s8sato
s8sato reviewed Oct 31, 2024
View reviewed changes
scripts/build_wasm.sh Outdated Show resolved Hide resolved
@aoyako aoyako marked this pull request as draft October 31, 2024 08:42
@aoyako aoyako marked this pull request as ready for review November 7, 2024 10:53
s8sato
s8sato reviewed Nov 25, 2024
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
@aoyako
chore: remove unused profile
92e2228 
Signed-off-by: Lohachov Mykhailo <[email protected]>
s8sato
s8sato previously approved these changes Nov 26, 2024
View reviewed changes
@aoyako
fix: optional output
7980eae 
Signed-off-by: Lohachov Mykhailo <[email protected]>
0x009922
0x009922 reviewed Dec 6, 2024
View reviewed changes
.config/nextest.toml Outdated Show resolved Hide resolved
crates/iroha_core/src/sumeragi/main_loop.rs Outdated Show resolved Hide resolved
crates/iroha_test_network/src/lib.rs Outdated Show resolved Hide resolved
crates/iroha_test_network/src/lib.rs Outdated Show resolved Hide resolved
crates/iroha_wasm_builder/src/lib.rs Outdated Show resolved Hide resolved
crates/iroha_wasm_builder/src/main.rs Outdated Show resolved Hide resolved
crates/iroha_wasm_builder/src/main.rs Outdated Show resolved Hide resolved
0x009922
0x009922 reviewed Dec 12, 2024
View reviewed changes
Copy link
Contributor

@0x009922 0x009922 left a comment

Choose a reason for hiding this comment

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

Can approve after last minor chores.

crates/iroha_test_network/src/lib.rs Outdated Show resolved Hide resolved
crates/iroha_test_samples/src/lib.rs Outdated Show resolved Hide resolved
crates/iroha_core/src/sumeragi/main_loop.rs Outdated Show resolved Hide resolved
0x009922
0x009922 previously approved these changes Dec 13, 2024
View reviewed changes
s8sato
s8sato previously approved these changes Dec 13, 2024
View reviewed changes
@aoyako
fix: parse lowercase
111824b 
Signed-off-by: Lohachov Mykhailo <[email protected]>
@aoyako aoyako dismissed stale reviews from s8sato and 0x009922 via 111824b December 13, 2024 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BAStos525 BAStos525 BAStos525 left review comments

@mversic mversic mversic left review comments

@s8sato s8sato s8sato approved these changes

@0x009922 0x009922 0x009922 left review comments

@SamHSmith SamHSmith Awaiting requested review from SamHSmith SamHSmith is a code owner

@dima74 dima74 Awaiting requested review from dima74 dima74 is a code owner

@outoftardis outoftardis Awaiting requested review from outoftardis outoftardis is a code owner

@DCNick3 DCNick3 Awaiting requested review from DCNick3 DCNick3 is a code owner

At least 2 approving reviews are required to merge this pull request.

Assignees

@0x009922 0x009922

@mversic mversic

@s8sato s8sato

@aoyako aoyako

Labels
config-changes Changes in configuration and start up of the Iroha
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add --release flag to iroha_wasm_builder
5 participants
@aoyako @0x009922 @mversic @s8sato @BAStos525