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

Fix RISCV64 (VisionFive2) Tumbleweed example build #1871

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

ldevulder
Copy link
Contributor

@ldevulder ldevulder commented Dec 5, 2023

This allow to build the Tumbleweed example for VisionFive2 RISCV64 board with Makefile.

This PR should adds:

  • OS build support
  • ISO build support
  • Disk build support

Next step will be to try to enable mutli-arch build support to provide toolkit binary/image for x86-64, aarch64 and riscv64.

@ldevulder ldevulder self-assigned this Dec 5, 2023
@ldevulder ldevulder requested a review from a team as a code owner December 5, 2023 08:44
@ldevulder ldevulder marked this pull request as draft December 5, 2023 08:44
@ldevulder ldevulder changed the title Fix RISCV64 Tumbleweed example build Fix RISCV64 (VisionFive2) Tumbleweed example build Dec 5, 2023
@ldevulder ldevulder changed the title Fix RISCV64 (VisionFive2) Tumbleweed example build [DRAFT] Fix RISCV64 (VisionFive2) Tumbleweed example build Dec 5, 2023
Copy link
Contributor

@kkaempf kkaempf left a comment

Choose a reason for hiding this comment

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

We're building against a moving target here.
Not ideal, but also without any alternatives 🤷🏻

@ldevulder
Copy link
Contributor Author

We're building against a moving target here.
Not ideal, but also without any alternatives 🤷🏻

Yes, we have to wait until RISCV64 is added to Factory/Tumbleweed.

@ldevulder ldevulder changed the title [DRAFT] Fix RISCV64 (VisionFive2) Tumbleweed example build Fix RISCV64 (VisionFive2) Tumbleweed example build Dec 11, 2023
@ldevulder ldevulder marked this pull request as ready for review December 11, 2023 16:47
@codecov-commenter
Copy link

codecov-commenter commented Dec 11, 2023

Codecov Report

Attention: 350 lines in your changes are missing coverage. Please review.

Comparison is base (840c80c) 75.96% compared to head (415468c) 72.47%.
Report is 3 commits behind head on main.

Files Patch % Lines
pkg/action/mount.go 5.94% 169 Missing and 5 partials ⚠️
cmd/config/config.go 3.50% 55 Missing ⚠️
pkg/config/config.go 15.78% 48 Missing ⚠️
pkg/types/v1/config.go 0.00% 28 Missing ⚠️
cmd/mount.go 23.07% 19 Missing and 1 partial ⚠️
pkg/features/features.go 18.75% 12 Missing and 1 partial ⚠️
pkg/constants/constants.go 0.00% 7 Missing ⚠️
pkg/systemd/unit.go 0.00% 3 Missing ⚠️
cmd/build-disk.go 0.00% 1 Missing ⚠️
cmd/init.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1871      +/-   ##
==========================================
- Coverage   75.96%   72.47%   -3.50%     
==========================================
  Files          66       69       +3     
  Lines        6912     7274     +362     
==========================================
+ Hits         5251     5272      +21     
- Misses       1290     1624     +334     
- Partials      371      378       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Makefile Outdated Show resolved Hide resolved
@@ -19,17 +19,22 @@ DOCKER_SOCK?=/var/run/docker.sock
GIT_COMMIT?=$(shell git rev-parse HEAD)
GIT_COMMIT_SHORT?=$(shell git rev-parse --short HEAD)
GIT_TAG?=$(shell git describe --candidates=50 --abbrev=0 --tags 2>/dev/null || echo "v0.0.1" )
VERSION?=${GIT_TAG}-g${GIT_COMMIT_SHORT}
VERSION?=$(GIT_TAG)-g$(GIT_COMMIT_SHORT)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for changing {} to () ? In my eyes {} is used for variable references and () for running commands and assigning the output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In bash yes but in Makefile () can be used too (see here). As far as I remember in Makefile syntax it is used for variables declared inside the Makefile, which is the case for most of them (more all here). {} works too in GNU Makefile (not sure in non-GNU version, but we don't use it I know).

In our case we already had both, the idea here was mainly to be coherent and use the same for all vars, and use the initial Makefile syntax.

Fix the RISCV64 build example with openSUSE Tumbleweed.

This commit also adds the `build-vf2-disk` to build raw disk image for
VisionFive2 board,

This commit also ensure that we are using consistent protection on
Makefile variables.

Signed-off-by: Loic Devulder <ldevulder@suse.com>
Copy link
Contributor

@davidcassany davidcassany left a comment

Choose a reason for hiding this comment

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

LGTM

@ldevulder ldevulder merged commit 575597e into main Dec 13, 2023
14 checks passed
@ldevulder ldevulder deleted the fix-riscv64-example-build branch December 13, 2023 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants