refactor: Switch to cargo-xtask for building the project#77
refactor: Switch to cargo-xtask for building the project#77
Conversation
📝 WalkthroughWalkthroughAdds an xtask Rust crate and a cargo alias to the workspace; registers xtask as a workspace member; provides CLI subcommands to build artifacts, run the backend/CLI, and reset initramfs; updates docs, sample backend env, and gitignore to integrate the new workflow. (43 words) Changes
Sequence DiagramssequenceDiagram
actor User
participant Cargo as Cargo Alias
participant xtask as xtask CLI
participant Env as .env Config
participant Backend as Backend Build
participant Agent as Agent Build
participant CLI as CLI Build
User->>Cargo: cargo xtask build
Cargo->>xtask: run --package xtask --
xtask->>Env: load .env (if present)
Env-->>xtask: config values
xtask->>Backend: cargo build --package backend
Backend-->>xtask: backend artifact
xtask->>Agent: cargo build --package agent (musl)
Agent-->>xtask: agent artifact
xtask->>CLI: cargo build --package cli
CLI-->>xtask: cli artifact
xtask->>xtask: copy & chmod agent -> tmps
xtask-->>User: Build complete
sequenceDiagram
actor User
participant Cargo as Cargo Alias
participant xtask as xtask CLI
participant Env as .env Config
participant BackendProc as Backend Process
User->>Cargo: cargo xtask run-backend
Cargo->>xtask: run --package xtask --
xtask->>Env: load .env (if present)
Env-->>xtask: runtime env vars
xtask->>BackendProc: sudo env [...] cargo run --package backend
BackendProc->>BackendProc: initialize VMs & network
BackendProc-->>User: logs / running state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@xtask/src/main.rs`:
- Around line 179-182: The extension check.compare in the loop is wrong because
Path::extension() only returns the suffix after the last dot; update the test
that currently compares path.extension() to Some("cpio.gz") (used in the
reset_initramfs/remove loop) to instead inspect the file name string and check
if it ends_with(".cpio.gz") (e.g. via path.file_name().and_then(|n|
n.to_str()).map(|s| s.ends_with(".cpio.gz")) or by using
to_string_lossy().ends_with), then keep the same println!("Removing: {:?}",
path) and remove_file(&path)? behavior when the predicate matches. Ensure you
reference the same variable name path used in the snippet.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8fcd1fdc-ecbb-42b7-8ae7-666983244edb
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
.cargo/config.toml.gitignoreCargo.tomlQUICKSTART.mdbackend/.env.exemplebackend/README.mdxtask/Cargo.tomlxtask/src/main.rs
| if path.extension().and_then(|e| e.to_str()) == Some("cpio.gz") { | ||
| println!("Removing: {:?}", path); | ||
| std::fs::remove_file(&path)?; | ||
| } |
There was a problem hiding this comment.
Bug: Extension check will never match .cpio.gz files.
Path::extension() returns only the part after the last dot. For a file like foo.cpio.gz, it returns "gz", not "cpio.gz". This means the current filter will never match any initramfs files, and reset_initramfs will silently do nothing.
🐛 Proposed fix using file_name() instead of extension()
- if path.extension().and_then(|e| e.to_str()) == Some("cpio.gz") {
+ if path
+ .file_name()
+ .and_then(|n| n.to_str())
+ .is_some_and(|name| name.ends_with(".cpio.gz"))
+ {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if path.extension().and_then(|e| e.to_str()) == Some("cpio.gz") { | |
| println!("Removing: {:?}", path); | |
| std::fs::remove_file(&path)?; | |
| } | |
| if path | |
| .file_name() | |
| .and_then(|n| n.to_str()) | |
| .is_some_and(|name| name.ends_with(".cpio.gz")) | |
| { | |
| println!("Removing: {:?}", path); | |
| std::fs::remove_file(&path)?; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@xtask/src/main.rs` around lines 179 - 182, The extension check.compare in the
loop is wrong because Path::extension() only returns the suffix after the last
dot; update the test that currently compares path.extension() to Some("cpio.gz")
(used in the reset_initramfs/remove loop) to instead inspect the file name
string and check if it ends_with(".cpio.gz") (e.g. via
path.file_name().and_then(|n| n.to_str()).map(|s| s.ends_with(".cpio.gz")) or by
using to_string_lossy().ends_with), then keep the same println!("Removing:
{:?}", path) and remove_file(&path)? behavior when the predicate matches. Ensure
you reference the same variable name path used in the snippet.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/README.md`:
- Around line 37-43: The README currently claims VM guest-console logging is
enabled by setting VM_LOG_GUEST_CONSOLE=true but the sample output only shows
the normal startup line; update the backend/README.md to either (a) add an
example guest-console log line (e.g. prefixed with backend::vm or
[guest-console]) immediately after the "INFO backend: Starting Backend
server..." line, or (b) add a single clarifying sentence stating that the extra
guest-console logs (tagged like backend::vm or [guest-console]) only appear once
a VM job starts; reference the VM_LOG_GUEST_CONSOLE env var and the
backend::vm/[guest-console] tag so readers know what to look for.
In `@QUICKSTART.md`:
- Around line 28-34: Update the "Start backend (terminal A)" section to use a
single canonical start location by changing the text to "From repository root"
(matching earlier steps) and ensure the command shown is "cargo xtask
run-backend"; also add a brief note that the backend requires root privileges
and that the `run-backend` xtask will invoke sudo to set up network interfaces
and VMs. Make the same location/privilege clarification for the similar
occurrence around line 36 so both instances consistently state "From repository
root" and mention sudo/automatic sudo usage by `cargo xtask run-backend`.
- Line 13: Update QUICKSTART.md to replace the placeholder line about needing a
VM kernel at `backend/vmlinux` with concrete, actionable steps: either provide
shell commands to build the vmlinux (e.g., clone kernel source, configure, and
build steps) or include a direct external link to official build/download
instructions, and add a brief note on where to place the resulting `vmlinux`
file (the `backend/` directory) and any expected filename/permissions; ensure
the text references `backend/vmlinux` so readers can find the target location
and remove the “instructions to come” placeholder.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dd9e7a65-bfd2-4ae3-a71f-e1e9a4a8bf7c
📒 Files selected for processing (2)
QUICKSTART.mdbackend/README.md
| To enable verbose VM guest console logging, add `VM_LOG_GUEST_CONSOLE=true` to the `backend/.env` file. | ||
|
|
||
| Expected log: | ||
|
|
||
| ```text | ||
| INFO backend: Starting Backend server on 127.0.0.1:8080 | ||
| ``` |
There was a problem hiding this comment.
Clarify what output changes when VM_LOG_GUEST_CONSOLE=true.
On Line 37 you describe verbose guest-console logging, but the Line 42 sample shows only the standard backend startup log. Please either add a guest-console example line or state that extra logs appear only after a VM job starts.
Suggested doc patch
-To enable verbose VM guest console logging, add `VM_LOG_GUEST_CONSOLE=true` to the `backend/.env` file.
+To enable verbose VM guest console logging, add `VM_LOG_GUEST_CONSOLE=true` to the `backend/.env` file.
+Additional guest kernel/init logs appear when a VM is started (not at backend boot).
Expected log:
```text
INFO backend: Starting Backend server on 127.0.0.1:8080
+INFO backend::vm: [guest-console] ... </details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @backend/README.md around lines 37 - 43, The README currently claims VM
guest-console logging is enabled by setting VM_LOG_GUEST_CONSOLE=true but the
sample output only shows the normal startup line; update the backend/README.md
to either (a) add an example guest-console log line (e.g. prefixed with
backend::vm or [guest-console]) immediately after the "INFO backend: Starting
Backend server..." line, or (b) add a single clarifying sentence stating that
the extra guest-console logs (tagged like backend::vm or [guest-console]) only
appear once a VM job starts; reference the VM_LOG_GUEST_CONSOLE env var and the
backend::vm/[guest-console] tag so readers know what to look for.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
| ## 2) Start backend (terminal A) | ||
|
|
||
| Run backend as root from its project folder: | ||
|
|
||
| ```bash | ||
| cd backend | ||
| sudo LANGUAGES_CONFIG_PATH=./config/languages.json \ | ||
| AGENT_BINARY_PATH=./cloude-agentd \ | ||
| INIT_SCRIPT_PATH=./init.sh \ | ||
| VM_KERNEL_PATH=./vmlinux \ | ||
| VM_INITRAMFS_DIR=./tmp \ | ||
| VM_LOG_GUEST_CONSOLE=false \ | ||
| ../target/debug/backend | ||
| cargo xtask run-backend | ||
| ``` |
There was a problem hiding this comment.
Make backend start location explicit and consistent.
There’s a mismatch between “From repository root” earlier and “from its project folder” on Line 30 while Line 33 uses cargo xtask run-backend (which is root-oriented in this doc). Please keep one canonical location to avoid confusion.
Suggested doc patch
-## 2) Start backend (terminal A)
-
-Run backend as root from its project folder:
+## 2) Start backend (terminal A)
+
+From repository root, run:
```bash
cargo xtask run-backendNote: The backend requires root privileges (sudo) to setup network interfaces and run VMs. The run-backend command will automatically use sudo to start the backend.
</details>
Also applies to: 36-36
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @QUICKSTART.md around lines 28 - 34, Update the "Start backend (terminal A)"
section to use a single canonical start location by changing the text to "From
repository root" (matching earlier steps) and ensure the command shown is "cargo
xtask run-backend"; also add a brief note that the backend requires root
privileges and that the run-backend xtask will invoke sudo to set up network
interfaces and VMs. Make the same location/privilege clarification for the
similar occurrence around line 36 so both instances consistently state "From
repository root" and mention sudo/automatic sudo usage by cargo xtask run-backend.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
| ### Required in practice | ||
|
|
||
| - `VM_KERNEL_PATH` (default `./vmlinux`): Linux kernel used to boot each VM. | ||
| - `AGENT_BINARY_PATH` (default `./cloude-agentd`): binary injected into initramfs. | ||
| - `INIT_SCRIPT_PATH` (default `./init.sh`): init script injected as `/init`. | ||
|
|
||
| ### Common runtime settings | ||
|
|
||
| - `BACKEND_SERVER_ADDR` (default `127.0.0.1:8080`) | ||
| - `BRIDGE_NAME` (default `cloudebr0`) | ||
| - `IP_RANGE` (default `10.39.1.0`) | ||
| - `IP_MASK` (default `24`, must be `<= 30`) | ||
| - `LANGUAGES_CONFIG_PATH` (default `./config/languages.json`) | ||
| - `VM_INITRAMFS_DIR` (default `./tmp`) | ||
| - `IP_ALLOCATIONS_PATH` (default `./tmp/ip_allocations.json`) | ||
| - `VM_LOG_GUEST_CONSOLE` (default `false`) | ||
| - `true/1/yes/on`: print guest kernel+init logs in backend terminal | ||
| - `false`: keep backend logs clean |
There was a problem hiding this comment.
Maybe remove that if it is already documented in .env.example ?
|
|
||
| ## Environment variables | ||
|
|
||
| Configuration is read from `backend/.env` (create from `.env.exemple` if needed). |
There was a problem hiding this comment.
Maybe move that line above cargo xtask run-backend, since it is a requirement (?)
| println!("Making agent binary executable..."); | ||
| let status = Command::new("chmod").arg("+x").arg(&agent_dst).status()?; | ||
| if !status.success() { | ||
| return Err("Failed to make agent binary executable".into()); | ||
| } |
There was a problem hiding this comment.
No need, binaries produced by cargo are executable by default
There was a problem hiding this comment.
Indeed, this was a direct translation of the quickstart but is unneccessary
| .arg(format!( | ||
| "PATH=/usr/sbin:{}", | ||
| env::var("PATH").unwrap_or_default() | ||
| )) |
There was a problem hiding this comment.
PATH not needed, was removed from QUICKSTART, but backend/README.md duplicated the info
2 participants
Summary by CodeRabbit
New Features
Documentation
Chores