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

630_run_efibootmgr.sh wrongly checks boot loader outside chroot #2051

Closed
wants to merge 1 commit into from
Closed

630_run_efibootmgr.sh wrongly checks boot loader outside chroot #2051

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion usr/share/rear/finalize/Linux-i386/630_run_efibootmgr.sh
View file
Open in desktop
Expand Up @@ -9,7 +9,7 @@ is_true $EFI_STUB && return 0

# UEFI_BOOTLOADER empty or not a regular file means using BIOS cf. rescue/default/850_save_sysfs_uefi_vars.sh
# Double quotes are mandatory here because 'test -f' without any (possibly empty) argument results true:
test -f "$UEFI_BOOTLOADER" || return 0
test -f "$TARGET_FS_ROOT/$UEFI_BOOTLOADER" || return 0
Copy link
Member

Choose a reason for hiding this comment

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

This code does the right thing but in a subtle way
because when UEFI_BOOTLOADER is empty (which means BIOS)

test -f "$TARGET_FS_ROOT/$UEFI_BOOTLOADER"

evaluates to

test -f "/mnt/local/"

which is not true because /mnt/local/ is a directory.

Copy link
Member

Choose a reason for hiding this comment

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

That's why explanatory comments help so much ;-)

I think they help not only others to understand how it is meant to work.
I think they help (perhaps even more) the one who makes the code
to get things sorted out in his mind.

At least this is what happpens to me when I make code
and then try to explain what I did.
When it gets hard for me to explain my own code I usually got
something wrong or at least I can make my code better
until I can well explain what my code does.
This is the reason behind why I make so much comments.


# Determine where the EFI System Partition (ESP) is mounted in the currently running recovery system:
esp_mountpoint=$( df -P "$TARGET_FS_ROOT/$UEFI_BOOTLOADER" | tail -1 | awk '{print $6}' )
Copy link
Member

Choose a reason for hiding this comment

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

It becomes clear that

test -f "$TARGET_FS_ROOT/$UEFI_BOOTLOADER"

must be used when one looks at the code how esp_mountpoint is set
but of course I did not do that when I did my "Blind attempt" :-(

Copy link
Member Author

Choose a reason for hiding this comment

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

I did not do that when I did my "Blind attempt" :-(

Don't so strict to your self, it was "Blind attempt" after all ...

V.

Expand Down