#1212 PR `merged`: Make UEFI_BOOTLOADER work more generally¶

Labels: enhancement, documentation, fixed / solved / done

jsmeix opened issue at 2017-03-01 12:48:¶

Trying to make how UEFI_BOOTLOADER works
more generally usable as follow-up of
https://github.com/rear/rear/pull/1204

jsmeix commented at 2017-03-01 12:59:¶

@gozora
hereby I did major changes in
rescue/default/850_save_sysfs_uefi_vars.sh
that (hopefully) make UEFI_BOOTLOADER
more generally usable.

I tried to keep all backward compatible
but I really need your careful review.

In particular I searched all ReaR scripts
for what variables are actually globally used
and I did not find that those variables that I
declared now as 'local' to be used elsewhere.

Furthermore I implemented a somewhat sophisticated
way how one same variable UEFI_BOOTLOADER
can be either specified as file to be used as UEFI bootloader
or as single string or array of filename globbing patterns
for 'find' to search for a file to be used as UEFI bootloader.
My implementation is based on the bash behaviour that
normal single string variables and arrays work compatible
in a very well reasonable way:

# var="string"

# echo "$var"
string

# echo "${var[@]}"
string

# var=( 'this' 'that' )

# echo "$var"
this

# echo "${var[@]}"
this that

# var="something"

# echo "$var"
something

# echo "${var[@]}"
something that

gozora commented at 2017-03-01 17:13:¶

Hello @jsmeix,

I'm definitely going to try this out, hope you don't mind a small delay in this review because lately I have lot of (primary) work tasks even during weekend so it might take longer then usual ...

V.

gdha commented at 2017-03-01 19:08:¶

@jsmeix serious change in code - testing will be easier then just looking at the code...I guess

jsmeix commented at 2017-03-02 08:49:¶

@gdha
I cannot test with reasonable effort all those various possible
scenarios (i.e. all those various Linux distributions) where
the right file for UEFI_BOOTLODER could be stored.

jsmeix commented at 2017-03-02 08:52:¶

@gozora
while sleeping over it I noticed that the current logic
is partially backwards - and I think we can get rid
of the BugError - I will do another commit...

gozora commented at 2017-03-02 11:08:¶

@jsmeix thanks for notification!
I was planing to take a look on it today afternoon/evening ...

V.

jsmeix commented at 2017-03-02 15:26:¶

@gozora
I think now you could have a look.

Unfortunately it became somewhat nested conditions but
currently I have no good idea how to make it less nested
(without using things like artificial helper variables).

jsmeix commented at 2017-03-02 15:32:¶

Whoops!
I had the wrong assumption that any UEFI related code
belongs to @gozora until now when I used

git log -p --follow usr/share/rear/rescue/default/850_save_sysfs_uefi_vars.sh

that shows me that @gdha had initially made it
so that hereby I humbly also request a review
from @gdha

jsmeix commented at 2017-03-02 16:14:¶

Caution:
In general

is_true $VAR

is not the logical opposite (i.e. the logical negation) of

is_false $VAR

see my explanation in usr/share/rear/lib/global-functions.sh

# two explicit functions to be able to test explicitly for true and false (see issue #625)
# because "tertium non datur" (cf. https://en.wikipedia.org/wiki/Law_of_excluded_middle)
# does not hold for variables because variables could be unset or have empty value
...
    # only if there is explicitly a 'true' value then is_true returns true
    # so that an unset variable or an empty value is not true
...
    # only if there is explicitly a 'false' value then is_false returns true
    # so that an unset variable or an empty value is not false
    # caution: for unset or empty variables is_false is false

Accordingly

is_true $USING_UEFI_BOOTLOADER || return

returns
if USING_UEFI_BOOTLOADER is explicitly a 'false' value
and also when USING_UEFI_BOOTLOADER is empty or unset
while in contrast

is_false $USING_UEFI_BOOTLOADER && return

returns
only if USING_UEFI_BOOTLOADER is explicitly a 'false' value
but not when USING_UEFI_BOOTLOADER is empty or unset.

jsmeix commented at 2017-03-03 12:16:¶

Basically directly after leaving the office yesterday
I remembered what I already did to avoid
code with deeply nested 'if...else' conditions:
I use an artificial 'for' clause that is run only once
so that I can at any point 'continue' with the code
after that 'for' clause, cf.
https://github.com/rear/rear/blob/master/usr/share/rear/restore/default/990_move_away_restored_files.sh

Since my above
https://github.com/rear/rear/pull/1212/commits/9a31a5fa9a75a72bfb01eedabadc104400982667
I even like to look at my code ;-)

@gozora
many thanks for your tests!

I think I will merge it soon - unless there are objections.

@gdha
of course when there are bugs, I will fix them.

[Export of Github issue for rear/rear.]

#1212 PR merged: Make UEFI_BOOTLOADER work more generally¶