feat(kagami wasm): move --profile into --cargo-args

[Levi0804]

Overview

This PR handles #5453, moving the --profile argument to --cargo-args and handling the case where the --profile argument is given multiple times.

Changes

1. Updated Args::Build handler branch to prioritize --profile argument inside --cargo-args, ignoring the other --profile argument (default or if any).
2. Updated build_wasm.sh to use --carg-args.

[0x009922]

Hey, thank you for opening the PR!

This seems to be the right direction. Notes:

• If user provides --profile, let's print a warning with eprintln! like warn: "--profile" arg is deprecated; please use "--cargo-args='--profile ..'" instead
• If there is a conflict between --profile and --cargo-args (which you handle), print another warning in addition, like warn: value from "--profile" is ignored in favor of profile in "--cargo-args"
• Please add unit tests for args parsing (there are some existing ones), ensuring the behaviour

[Levi0804]

Hey, I’ve added the tests and logs you suggested, and also refactored some code to avoid unnecessary allocations. Does the change fromProfile to Option<Profile> inside Args::Build look good to you?

Please let me know if you'd like me to change anything.

[Levi0804]

Hey, this works, but I just realized I could have done it a bit better using #[arg(value_parser = ...)] and the tests could be written more cleanly too. I’ll add those changes soon. What do you think?

[aoyako]

Should we keep the profile option @0x009922 @s8sato ? This is an internal tool, there is no need for backward compatibility.

[s8sato]

I agree. Not accepting the deprecated argument would simplify the implementation and improve maintainability.

[0x009922]

This is not an entirely internal tool, and is intended for users to help them build their smartcontracts.

Practically, yes, I would not insist on keeping the backward compatibility in our circumstances.

Ideologically, I would say that it is a good manner to keep it, especially if it's not too costly. We have a habit of breaking everything all the time while finding some excuses for it, and I don't think it is a good habit.

[s8sato]

I acknowledge that my response was somewhat hasty. Indeed, kagami wasm is intended for use not only by internal developers but also by contributors, node operators, and trigger/smart-contract developers.

I believe the root cause of this dispute is that we released the RCs before the specifications were truly finalized, which has forced us into a dilemma: whether to support backward compatibility at the expense of development velocity, or to ignore backward compatibility and risk breaking the interface.

Both options involve trade‑offs, but I would prioritize a proper RC release and opt for the latter.

[0x009922]

I am fine with both, so let's not care about backwards compatibility and just remove the --profile arg.

[s8sato]

Please let us know what you think @Levi0804

[Levi0804]

@s8sato Sorry for the delayed response. I’m good with either, but since there’s already a "cargo args" argument, moving profile there might be better.

[0x009922]

Then, could you please update the PR and remove the dedicated --profile argument? (and passing it into --cargo-args where used)

[Levi0804]

@0x009922 I have removed the dedicated --profile argument. Do these changes look good?

[0x009922]

Yes, thanks!

[Levi0804]

Hello, I've updated the code to use the parse function and added an else if branch for appropriate logging in different conditions.
Let me know if these changes look good to you.

[Levi0804]

Hey, this works, but I just realized I could have done it a bit better using #[arg(value_parser = ...)] and the tests could be written more cleanly too. I’ll add those changes soon. What do you think?

Also, I don't think this will be possible because profile depends on cargo_args.

[Levi0804]

Hello, do these changes look good?

[s8sato]

@Levi0804

It seems we’re divided on whether to retain the deprecated --profile option outside of --cargo-args (#5493 (comment)). I expect we can resume the discussion next Tuesday.

[0x009922]

LGTM.

Though, I now see that the internal Profile in iroha_wasm_builder is somewhat opinionated. It isn't necessary that WASMs are built either with release or deploy profile (this one is even specific to our repo). I'd propose to remove any notion of Profile entirely, as well as handling of the --profile argument in --cargo-args. But that's for a separate PR.

[Levi0804]

yeah, I see.

It isn't necessary that WASMs are built either with release or deploy profile

then will they all just default to release?

[0x009922]

then will they all just default to release?

Hmm, not sure. Actually, I've looked into the how wasm-pack handles profiles, and it has a limited set of pre-defined profiles (--dev, --profiling, --release), and uses --release as the default. So, maybe it's fine to leave things as is in iroha-wasm-builder... What do you think?