#3436 PR `closed`: Fix for simulation mode in sbin/rear¶

Labels: bug, won't fix / can't fix / obsolete

jsmeix opened issue at 2025-03-24 09:28:¶

Type: Bug Fix
Impact: Normal
Reference to related issue (URL):

https://github.com/rear/rear/pull/3424#issuecomment-2747079736

How was this pull request tested?

With the change 'rear -s ...' works again for me.

Description of the changes in this pull request:

In sbin/rear in simulation mode the workflow scripts
$SHARE_DIR/lib/[a-z]*-workflow.sh need to be executed (sourced)
to get the WORKFLOW_... functions defined,
otherwise sbin/rear could not run the requested workflow.

In simulation mode a WORKFLOW_... function is run
but its called scripts are not executed by the Source function.

pcahyna commented at 2025-03-24 10:49:¶

Thanks for the prompt fix, @jsmeix, but wouldn't it be better to revert to use source here unconditionally, instead of complicating the code? It looks like Source and source have a bit different semantics (the former skips the actual sourcing in simulation mode) and in the case of loading library function we actually want the semantics of the latter instead of the former (the former looks better suited for actual code execution, i.e. executing scripts, not loading function definitions).

Note that the commit message of 7df7dc6499b7703b98d7293fbbf5de4d514a29ac says:

... we can use Source to source our normal scripts
in usr/share/rear/lib

but it does not say why should we use it (merely that we can use it).

jsmeix commented at 2025-03-24 13:52:¶

Both 'source' and 'Source' seem to work in practice.

'source' is proven to work in practice because we used that
all the time at that specific code place only in sbin/rear
until 2 weeks ago when I did
https://github.com/rear/rear/commit/7df7dc6499b7703b98d7293fbbf5de4d514a29ac

On the other hand 'Source' is explicitly meant
to source ReaR scripts and ReaR config files
(it provides additional functionality and safeguards)
and this is also proven to work in practice because
we use that all the time at several code places.

I think 'source' was used at that code place in sbin/rear
because before https://github.com/rear/rear/pull/3424
the 'Source' function was not yet available
at that code place because the 'Source' function
was defined in /usr/share/rear/lib/framework-functions.sh
so I think plain 'source' was used at that code place
basically as some kind of "makeshift" because
the actually right function to source ReaR scripts
and ReaR config files was just not yet available.
But I cannot prove why 'source' was used at that code
place in sbin/rear without a comment at that code
which explains the WHY.

With my more complicated code here I intend to use
the 'Source' function for what I think it is meant
to be used in normal use cases so in the special use case
of the simulation mode I need additional exceptional code.

Compare how much code was executed in simulation mode before
with how much code is executed in simulation mode now with
the changes in this pull request - I mean when we call
that stuff "simulation mode" we should avoid executing
code which is not needed for the simulation.

By the way:
I do not enjoy those endless special case code
and exceptional stuff handling and whatnot.
I know "good old times" when computing had been much simpler
with much less features and all was much less user friendly ;-)
What really annoys me is that rather often underlying foundations
behave unpredictably, unreliably, insecure, crude, rude,...
so higher level programmers must endlessly implement
tons of additional rather complicated makeshift code
to somehow mitigate shortcomings in underlying foundations.

pcahyna commented at 2025-03-24 15:49:¶

I do not enjoy those endless special case code
and exceptional stuff handling and whatnot.
I know "good old times" when computing had been much simpler
with much less features and all was much less user friendly ;-)
What really annoys me is that rather often underlying foundations
behave unpredictably, unreliably, insecure, crude, rude,...
so higher level programmers must endlessly implement
tons of additional rather complicated makeshift code
to somehow mitigate shortcomings in underlying foundations.

In my experience the "complicated makeshift code" and shortcomings in underlying foundations is often not the fault of the code, but of the specification - i.e. the problem has not been introduced at the coding phase but at the design phase - it was not well specified what the code actually should do and thus how one should implement it and what other code can expect from it.

Consider this case: to me it is not exactly clear what is the intended difference of Source vs. source (your wrapper introduced in ReaR). You write that Source provides additional functionality and safeguards, but safeguards are introduced in your source wrapper already. Regarding the additional functionality, it needs to be specified for which use it is intended (e.g. because in this case the functionality is indeed the problem). You write that

'Source' is explicitly meant to source ReaR scripts and ReaR config files"

and that you think

plain 'source' was used at that code place basically as some kind of "makeshift" because the actually right function to source ReaR scripts and ReaR config files was just not yet available.

I am not sure that this was the original intent though. To me it seems rather that Source has had the more restricted purpose of sourcing the scripts in the scripting framework stages (fromSourceStage), so reading libraries and configuration files would not be included and this would explain why to use source here.

Have a look what Source is doing:

    # Simulate sourcing the scripts in $SHARE_DIR
    if test "$SIMULATE" && expr "$source_file" : "$SHARE_DIR" >/dev/null; then
        LogPrint "Source $relname"
        return
    fi

