#2461 Issue closed: How to deal with 'cd /some/dir' in ReaR scripts?

Labels: cleanup, fixed / solved / done

jsmeix opened issue at 2020-07-22 12:09:

In general cd /some/dir in a ReaR script is problematic.
Usually it should be pushd /some/dir with matching popd.

At some places we have code like

( cd /some/dir
  some commands
)

which uses a subshell which has the disadvantage that
global variables cannot be modified inside the subshell
(they can be modified but that has no result outside the subshell).

I think normally code like

pushd /some/dir
some commands
popd

looks somehow "better" to me in ReaR scripts
but here the drawback is that code like

pushd /some/dir
...
if ! command ; then
    LogPrint "command failed"
    return 1
fi
...
popd

would return from the script without matching popd
so one must carefully have popd at all 'side exits' like

pushd /some/dir
...
if ! command ; then
    LogPrint "command failed"
    popd
    return 1
fi
...
popd

I wonder if we should perhaps do in the Source function
something like

function Source () {
    local current_dir=$( pwd )
    ...
    source "$source_file"
    source_return_code=$?
    cd $current_dir

to ensure that after each script
we are back in ReaR's usual working directory?

Cf. https://github.com/rear/rear/pull/2445#discussion_r458071630

jsmeix commented at 2020-08-03 14:41:

Because both "cd in subshell" and "pushd / popd" have pros and cons
so there is no "one best way" how to change the working directory in scripts
and because the only thing that actually matters is that all scripts
get launched with one same predictable working directory
I think meanwhile that
each script can implement changing its working directory as it fits best for the particular script
and I will only implement to ensure all scripts get launched with one same working directory.

jsmeix commented at 2020-08-13 11:46:

ReaR does not run with some kind of static working directory
but it runs with a predictable working directory which is
the current working directory when rear is launched.

For a test I added a LogPrint to the Source() function in lib/framework-functions.sh

function Source () {
    local source_file="$1"
    LogPrint "Sourcing $source_file with current working directory $( pwd )"

Then I did

# cd /other/mydir

linux-h9wr:/other/mydir # /root/rear.github.master/usr/sbin/rear -D mkbackup
Relax-and-Recover 2.6 / Git
Running rear mkbackup (PID 6960 date 2020-08-13 13:11:40)
Using log file: /root/rear.github.master/var/log/rear/rear-linux-h9wr.log
Sourcing /root/rear.github.master/etc/rear/os.conf with current working directory /other/mydir
Sourcing /root/rear.github.master/usr/share/rear/conf/Linux-i386.conf with current working directory /other/mydir
Sourcing /root/rear.github.master/usr/share/rear/conf/GNU/Linux.conf with current working directory /other/mydir
Sourcing /root/rear.github.master/usr/share/rear/conf/SUSE_LINUX.conf with current working directory /other/mydir
Sourcing /root/rear.github.master/etc/rear/local.conf with current working directory /other/mydir
Sourcing /root/rear.github.master/usr/share/rear/init/default/005_verify_os_conf.sh with current working directory /other/mydir
...
Sourcing /root/rear.github.master/usr/share/rear/backup/NETFS/default/980_umount_NETFS_dir.sh with current working directory /other/mydir
Sourcing /root/rear.github.master/usr/share/rear/backup/default/990_post_backup_script.sh with current working directory /other/mydir
Exiting rear mkbackup (PID 6960) and its descendant processes ...
Running exit tasks
You should also rm -Rf /tmp/rear.ly800lywpxoEj6W

linux-h9wr:/other/mydir # ls
[no output]

I verified in the log file that all what was sourced by the Source() function
was sourced with current working directory /other/mydir
and no file is written to the current working directory /other/mydir
so at least the scripts for "rear mkbackup" with BACKUP=NETFS
seem to work properly.

I think I will add to usr/sbin/rear something like

readonly WORKING_DIR="$( pwd )"

and add to the Source() function something like

function Source () {
    ...
    source "$source_file"
    ...
    cd "$WORKING_DIR" || LogPrintError "Failed to 'cd $WORKING_DIR'"

to ensure that after each sourced file we are back in the usual working directory
cf. https://github.com/rear/rear/pull/2478/commits/5fa006ef842e93eed5daeeae58955116d7980888
because the first source "$source_file" is launched from the usual working directory anyway
and the intent behind this issue is to make things safe against if the working directory
was permanently changed within a script or code in a config file.

jsmeix commented at 2020-08-14 12:08:

With https://github.com/rear/rear/pull/2478 merged this issue is done.


[Export of Github issue for rear/rear.]