-
Notifications
You must be signed in to change notification settings - Fork 109
add workspace Cargo.toml #217
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @abel-von, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
I've introduced a root Cargo.toml to establish a Rust workspace, which will improve project management and reduce disk space usage. Concurrently, I've updated the shim crate to align with a new containerd sandbox API, involving significant refactoring of how task and process data are handled. Minor adjustments were also made to the quark crate for argument parsing and to Makefile build commands to reflect the new workspace structure.
Highlights
- Introduced a Rust workspace: A new Cargo.toml file was added at the project root, defining a workspace that includes vmm/task, vmm/sandbox, vmm/common, runc, quark, wasm, and shim. This centralizes build configurations, including release profiles and dependency patches.
- Updated shim crate for new containerd sandbox API: The shim crate was refactored to align with an updated containerd sandbox API. This involved modifying SandboxData to include a Sandbox struct, changing the Sandbox trait implementation to SandboxTrait (from containerd_shim), and overhauling prepare_container and prepare_exec to manage task and process resources by storing them as extensions within the sandbox data, rather than making direct prepare calls to the sandbox_v2_cli. New helper methods (get_tasks_extension, update_tasks_extension, update) were added to handle the serialization and deserialization of task resources to/from sandbox extensions.
- Enhanced quark crate with command-line argument parsing: The quark crate now utilizes the clap library to parse command-line arguments for configuration, such as working directory (--dir) and listen address (--listen), improving its flexibility.
- Streamlined build process: The Makefile was updated to reflect the new workspace structure, adjusting cp commands to correctly locate built binaries in the unified target/release directory.
- Centralized release profiles: Redundant [profile.release] sections were removed from individual sub-crate Cargo.toml files, as these are now managed by the root Cargo.toml workspace.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a Cargo workspace to unify the project's build process and dependency management, which is a great structural improvement. It also includes updates to the shim and sandbox components to align with a new API. The changes are mostly correct and well-implemented. However, I've found a critical bug in the purge logic that could lead to data loss, along with a few medium and high severity issues related to error handling, testing, and documentation. Please address these points to improve the robustness and correctness of the code.
39bd207 to
7c1f1fa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a workspace Cargo.toml configuration to enable opening the entire project without excessive disk space usage, and updates the shim code to be compatible with the new sandbox API in containerd.
- Introduces a workspace-level Cargo.toml with build optimization settings
- Updates shim code to work with the new containerd sandbox API structure
- Consolidates profile settings and dependency patches at the workspace level
Reviewed Changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Cargo.toml | New workspace configuration with build profiles and dependency patches |
| wasm/Cargo.toml | Adds default feature for wasmtime |
| wasm/src/sandbox.rs | Updates task service creation and imports for new sandbox API |
| vmm/task/Cargo.toml | Removes individual profile settings and updates libcontainer version |
| vmm/sandbox/Cargo.toml | Removes individual profile settings |
| vmm/common/Cargo.toml | Removes individual profile settings |
| vmm/task/src/youki.rs | Refactors pipe creation to use standard library fchown |
| shim/src/task.rs | Updates API calls to match new sandbox interface |
| shim/src/sandbox.rs | Major refactoring for new sandbox API structure |
| shim/src/data.rs | Adds Sandbox field to SandboxData |
| quark/src/sandbox.rs | Adds update method implementation |
| quark/src/main.rs | Adds command-line argument parsing |
| quark/src/args.rs | New command-line argument definitions |
| quark/Cargo.toml | Adds clap dependency |
| Makefile | Updates build paths for workspace structure |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
7c1f1fa to
97daeac
Compare
c6d6c93 to
2191d3e
Compare
so that we can open the whole project without too much disk space occupied Signed-off-by: Abel Feng <[email protected]>
Signed-off-by: Abel Feng <[email protected]>
Signed-off-by: Abel Feng <[email protected]>
2191d3e to
a7e3d38
Compare
kevin-wangzefeng
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
Burning1020
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
so that we can open the whole project without too much disk space occupied.
and shim seems not updated with the new sandbox api in containerd, along with other crates. we have to update codes in shim to make it compile.