-
Notifications
You must be signed in to change notification settings - Fork 110
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
Restructure the CLI #702
Comments
A few questions to start:
|
|
More explicitly for this proposal, I'm intending to validate that the option groups specified through |
👍
Merging them would help to avoid unproductive discussions about when a new codegen-related flag is actually a debug flag or the opposite.
That makes sense for now. |
This commit introduces a `build` command in the CLI, which behaves exactly as the compile command and introduces a deprecation warning to the existing `compile` command. Additionally, as a preparation for the CLI redesign discussed in bytecodealliance#702, this commit introduces a small refactoring of the code generation process, by: * Introducing a code generator builder, to abstract and validate all the code generation option combinations. * Introducing proper static and dynamic code generator modules, making it easier to divide the responsilibities of each. The main motivation for the refactoring is to: * Make it easier to finalize the CLI redesign. * Share code between the `compile` and `build` command while they must be equivalent. * Make it easier to keep the `compile` command frozen while it becomes deprecated and at the same time evolve the `build` command independently. NB: Given that the `compile` and `build` command are exactly the same, and that this change is purely mechanical; this change doesn't introduce integration tests for the `build` command. The plan is to introduce tests once other options are added to the `build` command as part of the CLI redesign. Alternatively, tests can be added, however, it would require either duplicating the entire test suite or adding testing infrastructure to minimize duplication, which will increase the size of the change.
* Introduce the `build` command in the CLI This commit introduces a `build` command in the CLI, which behaves exactly as the compile command and introduces a deprecation warning to the existing `compile` command. Additionally, as a preparation for the CLI redesign discussed in #702, this commit introduces a small refactoring of the code generation process, by: * Introducing a code generator builder, to abstract and validate all the code generation option combinations. * Introducing proper static and dynamic code generator modules, making it easier to divide the responsilibities of each. The main motivation for the refactoring is to: * Make it easier to finalize the CLI redesign. * Share code between the `compile` and `build` command while they must be equivalent. * Make it easier to keep the `compile` command frozen while it becomes deprecated and at the same time evolve the `build` command independently. NB: Given that the `compile` and `build` command are exactly the same, and that this change is purely mechanical; this change doesn't introduce integration tests for the `build` command. The plan is to introduce tests once other options are added to the `build` command as part of the CLI redesign. Alternatively, tests can be added, however, it would require either duplicating the entire test suite or adding testing infrastructure to minimize duplication, which will increase the size of the change. * Collapse `provider_version` call in `builder` * Improve some documentation * Clarify `source_compression` docs * Stylistic changes * Add NOTICE to help text of `compile` * Use `eprintln!` instead of `println!` * Use `build` in the README.md
This commit refactors the integration tests to use the `javy-runner` crate. The main motivation for this change is to reduce duplication and make it easier to test more complex scenarios in the future e.g., the upcoming changes in bytecodealliance#702
* Refactor integration tests This commit refactors the integration tests to use the `javy-runner` crate. The main motivation for this change is to reduce duplication and make it easier to test more complex scenarios in the future e.g., the upcoming changes in #702 * Style changes * More style changes
Hi Saúl, I have one question (or feature request) regarding the CLI refactoring. As the Javy-CLI npm package was recently deprecated, would it be possible to release the javy-cli crate on crates.io similar to the javy crate itself? Ideally, the crate would also include a Technically, as a fully-fledged replacement for the npm package, a new wrapper using napi.rs could be created, where the workaround (downloading the binary from Github and executing it in a child-process) is no longer needed. I see the point that this does introduce a lot more implementation efforts for sure though. Would love to hear your opinion on these suggestions 😀 I'd be open to support this also. |
Would you be open to describing how you were using the |
Hi Jeff, Sure! I am creating tooling around the Javy CLI. This means that I create a wrapper wherein as part of the overarching build also the Javy CLI is used. As the Javy-CLI is used as one part in a bigger build process, we effectively use it as a library which the existing npm package kind of simulated (as it effectively delegated the function calls to child process invocations of the binary builds). Such use cases would be much simplified if the APIs the Javy CLI provides as an executable were also available as a library. And of course, publishing the CLI crate on crates.io would be very helpful as it could then be used as a dependency. In my particular (and surely special) case where ultimately an npm package is created, I could set the javy-cli-lib crate as a dependency in a project where napi.rs or a different technology is used to transform it into a native Node Addon. From technological point of view, it should also be quite easy as Rust allows one library crate and one-to-many binary crates per project. If Also for using the Javy CLI standalone – without a wrapper use case – publishing it is beneficial as consumers could simply install it via cargo I hope this describes the use case and motivation a bit better, otherwise feel free to come back to me. I am happy to discuss :) |
Okay, it sounds like you use a Javy binary inside a cross-platform Node (or Node-API compatible) application and would like a way to get either a single binary that's compatible for whatever the current platform is or get an embeddable library you can call into. Is that correct?
It's actually a bit more complicated 😅. Cargo/Rust don't really have a great way of expressing that a Rust crate targeting a native architecture has a dependency on a Rust crate that should be compiled to Wasm as is the case with a native |
Hi @konradkoschel -- just wanted to add a bit to what Jeff mentioned above. In general, I agree that it would be useful to have the
As mentioned in the previous comment, the biggest wrinkle here is probably figuring out a way to model the dependency between the In general I wanted to say that I'm personally in favor of publishing the CLI to crates.io. However, I don't have the bandwidth to take this feature on at the moment, but we welcome contributions. If you'd like I can help guide the development and review PRs. If that's the case, a good first step could to open an issue so that we can discuss and agree on a path forward. |
Thank you two for the answers! I fully understand the issue with the WASM dependency – this makes it a lot more complicated than thought for sure. I'll make up my mind on how to best address this and if also my bandwidth allows it, I'll try to prepare a PR 👍 |
This proposal suggests a restructuring of Javy's CLI interface to:
Command Flexibility
Javy currently offers a wide range of configuration options that control the JavaScript language features available to the generated code. However, there are limitations:
Semantically Meaningful Commands, Sub-Commands and Options
The CLI currently features two main commands:
compile
andemit-provider
. The compile command does not accurately reflect its actions and could be better named to represent actual AOT compilation of JavaScript, a feature we plan to explore in the future.Proposal
The high-level proposal includes:
compile
command in favor of a new command, tentatively namedbuild
. Avoid introducing breaking changes or new features through thecompile
command.build
command with meaningful options to control JavaScript and code generation features as described above.compile
command to emit a deprecation notice, indicating that it will be deprecated in the next major release of the CLI.CLI Interface
The proposal suggests, according to the bullet points above, to have the new
build
command look like:For example, in order to control the name section generation, one could do:
The suggested CLI layout, takes inspiration from Wasmtime's CLI layout, which, I think offers multiple benefits, notably:
-D
group, across thebuild
andemit-provider
commands, for cases in which the user wants to controlwalrus
/wasm-opt
options.enable-<group>-<option>
anddisable-<group>-<option>
. Aside from the verbosity, with this approach, we need to introduce at least two options per group (assuming each option allows at least enabling or disabling).Proposed order of operations
build
command, which will initially behave similar to the current compile command.compile
command.-J
options group-D
options group-C
options group4
of the CLI with the deprecatedcompile
command. (After having collected feedback / agreed on the deprecation timeline)The text was updated successfully, but these errors were encountered: