release/amd64: cleanup code duplication #1571
Conversation
There was a problem hiding this comment.
I think checking for the existence of the file is fine vs checking MK_LOADER_IA32. That said, I think the cleanup to use a helper extra_args variable to reduce code duplication is useful in its own right (and probably worthy of being its own commit). I do think bootx64 belongs in extra_args though.
| else | ||
| make_esp_file ${espfilename} ${fat32min} ${BASEBITSDIR}/boot/loader.efi | ||
| if [ ${MK_LOADER_IA32} = "yes" ]; then | ||
| extra_args="${BASEBITSDIR}/boot/loader_ia32.efi bootia32" |
There was a problem hiding this comment.
Should "bootx64" be in the extra_args case here? It's not present in the existing code for the !LOADER_IA32 case.
There was a problem hiding this comment.
It shouldn't matter. If make_esp_file isn't given the "bootx64", it automatically determines it using uname -m.
I just thought this would make more sense since the arguments are given in pairs, and the condition was for the ia32 pair.
Signed-off-by: Ahmad Khalifa <[email protected]>
VexedUXR
force-pushed
the
ImproveRelease
branch
from
da3e113 to
8d7d3f1
Compare
|
Ok, went back to simply checking if the loader files exist. :) |
VexedUXR
changed the title
Improve the release script by checking
MK_LOADER_IA32instead of the jankyif -f "${BASEBITSDIR}/boot/loader_ia32.efi"I'm hesitant about putting this in install-boot.sh, it feels out of place.
Maybe a new script in release/amd64 that gets sourced by both
make-memstickandmkisoimageswould be better? I'm not sure..This also makes the script complain if
MK_LOADER_IA32isyesbut the file is missing, instead of silently not copying it.FWIW it also allows the user to build the ia32 loader, but omit it from the img, though I'm not sure what the use case would be.
I've been meaning to do this since 599273f landed but forgot, so here it is.