Skip to content

feat(arm64): create docker image for mkimage-iso-bios (amd64 toolsuite)#4103

Open
SmartArray wants to merge 2 commits intolinuxkit:masterfrom
SmartArray:feat/iso-arm64
Open

feat(arm64): create docker image for mkimage-iso-bios (amd64 toolsuite)#4103
SmartArray wants to merge 2 commits intolinuxkit:masterfrom
SmartArray:feat/iso-arm64

Conversation

@SmartArray
Copy link

@SmartArray SmartArray commented Feb 4, 2025

- What I did
This is a WIP PR. I am using this great project on my M1 Mac and I noticed a couple of output formats are not working since they rely on syslinux alpine dependency.

I checked for what we are using the dependency and it only seemed to be the BIOS bootloader file, and the boot process. Since those are only being used to build the ISO image, this should be fine.

Note that the resulting image cannot run on arm64 systems, but only amd64, since the boot process is using AMD64 opcodes.

Warning

Dear maintainers, please verify if this is useful at all, because maybe people would expect to be able to create arm64 ISO images. However, it is common knowledge that arm64 commonly uses UEFI instead of BIOS for booting. So I hope this Pull Request is useful.
I put this PR into WIP, because we might be able to apply this for other output formats too. However, I haven't checked them yet.

- How I did it
I removed the syslinux dependency and introduced a multistage build, in which I pull the amd64 version of alpine and copy the required files

- How to verify it
Create ISO-bios image on M1-M3 Mac. It should be able to create an ISO image.

- Description for the changelog
Add output support for amd64 architecture

- A picture of a cute animal (not mandatory but encouraged)
2273d8ea3f69d241ef84030187dfa90c

@deitch
Copy link
Collaborator

deitch commented Feb 5, 2025

Oh, oh, I like this. We never did it because it always was x86 (BIOS is x86 only, as opposed to arm64, which is UEFI, as you pointed out), but there is no reason you should not be able to build for x86 while on arm64 (or riscv64).

Please do the following:

  1. squash the commits to a single one
  2. sign the commit(s) git commit -s
  3. run ./scripts/update-component-sha.sh --pkg ./tools/mkimage-iso-bios to update any dependencies.

@deitch
Copy link
Collaborator

deitch commented Feb 5, 2025

Oh yeah, and remove from "draft"

@SmartArray
Copy link
Author

@deitch The PR is in draft mode because I could need your advice on what output formats did not work on arm64. And whether there is a different architecture you would like me to check.

Thanks for pointing out the contributing guidelines. I will do it right away

@SmartArray SmartArray marked this pull request as ready for review February 5, 2025 20:12
@SmartArray
Copy link
Author

I checked the other ones, it seems that we should leave it with mkimage-iso-bios for now

@SmartArray SmartArray force-pushed the feat/iso-arm64 branch 2 times, most recently from e0c41c9 to 24d4166 Compare February 5, 2025 20:31
@SmartArray
Copy link
Author

@deitch

Here we go. Everything should be ready for review now.

@deitch
Copy link
Collaborator

deitch commented Feb 6, 2025

One of the commits isn't signed. Either sign that one, too, or squash them to a single signed commit, please.

@SmartArray SmartArray force-pushed the feat/iso-arm64 branch 2 times, most recently from e0b2a8a to 812a9f1 Compare February 6, 2025 19:07
@SmartArray
Copy link
Author

Here we go!

@deitch
Copy link
Collaborator

deitch commented Feb 6, 2025

The DCO still is failing. Please check how to pass it here

Signed-off-by: Yoshi Jaeger <github@jaeger.berlin>
Signed-off-by: Yoshi Jaeger <github@jaeger.berlin>
@SmartArray
Copy link
Author

@deitch

Weird, I think it was due to the missing Signed-Off message. The DCO check seems to be valid now, but there is no verified tag next to my commit. Is that expected?

@deitch
Copy link
Collaborator

deitch commented Feb 7, 2025

Looks good to me. We don't actually exercise it as part of CI (the tests there mostly use UEFI), so please confirm that you did a build and booted it successfully.

@deitch
Copy link
Collaborator

deitch commented Feb 7, 2025

Tests are failing because tools/ packages are not pushed automatically by CI. I can handle that and restart the failing tests.

@deitch
Copy link
Collaborator

deitch commented Feb 7, 2025

Pushed out, restarted failing tests. Let's see if CI is clean.

@deitch
Copy link
Collaborator

deitch commented Feb 7, 2025

Getting failure here. Did you forget to copy over some dependencies?

@SmartArray
Copy link
Author

SmartArray commented Feb 7, 2025

Thanks for your time

I see this

[STDERR  ] 2025-02-07T10:07:20.751080629Z: Error loading shared library libuuid.so.1: No such file or directory (needed by /syslinux/isohybrid)
[STDERR  ] 2025-02-07T10:07:20.751091189Z: Error relocating /syslinux/isohybrid: uuid_generate: symbol not found
[STDERR  ] 2025-02-07T10:07:20.751101128Z: 
[STDERR  ] 2025-02-07T10:07:20.751113602Z: 2025/02/07 10:07:20 error during command execution: error writing outputs: Error writing iso-bios output: docker run linuxkit/mkimage-iso-bios:ca3da0ac21b1b307eb922163ee4607014d28ea5b failed: exit status 127 output:

And that's confusing because all we did was removing syslinux dep.

Screenshot 2025-02-07 at 11 34 44

So that points to the fact that libuuid is a dependency of syslinux...

Edit: Yes, I just verified this.
https://pkgs.alpinelinux.org/package/edge/main/x86/syslinux

So if we add libuuid to the list of deps, we should be fine (works on all platforms)

@deitch
Copy link
Collaborator

deitch commented Feb 7, 2025

Yes I think that makes sense. The current method was apk add syslinux, which included all dependencies. By doing it in an earlier stage and just copying over the files you know, you risk missing some.

Might be worth running a test manually. Build this locally and try to run syslinux and see what dependencies are missing before pushing out again.

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.

2 participants