feature(cargo-miden): passthrough unknown options in cargo miden build to midenc

[greenhat]

Close #320

This PR refactors cargo miden build to parse all command-line arguments through the midenc compiler's Compiler struct instead of using a separate custom argument parser. This adds hidden cargo-specific options (--release, --manifest-path, --workspace, --package) to the Compiler struct in midenc-compile, which are recognized by cargo miden build and forwarded to the underlying cargo build invocation. The midenc options (currently only --emit) are passed to the compilation stage.

Note: When the --emit option is passed it is ignored since cargo-miden uses the -o option for the Miden package file and it overrides the --emit. See created #724 for details. I'd like to tackle it separately since this PR is about options passthrough.

[bitwalker]

I was imagining the handling of arguments would work more like so:

1. Add options we want to recognize as build options for cargo to the set of midenc options, e.g. --release. Thus midenc's options are a superset of the cargo options we want to support.
2. Capture all arguments after cargo miden build as a Vec<OsString>/Vec<String>, and use Compiler::try_parse(&args) to parse those arguments into the midenc compiler options (IIRC try_parse is the correct method of clap::Parser, but I'd have to check the docs to know, in any case, you get the gist).
3. Override any of the Compiler options directly, as you see fit/as needed by cargo miden build, rather than trying to parse/modify the raw argument vector.
4. Extract any cargo-specific options we want to forward to cargo from the Compiler options when invoking cargo

That will be far simpler than the approach here IMO.

NOTE: With my PR that switches to miden-vm v0.18, I've also simplified the midenc-driver interface significantly (i.e. the subcommand handling is gone, and midenc is equivalent to the previous midenc compile). That has made instantiating the driver significantly simpler as well, so we could base this PR on that branch instead to take advantage of that, since we should be able to merge the v0.18 upgrade this week.

[greenhat]

I was imagining the handling of arguments would work more like so:

1. Add options we want to recognize as build options for cargo to the set of midenc options, e.g. --release. Thus midenc's options are a superset of the cargo options we want to support.
2. Capture all arguments after cargo miden build as a Vec<OsString>/Vec<String>, and use Compiler::try_parse(&args) to parse those arguments into the midenc compiler options (IIRC try_parse is the correct method of clap::Parser, but I'd have to check the docs to know, in any case, you get the gist).
3. Override any of the Compiler options directly, as you see fit/as needed by cargo miden build, rather than trying to parse/modify the raw argument vector.
4. Extract any cargo-specific options we want to forward to cargo from the Compiler options when invoking cargo

That will be far simpler than the approach here IMO.

I did not think about this. I like the idea. Besides --release there are also --manifest-path, --workspace and --package options that we probably want to support (also --features?). Do you think they fit to be midenc options?

NOTE: With my PR that switches to miden-vm v0.18, I've also simplified the midenc-driver interface significantly (i.e. the subcommand handling is gone, and midenc is equivalent to the previous midenc compile). That has made instantiating the driver significantly simpler as well, so we could base this PR on that branch instead to take advantage of that, since we should be able to merge the v0.18 upgrade this week.

Sure. Do you mean this one - #681? It looks stale.

[greenhat]

@bitwalker In our discussion elsewhere I mentioned the --manifest-path, --workspace, etc. options might not be a good fit in midenc Compiler struct options and you suggested I look into codegen and unstable options as an example of compartmentalization. AFAIU, they are accessible only via a prefix (-C for codegen and -Z for unstable). But the premise of going with familiar cargo options is that they will be used without the prefix as one uses them in cargo build --workspace . If let's say we choose the -B prefix then --workspace can only be invoked as cargo miden build -B --workspace.
There is a value in keeping the cargo miden build compatible with cargo build options. I'd prefer to avoid the compartmentalization and add these options in the Compiler struct so they can be invoked without any prefix. What do you think?

[bitwalker]

@greenhat I think that's fine, let's make them first-class options then, perhaps just decorated with #[clap(hidden(true))] (or whatever the option is, maybe it was hide(true)) to keep them out of the default help of midenc

[greenhat]

Done. Ready for review.