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.

Sign up for GitHub

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

Jump to bottom

Add script for building ipxe #351

Merged
merged 1 commit into from
Nov 7, 2024
Merged

Add script for building ipxe #351

merged 1 commit into from
Nov 7, 2024

Conversation

asdil12
Copy link
Member

@asdil12 asdil12 commented Nov 6, 2024

r-richardson
r-richardson reviewed Nov 6, 2024
View reviewed changes
ipxe/build.sh Outdated Show resolved Hide resolved
Martchus
Martchus reviewed Nov 6, 2024
View reviewed changes
Copy link
Contributor

@Martchus Martchus left a comment

Choose a reason for hiding this comment

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

It is considered a bad practice to conduct a build as root but as part of this script it would have to be conducted as root (otherwise the rest of the script doesn't work). Maybe it makes more sense to use sodo within the script as needed.

Martchus
Martchus reviewed Nov 7, 2024
View reviewed changes
ipxe/build.sh
#make -j3 EMBED=../../boot.ipxe CROSS=aarch64-suse-linux- bin-arm64-efi/ipxe.efi

# undionly / snponly uses the network driver of the PXE / UEFI stack
make -j3 EMBED=../../boot.ipxe bin/undionly.kpxe bin-x86_64-efi/snponly.efi
Copy link
Contributor

@Martchus Martchus Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Is the build system broken or why are you limiting this to 3 cores? If it is broken it would make more sense to limit it to 1 core. Otherwise I'd leave it out and not override MAKEFLAGS.

Copy link
Member Author

@asdil12 asdil12 Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

the netboot vm only has 2 cores

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, so this is tailored to run in a specific VM.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is here to also serve as a documentation / quick way to setup ipxe or change the embedded boot.ipxe script.

Martchus
Martchus approved these changes Nov 7, 2024
View reviewed changes
Copy link
Contributor

@Martchus Martchus left a comment

Choose a reason for hiding this comment

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

Despite the issue mentioned in the in-line comment good enough to merge.

@mergify mergify bot merged commit 867408e into os-autoinst:master Nov 7, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@r-richardson r-richardson r-richardson left review comments

@Martchus Martchus Martchus approved these changes

@perlpunk perlpunk perlpunk approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@asdil12 @perlpunk @Martchus @r-richardson