Is this intended for reading configuration files? What about configuration files under usr/share/rear/conf ? To me it looks like they will get skipped - thus simulation mode will work without default values of variables defined - is that intended? I have never used simulation mode, but to me it looks like the steps in the simulation could be quite different from the actual run, which defeats the purpose of the simulation.

you write:

But I cannot prove why 'source' was used at that code
place in sbin/rear without a comment at that code
which explains the WHY.

and I agree, I think we should take this (otherwise quite trivial) PR as an opportunity to better specify the intended use of the source wrapper vs. Source.

P.S. I am of course also guilty here - I should have noticed the shift in meaning of Source in the review of the PR where you made the changes.

jsmeix commented at 2025-03-24 16:15:¶

My main point why I am thinking that 'Source' is explicitly
meant to source ReaR scripts and ReaR config files is that
we use 'Source' in sbin/rear to source ReaR config files
and in 'SourceStage' to source ReaR scripts so I assumed
that also our usr/share/rear/lib/*.sh scripts are meant
to be sourced via 'Source'?

jsmeix commented at 2025-03-25 13:38:¶

This pull request cannot proceed as intended
because its root cause(s) are way off-topic,
cf. https://github.com/rear/rear/issues/3438
which is a bigger task for the future as time permits.

What I will do to fix things for our current simulation mode
is to simply revert my
https://github.com/rear/rear/commit/7df7dc6499b7703b98d7293fbbf5de4d514a29ac
to just make things working again as they did before.

jsmeix commented at 2025-03-25 13:51:¶

Done via
https://github.com/rear/rear/commit/1128f1b616b370e49ac7be7f0ae0fa5ab04bc638

jsmeix commented at 2025-03-25 13:58:¶

This pull request also
revealed that currently it is not clear
when 'source' versus 'Source' should be used in ReaR
so that area needs to be sorted out and cleaned up.
This is a bigger task for the future as time permits via
https://github.com/rear/rear/issues/3439

pcahyna commented at 2025-03-25 16:33:¶

My main point why I am thinking that 'Source' is explicitly
meant to source ReaR scripts and ReaR config files is that
we use 'Source' in sbin/rear to source ReaR config files

My bad - I thought that using Source for ReaR config files is a recent change, but in fact it has been like this since the beginning. I also thought that Source is used for usr/share/rear/conf/default.conf, but it is not (and this is an irregularity, but it prevents what I wrote:

What about configuration files under usr/share/rear/conf ? To me it looks like they will get skipped - thus simulation mode will work without default values of variables defined - is that intended?

)
https://github.com/rear/rear/blob/a61075364b8f0d92c62d63be71467a31e0d4a39d/usr/sbin/rear#L414

The other platform-specific configuration defaults are read using Source though:
https://github.com/rear/rear/blob/a61075364b8f0d92c62d63be71467a31e0d4a39d/usr/sbin/rear#L798
so simulation mode will work without values defined there. We agreed that this is a bug.

Therefore, I would not claim anymore that Source is not intended for reading library files, as it is indeed intended for configuration files (so not just for code snippets under stages). For library files we can use it or avoid it as we decide.

jsmeix commented at 2025-03-26 12:25:¶

We cannot use 'Source' in sbin/rear to source default.conf
because we need to source default.conf before we source
_framework-setup-and-functions.sh where 'Source' gets defind.
The comments in sbin/rear hopefully explain why we do it this way.

jsmeix commented at 2025-03-26 12:40:¶

In general regarding 'Source' versus 'source' in simulation mode:

See my above
https://github.com/rear/rear/pull/3436#issuecomment-2748210435
excerpt

... I mean when we call
that stuff "simulation mode" we should avoid executing
code which is not needed for the simulation

In contrast to default.conf that is not meant to be
modified by the user, the user config files are meant
for the user to also (as needed) execute what he likes, see
https://github.com/rear/rear/blob/rear-2.9/etc/rear/local.conf

# Because 'source' executes the content as a bash script you can run commands
# within your configuration files, in particular commands to set different
# configuration values depending on certain conditions as you need like
# CONDITION_COMMAND && VARIABLE="special_value" || VARIABLE="usual_value"
# but that means such commands get always executed when 'rear' is run.
# You must ensure commands in configuration files work always without errors
# regardless in which environment your commands will be run, in particular
# on your original system (i.e. during "rear mkrescue" or "rear mkbackup")
# and also within the ReaR recovery system (i.e. during "rear recover").

By pure luck my wording
"such commands get always executed when 'rear' is run"
includes also the simulation mode so we are safe
from a technical point of view
"dear user, you have been told so"
but on the other hand my wording
"You must ensure commands in configuration files
work always without errors"
is not yet sufficiently safe so it shoud better read
something like
"You must ensure commands in configuration files
work always without errors or side effects"
because a user is likely not so happy when things
get modified in his system in simulation mode because
his commands were also executed in simulation mode.

[Export of Github issue for rear/rear.]

#3436 PR closed: Fix for simulation mode in sbin/rear¶