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

[13.4-stable] Simplify mkimage-iso-efi entrypoint script and its single consumer in… #4488

Merged

Conversation

andrewd-zededa
Copy link
Contributor

@andrewd-zededa andrewd-zededa commented Dec 13, 2024

… pkg/eve/runme.sh

Backport of #4480

The entrypoint script make-efi in mkimage-iso-efi is used in 2 places:

  • the script makeiso.sh, where it expects to pipe a tar stream to the container's stdin
  • the script runme.sh, as the entrypoint to pkg/eve, i.e. the container image lfedge/eve

This does not change the first use case. The make-efi is changed so that it always reads a tar stream from stdin. This makes it a much simpler program to understand (consistency is nice).

For the case of runme.sh, that script is changed so that instead of assuming it has the installer in a particular directory, it explicitly takes the installer.img, unpackages it into a temporary working directory, and creates a tar stream from the contents of that directory, so that it can stream it to make-efi.

This has the important benefit of fixing the bug wherein making an installer_iso from docker run lfedge/eve installer_iso gives an ISO that does not install correctly.

Signed-off-by: Avi Deitcher avi@deitcher.net
(cherry picked from commit 4df571e)

@OhmSpectator OhmSpectator changed the title Simplify mkimage-iso-efi entrypoint script and its single consumer in… [13.4-stable] Simplify mkimage-iso-efi entrypoint script and its single consumer in… Dec 13, 2024
@OhmSpectator OhmSpectator added the stable Should be backported to stable release(s) label Dec 13, 2024
@OhmSpectator
Copy link
Member

It looks like there are conflicts in pkg/eve/runme.sh Are we sure this should be backported to 13.4 and that the problem fixed wasn’t introduced later in the master branch?

@OhmSpectator
Copy link
Member

Strange, I don't see other changes between 13.4-stables and master in this file...

@eriknordmark
Copy link
Contributor

@andrewd-zededa can you try rebasing it on the upstreams 13.4-stable branch?

… pkg/eve/runme.sh

The entrypoint script make-efi in mkimage-iso-efi is used in 2 places:

* the script makeiso.sh, where it expects to pipe a tar stream to the container's stdin
* the script runme.sh, as the entrypoint to pkg/eve, i.e. the container image lfedge/eve

This does not change the first use case. The make-efi is changed so that it *always* reads a tar stream from stdin.
This makes it a much simpler program to understand (consistency is nice).

For the case of runme.sh, that script is changed so that instead of assuming it has the installer in a particular directory,
it explicitly takes the installer.img, unpackages it into a temporary working directory, and creates a tar stream from the contents of that
directory, so that it can stream it to make-efi.

This has the important benefit of fixing the bug wherein making an installer_iso from `docker run lfedge/eve installer_iso` gives
an ISO that does not install correctly.

Signed-off-by: Avi Deitcher <avi@deitcher.net>
(cherry picked from commit 4df571e)
@eriknordmark
Copy link
Contributor

@andrewd-zededa can you try rebasing it on the upstreams 13.4-stable branch? I can't tell why it shows a conflict for that one line (unless there is some invisible whitespace differerence).

@eriknordmark eriknordmark force-pushed the backport-pr4480-13.4-stable branch from 4e9a3f3 to c6dbbab Compare December 14, 2024 08:47
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

LGTM

@eriknordmark eriknordmark merged commit 5845a4d into lf-edge:13.4-stable Dec 14, 2024
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stable Should be backported to stable release(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants