[refactor] Improve the VmBuilder::with_plugin API

[apepkuss]

In this PR, the VmBuilder::with_plugin API is updated to make it much more user-friendly, flexible and easier-to-use for developers.

[juntao]

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.

---

Overall, the patch titled "[refactor] Improve the VmBuilder::with_plugin API" includes several changes to improve the VmBuilder::with_plugin API. The modifications made seem to be consistent with the codebase, and there do not appear to be any major issues or errors. However, the impact of these changes on the wider system is unclear without understanding the overall context of the codebase. It would be helpful to have more documentation explaining the changes and providing examples of usage.

There are a few specific findings to note:

• The use of the with_plugin method internally in the modified methods is a good improvement that eliminates duplicated code.
• The addition of the with_plugin_rustls method introduces a new plugin called "rustls". The purpose and impact of this new plugin should be clarified in the pull request description or communicated to the team for further discussion.
• There is an inconsistency in handling the mnames parameter in the with_plugin method and the modified methods. This should be resolved for better code clarity and consistency.
• There are some changes in the test code and the VmBuilder configuration, but no potential problems were identified with these changes.

Additionally, there are a few points regarding the installation step, the cmake command, and the skipped test:

• The addition of the libssl-dev package to the installation step and the -DWASMEDGE_PLUGIN_WASI_CRYPTO=On flag to the cmake command should be documented clearly.
• It's important to understand why the test_vmbuilder test is skipped and whether it is a temporary or intended change.
• Adding comments or explanations in the code changes can improve readability and understanding for other contributors.

Details

Commit bcd144e418fbfd6d1fcd28acc86250184df1921b

Key changes in this patch:

• The plugins field in the VmBuilder struct has been modified to store a vector of tuples (String, Vec<String>) instead of (String, String).
• The with_plugin_wasi_nn, with_plugin_wasmedge_process, and with_plugin methods have been updated to use a Vec<String> instead of a single String argument.
• The with_plugin method has also been updated to accept an optional Vec<T> for the mnames argument.

Potential problems:

• There doesn't seem to be any problem with the modifications made in this patch. The changes improve the VmBuilder::with_plugin API by allowing multiple plugin module names to be specified for a single plugin name. This provides more flexibility when adding plugins to the VmBuilder.
• The code looks properly refactored and the changes seem to be consistent with the rest of the codebase.
• However, without understanding the overall context of the codebase, it is difficult to determine the impact of these changes on the wider system. It would be helpful to understand the usage of the VmBuilder and the impact of these changes on other parts of the codebase.
• Additionally, it might be worth considering adding documentation comments to the updated methods to explain the changes in the API and provide examples of usage.

Commit f88819391cbbc879b23b337674e61a6221f54e1a

The key changes in this pull request are as follows:

• The with_plugin_wasi_nn, with_plugin_wasi_crypto, with_plugin_wasmedge_process methods in the VmBuilder struct have been modified to use the with_plugin method internally.
• A new method with_plugin_rustls has been added to the VmBuilder struct.

First, the use of the with_plugin method internally in the modified methods is a code improvement that eliminates duplicated code. This change improves the maintainability of the code.

Second, the addition of the with_plugin_rustls method introduces a new plugin called "rustls" to the VmBuilder. This new plugin is not mentioned in the pull request's title or description. It is important to clarify the purpose and impact of this new plugin in the pull request description or communicate it to the team for further discussion.

The potential problem identified is the inconsistency in handling the mnames parameter in the with_plugin method and the modified methods. In the modified methods (with_plugin_wasi_nn, with_plugin_wasi_crypto, and with_plugin_wasmedge_process), the mnames parameter is set to None, whereas in the with_plugin method, it accepts an Option<Vec<&str>>. This inconsistency should be resolved for better code clarity and consistency.

Commit 10c736c6c9e4468675da494bc71e6810b23cf19d

Key changes:

• The code in VmBuilder::with_plugin has been refactored to improve the API.
• The function now takes two arguments: pname (name of the plugin) and mnames (names of the plugin modules to be registered).
• The mnames parameter is now an Option<Vec<&str>>, allowing it to be None if all modules in the plugin are to be registered.

Potential problems:

• No potential problems were identified in this patch.

Overall, this patch appears to be a simple refactoring of the VmBuilder::with_plugin API.

Commit d23e93b215fd05ce30a02562a0938c61e16f799d

Key Changes:

• Updated test code in vm.rs file.
• Uncommented .with_plugin_wasi_crypto() method call.
• Commented out .with_plugin("rustls", "rustls_client") method call.
• Uncommented .with_plugin("wasi_crypto", None) method call.
• Commented out .with_plugin("rustls", None) method call.

Potential Problems:

• There are no identified potential problems in this patch. The changes seem to be updating the test code and modifying the configuration of the VmBuilder with plugins.

Commit 53d152df5f08a23ff02c7121c9016d48cfe07e8f

Key changes in the patch include:

• Added libssl-dev package to the installation step.
• Added -DWASMEDGE_PLUGIN_WASI_CRYPTO=On flag to the cmake command.

Potential problems:

• It's unclear why the libssl-dev package was added to the installation step. This change should be documented properly in the commit message or pull request description.
• It's unclear why the -DWASMEDGE_PLUGIN_WASI_CRYPTO=On flag was added to the cmake command. This change should also be documented clearly in the commit message or pull request description.
• In the test stage, the test_vmbuilder is skipped. It's important to understand why this test is being skipped and whether it's temporary or intended.
• There are no comments or explanation provided in the code changes. Adding comments can improve readability and understanding for other contributors.

[apepkuss]

@hydai Could you please help review this PR? Thanks a lot!

[apepkuss]

@hydai Thanks for the review!