Skip to content

Conversation

@tuler
Copy link
Member

@tuler tuler commented Jul 22, 2025

This PR let the default value of the ram-image to be driven by the cartesi-machine CLI tool (used by build).
If cartesi-machine is installed in the system (macOS or Linux) the default location of the linux.bin will be defined by the cartesi-machine installation.
If the CLI uses the cartesi-machine from the SDK, then it uses the default value of that.
Either way, if the cartesi.toml does not define the value of ram_image it will work regardless we use the system cartesi-machine or the docker sdk cartesi-machine.

Note that if the user defines a ram_image in the cartesi.toml that path will be used. If it's a system cartesi-machine the path will be considered a host path. If it's a docker sdk cartesi-machine the path will be considered a path inside the docker sdk.

@tuler tuler requested a review from endersonmaia July 22, 2025 23:44
@changeset-bot
Copy link

changeset-bot bot commented Jul 22, 2025

🦋 Changeset detected

Latest commit: e85216b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@cartesi/cli Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

Coverage Report for ./apps/cli

Status Category Percentage Covered / Total
🔵 Lines 25.18% 279 / 1108
🔵 Statements 25.08% 284 / 1132
🔵 Functions 25.77% 50 / 194
🔵 Branches 21.7% 112 / 516
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
apps/cli/src/config.ts 89.2% 77.22% 94.11% 89.01% 66-67, 218, 226, 236, 256, 269, 279, 282, 292, 333, 344-348, 360, 544, 566-571
apps/cli/src/machine.ts 0% 0% 0% 0% 5-126
Generated in workflow #639 for commit e85216b by the Vitest Coverage Report Action

Copy link
Contributor

@endersonmaia endersonmaia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that if the user defines a ram_image in the cartesi.toml that path will be used. If it's a system cartesi-machine the path will be considered a host path. If it's a docker sdk cartesi-machine the path will be considered a path inside the docker sdk.

I think we could at least emit a warning if the ram_image is defined in the cartesi.toml and sdk's cartesi-machine is used.

The user can't inject a linux.bin inside the sdk easily.

I think the best approach would mount the ram_image into sdk container during build. This way, cartesi.toml ram_image path would always be the host's one.

I won't block, we can decide this later.

@tuler
Copy link
Member Author

tuler commented Jul 23, 2025

I think the best approach would mount the ram_image into sdk container during build. This way, cartesi.toml ram_image path would always be the host's one.

It can be complicated to mount, the docker desktop must have permission to mount the specific directory of the ram_image.
I think the use case for replacing the ram_image is an advanced one, and users who need that will likely have the cartesi-machine on the host, or have the experience to extend the sdk with a custom one.

@tuler tuler merged commit 80bea97 into prerelease/v2-alpha Jul 23, 2025
4 checks passed
@tuler tuler deleted the feature/ram-image branch July 23, 2025 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants