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

bib: extract common platformFor() helper and add tests #693

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mvo5
Copy link
Collaborator

@mvo5 mvo5 commented Oct 25, 2024

[build on top of https://github.com//pull/692]

This commit extracts a common platformFor helper to generate
a platform for the (arch, imageForamat, UEFIVendor) tuple.

Note that this also change is not a pure refactor but change
the following behavior:

  • QCOW2Compat: "1.1" is no longer set because this is the
    default in qemu since 2013 (upstream commit 8ad1898c
    version qemu 1.7)
  • uefi vendor is now set for the disk image according to source
    info. This used to be hardcoded for aarch64 and unset for the
    others. This should be mostly irrelevant as only installers
    and the bootc legacy pipeline that uses ostree stages directly
    need it. But having it should do not harm and until PR#689
    is merged we will need for the bootc legacy pipeline.

But if this looks too risky I can just close the PR, adding architectures
happens rarely enough and maybe this refactor is overkill.

This commit populates the "img.Platform" struct when building
ISOs for s390x and ppc64le.

No tests right now sadly, this should be looked into but given
that it is known broken right now this change can hardly make
it worse.

Closes: osbuild#686
This commit extracts a common `platformFor` helper to generate
a platform for the (arch, imageForamat, UEFIVendor) tuple.

Note that this also change is not a pure refactor but change
the following behavior:

- `QCOW2Compat: "1.1"` is no longer set because this is the
  default in qemu since 2013 (upstream commit 8ad1898c
  version qemu 1.7)
- uefi vendor is now set for the disk image according to source
  info. This used to be hardcoded for aarch64 and unset for the
  others. This should be mostly irrelevant as only installers
  and the bootc legacy pipeline that uses ostree stages directly
  need it. But having it should do not harm and until PR#689
  is merged we will need for the bootc legacy pipeline.

But if this looks too risky I can revert, adding architectures
happens rarely enough and maybe this refactor is overkill.
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.

1 participant