#1401 PR `merged`: Fix where output should go to the original STDOUT¶

Labels: enhancement, bug, cleanup, fixed / solved / done

jsmeix opened issue at 2017-07-03 12:03:¶

Fix regressions because of redirected STDOUT, see
https://github.com/rear/rear/issues/1398

First step:
Let the help workflow output to the original STDOUT

jsmeix commented at 2017-07-04 14:34:¶

@gdha
restore/NSR/default/400_restore_with_nsr.sh
is initially from you and it contains 'echo' commands
where I do not fully understand how they are meant
so that I like to get feedback from you if my fix in
https://github.com/rear/rear/pull/1401/commits/3aaf8d4595aaa48b89122c0b03fa5a6d8b210555
is right.

jsmeix commented at 2017-07-05 08:55:¶

@gdha
in restore/NSR/default/400_restore_with_nsr.sh
I do not understand how the following code is meant to work:

BLANK=" "
...
    echo -ne "\r${BLANK:1-COLUMNS}\r"

BLANK contains only one single space character
and on a 80 characters wide terminal
1-COLUMNS evaluates to -79 so that
${BLANK:1-COLUMNS}
should be the last 79 characters in BLANK
which cannot work according to what I tested:

BLANK="x"

# echo $COLUMNS
80

# set -x

# echo "${BLANK:1-COLUMNS}" | od -a
+ echo ''
+ od -a
0000000  nl
0000001

From my point of view the whole reformatting of
the 'recover' command output is oversophisticated
and should be dropped to keep ReaR simple and
straightforward like

# Use the original STDIN STDOUT and STDERR when 'rear' was launched by the user for the NSR 'recover' command:
recover -s $(cat $VAR_DIR/recovery/nsr_server) -c $(hostname) -d $TARGET_FS_ROOT -a $(cat $VAR_DIR/recovery/nsr_paths) 0<&6 1>&7 2>&8

I use explicitly also the original STDIN to be future-proof
and prepared if the 'recover' command does a user dialog.

Or is there a undocumented special reason behind
why the 'recover' command output must be reformatted?

I mean:
Why would ReaR need to reformat the 'recover' command output
compared to when the user calls 'recover' manually in a terminal?

Or in other words:
If the 'recover' command output looks bad, it is an issue
in that command and not something that ReaR should
try to "fix".

Furthermore:
There is no error checking of the 'recover' command.
Is it perhaps broken so that it returns zero exit code in any case?
If not the code in ReaR should be simple straightforward
and fail-safe like:

# Use the original STDIN STDOUT and STDERR when 'rear' was launched by the user for the NSR 'recover' command:
if ! recover -s $(cat $VAR_DIR/recovery/nsr_server) -c $(hostname) -d $TARGET_FS_ROOT -a $(cat $VAR_DIR/recovery/nsr_paths) 0<&6 1>&7 2>&8 ; then
    Error "NSR 'recover' command failed"
fi

gdha commented at 2017-07-05 09:19:¶

@jsmeix It is 4 years ago I entered the code, so it is hard to remember why that piece of code was written as that. That being said I have no means to test the code, so whatever we change it could break the current code.

jsmeix commented at 2017-07-05 12:17:¶

@gozora
regarding your restore/BORG/default/300_load_archives.sh

Could you have a look at my
https://github.com/rear/rear/pull/1401/commits/5398f290452134c4f6a65669377fa5896f475d78
whether or not it looks o.k. for you how I have adapted it
so that it should work when STDOUT is redirected.

I think it could be much more simplified if the
UserInput function was fully used but for now
I preferred to do only minimal adaptions to make it work.

By the way I also replaced "while(true)" with "while true"
to avoid a useless subshell.

jsmeix commented at 2017-07-07 11:07:¶

I think I found and fixed (hopefully) almost all places
where 'echo' was used to output on STDOUT.

I would like to merge it now - unless there are
immediate objections.

Of course I will fix issues and remaining things
with output when STDOUT is redirected.

schlomo commented at 2017-07-07 11:26:¶

Thanks a lot for your tireless work on this topic!

[Export of Github issue for rear/rear.]

#1401 PR merged: Fix where output should go to the original STDOUT¶