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

README.md: A few tweaks #17

Closed
wants to merge 1 commit into from
Closed

Conversation

cgwalters
Copy link
Contributor

  • Clarify functionality
  • Link to centos-bootc
  • Use new image reference
  • Target c9s because ELN is more aggressive and has bugs

- Clarify functionality
- Link to centos-bootc
- Use new image reference
- Target c9s because ELN is more aggressive and has bugs
@cgwalters cgwalters marked this pull request as ready for review November 28, 2023 15:15
@cgwalters
Copy link
Contributor Author

Ah yes, switching to c9s is going to be a problem because of the hardcoded uefi vendor stuff:

  "uefi": {
    "vendor": "fedora",
    "install": true,
    "unified": true
  },

Which gets into the bootupd handling, which in turn gets into using bootc install.

@cgwalters cgwalters marked this pull request as draft November 28, 2023 15:22
@cgwalters
Copy link
Contributor Author

which in turn gets into using bootc install.

We're going to need to have a real debate about this soon because it's quite important.

@achilleas-k
Copy link
Member

Ah yes, switching to c9s is going to be a problem because of the hardcoded uefi vendor stuff

Let's pull all that out into a config then. Build-root repos are already externally defined but embedded at compile time. We can do the same so that the binary is self contained or keep a config with some strings in the repo.

@teg
Copy link
Member

teg commented Nov 28, 2023

Sounds like the kind of stuff that should be declared in the container metadata? at least if it declares the distro, we can have a mapping (as I believe we do) from distro to things like UEFI vendor.

Copy link
Contributor

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM

@cgwalters cgwalters mentioned this pull request Dec 5, 2023
@cgwalters
Copy link
Contributor Author

Sounds like the kind of stuff that should be declared in the container metadata? at least if it declares the distro, we can have a mapping (as I believe we do) from distro to things like UEFI vendor.

Yes, in theory we could do that. But this also leads to #18

where bootupd basically handles this dynamically. Now in theory of course we could change the osbuild bootloader code to do the same; I actually don't understand why the vendor is hardcoded instead of being detected today.

@ondrejbudai
Copy link
Member

What about splitting this PR? I'm happy to make explicit in the README.md that this is paired with centos-bootc.

However, we are not ready to make the centos container the default experience, we still have some issues with them, see #20. Note that @mvo5 is working on a short-term fix for this, before we can fully switch to bootc.

@cgwalters
Copy link
Contributor Author

This is obsolete.

@cgwalters cgwalters closed this Jan 19, 2024
mvo5 added a commit to mvo5/bootc-image-builder that referenced this pull request Apr 24, 2024
A while ago there was PR#17 that emphasised centos stream9 instead
of fedora:eln. We could not take it back then because the iso
installer was not quite ready for centos and there were hardcoded
EFI names. But now everything is in place so this cherry picks
the original changes from Colin.

See also osbuild#17
github-merge-queue bot pushed a commit that referenced this pull request Apr 24, 2024
A while ago there was PR#17 that emphasised centos stream9 instead
of fedora:eln. We could not take it back then because the iso
installer was not quite ready for centos and there were hardcoded
EFI names. But now everything is in place so this cherry picks
the original changes from Colin.

See also #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.

5 participants