Skip to content
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

cli: enforce build for linux/riscv64 platform #73

Merged
merged 3 commits into from
Oct 22, 2024

Conversation

endersonmaia
Copy link
Contributor

Since the target is always a Cartesi Machine (riscv64), cartesi build should enforce the docker build environment via --platform argument.

This way Dockerfile from cartesi/application-templates should be something like this.

javascript

#BUILDER
FROM --platform=$BUILDPLATFORM node:20.16.0-bookworm AS build-stage
(...)

#TARGET
FROM cartesi/node:20.16.0-jammy-slim
(...)

We should only need to define --platform when we want to run something on the host platform, for a cross-compilation or build stage that could be faster without the linux/riscv64 qemu of the docker builder.

The example above uses the $BUILDPLATFORM, which is recommended so it will use the host platform.

The second and last FROM doesn't need a --platform since this will be enforced by the cartesi build.

@endersonmaia endersonmaia self-assigned this Sep 9, 2024
Copy link

changeset-bot bot commented Sep 9, 2024

🦋 Changeset detected

Latest commit: 42a2e27

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

@tuler
Copy link
Member

tuler commented Sep 9, 2024

I think this will break compatibility with current Dockerfiles out there, right?

@endersonmaia
Copy link
Contributor Author

I think this will break compatibility with current Dockerfiles out there, right?

Yes. 🤔

What do you suggest? make it a minor ?

And we should also update the applications-templates.

@endersonmaia endersonmaia changed the title feat(cli): enforce build for linux/riscv64 paltform cli: enforce build for linux/riscv64 platform Sep 9, 2024
@tuler
Copy link
Member

tuler commented Sep 9, 2024

What do you suggest? make it a minor ?

I think it would be a minor.

@tuler
Copy link
Member

tuler commented Oct 14, 2024

As this breaks Dockerfile compatibility let's put it together with other changes that changes Dockerfile, like the v2 initiative.

@endersonmaia endersonmaia force-pushed the feature/enforce-target-paltform branch from 3c6ea12 to 5388268 Compare October 22, 2024 18:16
@endersonmaia endersonmaia changed the base branch from main to prerelease/v2-alpha October 22, 2024 18:16
@endersonmaia endersonmaia force-pushed the feature/enforce-target-paltform branch 2 times, most recently from 2264272 to 69f6a5e Compare October 22, 2024 18:25
Copy link
Contributor

github-actions bot commented Oct 22, 2024

Coverage Report for ./apps/cli

Status Category Percentage Covered / Total
🔵 Lines 30.76% 292 / 949
🔵 Statements 30.9% 297 / 961
🔵 Functions 37.41% 52 / 139
🔵 Branches 26.95% 117 / 434
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Changed Files
apps/cli/src/builder/docker.ts 92.1% 78.57% 75% 94.59% 40, 130-136
Generated in workflow #206 for commit 42a2e27 by the Vitest Coverage Report Action

since the target is always a Cartesi Machine (riscv64), cartesi build
should enforce the docker build environment via --platform argument
@endersonmaia endersonmaia force-pushed the feature/enforce-target-paltform branch from 69f6a5e to 60736db Compare October 22, 2024 18:56
@endersonmaia endersonmaia merged commit 29ba930 into prerelease/v2-alpha Oct 22, 2024
2 checks passed
@endersonmaia endersonmaia deleted the feature/enforce-target-paltform branch October 22, 2024 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants