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.
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
Possible regressions because of redirected STDOUT #1398
Comments
So did you check the use of an editor / viewer in the recovery workflow when the disks are different? IMHO we should fix at least that part. |
The calls of 'less' and 'vi' in select choice in "${choices[@]}"; do case "$REPLY" in (1) less $RUNTIME_LOGFILE;; (2) less $VAR_DIR/layout/config/df.txt;; (3) rear_shell;; (4) vi $LAYOUT_FILE;; (5) vi $LAYOUT_CODE;; esac done 0<&6 1>&7 2>&8 The 'vi' calls in function rear_shell () { ... HISTFILE="$histfile" bash --noprofile --rcfile $bashrc 0<&6 1>&7 2>&8 } I had adapted the 'read' and 'select' calls and the rear_shell But I cannot know what other arbitrary interactive tools |
So again, should we revert this or not? How could we decide this question? |
Just test it out and lets make a decision afterwards
Gratien
… Op 28 jun. 2017 om 21:31 heeft Schlomo Schapiro ***@***.***> het volgende geschreven:
So again, should we revert this or not? How could we decide this question?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
@schlomo - exec 1>&2 + # exec 1>&2 to get back the old behaviour without redirected STDOUT. @gdha |
During #1391 Now I explicitly tested the latter and it works (as expected). FYI how I tested it: At the beginning of init/default/030_update_recovery_system.sh UserOutput "user output" LogUserOutput "log user output" LogPrint "test stdout and stderr output" echo "stdout output" echo "stderr output" 1>&2 LogPrint "test command stdout and stderr output" cat /etc/issue qqq LogPrint "test read" read -p "Type some user input: " 0<&6 1>&7 2>&8 LogPrint "user input is '$REPLY'" LogPrint "test select" test_file="/tmp/rear_test_file" rear_shell_history="echo foo echo bar echo baz" choices=( "View Relax-and-Recover log" "Go to Relax-and-Recover shell" "Edit $test_file" "View $test_file" "Exit select" ) # Use the original STDIN STDOUT and STDERR when rear was launched by the user # to get input from the user and to show output to the user (cf. _input-output-functions.sh): select choice in "${choices[@]}"; do case "$REPLY" in (1) less "$RUNTIME_LOGFILE";; (2) rear_shell "" "$rear_shell_history";; (3) vi "$test_file";; (4) less "$test_file";; (5) break;; esac LogPrint "User selected: $REPLY) ${choices[$REPLY-1]}" # Reprint menu options when returning from less, shell or vi Print "" for (( i=1; i <= ${#choices[@]}; i++ )); do Print "$i) ${choices[$i-1]}" done done 0<&6 1>&7 2>&8 For me it works as expected. To test if it also works with redirected STDIN (to be future-proof) # Discard STDIN as a test: exec 0</dev/null For me it still works as expected. I did a "rear mkbackup" and then |
Like this: $ sudo strace -qq -f -e execve rear mkrescue 2>&1 | sed -ne 's#.*execve("\([^"]*\).*$#\1#p' | sort -u
/bin/btrfs
/bin/cat
/bin/chmod
/bin/chown
/bin/cp
/bin/cpio
/bin/date
/bin/dd
/bin/df
/bin/efibootmgr
/bin/egrep
/bin/findmnt
/bin/grep
/bin/gunzip
/bin/gzip
/bin/hostname
/bin/ln
/bin/ls
/bin/mkdir
/bin/mktemp
/bin/mount
/bin/mv
/bin/pidof
/bin/ps
/bin/readlink
/bin/rm
/bin/sed
/bin/stty
/bin/systemd-notify
/bin/tar
/bin/umount
/bin/uname
/lib64/ld-linux-x86-64.so.2
/sbin/blkid
/sbin/cat
/sbin/depmod
/sbin/dmsetup
/sbin/ethtool
/sbin/grep
/sbin/gzip
/sbin/ip
/sbin/lsmod
/sbin/mkfs.vfat
/sbin/modinfo
/sbin/modprobe
/sbin/parted
/sbin/udevadm
/usr/bin/awk
/usr/bin/basename
/usr/bin/blkid
/usr/bin/cat
/usr/bin/cut
/usr/bin/dig
/usr/bin/dirname
/usr/bin/du
/usr/bin/dumpkeys
/usr/bin/file
/usr/bin/find
/usr/bin/getent
/usr/bin/getopt
/usr/bin/grep
/usr/bin/grub-mkimage
/usr/bin/gzip
/usr/bin/head
/usr/bin/hexdump
/usr/bin/id
/usr/bin/ldd
/usr/bin/logger
/usr/bin/lsattr
/usr/bin/lsb_release
/usr/bin/md5sum
/usr/bin/pgrep
/usr/bin/seq
/usr/bin/sort
/usr/bin/stat
/usr/bin/strings
/usr/bin/syslinux
/usr/bin/tac
/usr/bin/tail
/usr/bin/tee
/usr/bin/touch
/usr/bin/tr
/usr/bin/wc
/usr/bin/xargs
/usr/bin/xorrisofs
/usr/local/bin/bash
/usr/local/bin/blkid
/usr/local/bin/cat
/usr/local/bin/grep
/usr/local/bin/gzip
/usr/local/sbin/bash
/usr/local/sbin/blkid
/usr/local/sbin/cat
/usr/local/sbin/grep
/usr/local/sbin/gzip
/usr/sbin/bash
/usr/sbin/blkid
/usr/sbin/cat
/usr/sbin/chroot
/usr/sbin/grep
/usr/sbin/grub-probe
/usr/sbin/gzip
/usr/sbin/rear If you want to check an interactive strace -qq -f -o log.txt -e execve rear recover
sed -ne 's#.*execve("\([^"]*\).*$#\1#p' <log.txt | sort -u | tee >/mnt/local/root/rear-progs.txt |
@schlomo Perhaps I can use it for another task but for what I would like But what I would like to find out is what interactive stuff if CONDITION ; then echo "Please confirm removal of $some_file" rm -i $some_file fi When CONDITION is not true when I run it in my environment Simply put: |
I think static code analysis for bash is really tricky.
Maybe we can actually use this last part to automatically populate the HTH, |
Things are somewhat worse than expected. I will fix as many regressions as I can If current redirected STDOUT is too bad for normal usage # Make stdout the same what stderr already is. # This keeps strict ordering of stdout and stderr outputs # because now both stdout and stderr use one same file descriptor: -exec 1>&2 +test "$REAR_STDOUT_INTO_LOG" && exec 1>&2 Now one can call REAR_STDOUT_INTO_LOG=y usr/sbin/rear ... to run it with redirected STDOUT for testing |
@schabrolles restore/TSM/default/400_restore_with_tsm.sh Could you please have a look? |
Most of the I don't think we need to copy this in a log file ... May be only the result (number of files restored) |
@schabrolles I like it so much to have functions that are named # type -a star star is /usr/bin/star # man star ... DESCRIPTION Star is a very fast tar(1) like tape archiver with improved functionality. ;-) You may have a look at my changed |
@schabrolles we got rid of the progress indicator (spinner) in ReaR, is your code doing something like that? |
I wonder in general to what extent an animated cursor # for each animated_cursor function call # show one of the characters / - \ | function animated_cursor () { ... } long_running_command \ | while read long_running_command_stdout ; do echo $long_running_command_stdout animated_cursor done When long_running_command outputs there is no real need I think a more useful implementation might be like long_running_command_output="" long_running_command 2>&1 \ | while read -t 1 long_running_command_output || TODO ; do if test "$long_running_command_output" ; then echo $long_running_command_output long_running_command_output="" else animated_cursor fi done The problem is that read returns non-zero exit code when it |
The only thing that works for me is this way long_running_command_output="" { echo foo ; sleep 5 ; cat /etc/issue ; sleep 5 ; cat qqq ; sleep 5 ; echo FINISHED ; } 2>&1 \ | while read -t 1 long_running_command_output || true ; do if test "$long_running_command_output" ; then test "FINISHED" = "$long_running_command_output" && break echo $long_running_command_output long_running_command_output="" else animated_cursor fi done 1>&7 via an artificial special final "FINISHED" string |
@jsmeix @schabrolles @schlomo I would prefer to get rid with the animation - any objection? |
That's what I meant. |
I also would prefer to get rid of the animated cursor, In general I think ReaR does not need to provide |
…ant_for_the_user_issue_1398 Fix where output should go to the original STDOUT (mainly when plain 'echo' was used for output) to avoid regressions because of redirected STDOUT see #1398 What is not yet fixed are arbitrary program calls like "less some_file" or "vi another_file".
With #1401 merged |
@gdha @schlomo @gdha - exec 1>&2 + # exec 1>&2 Alternativerly we may have STDOUT redirected by default What do you think what is best for ReaR v 2.2? For ReaR v 2.3 that is currently dated end of October 2017 |
@jsmeix @schlomo The purpose of ReaR v2.2 is to have an intermediate package on request of a customer (where Schlomo did some updates for [which are not yet in the master]). The question of STDOUT redirects by default? If you do not mind to post-pone to the 2.3 release then I would go for that track? |
+1
…On 14 July 2017 at 11:46, gdha ***@***.***> wrote:
@jsmeix <https://github.com/jsmeix> @schlomo <https://github.com/schlomo>
The purpose of ReaR v2.2 is to have an intermediate package on request of a
customer (where Schlomo did some updates for [which are not yet in the
master]). The question of STDOUT redirects by default? If you do not mind
to post-pone to the 2.3 release then I would go for that track?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1398 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGMCLroBxhumcPtS5tAqWlhr_43l4Jsks5sNzjwgaJpZM4OICrD>
.
|
The redirection of stdout into the log file is postponed for ReaR v2.2 see #1398 (comment)
With |
Sufficiently done for ReaR 2.3. Remaining things can be fixed for ReaR 2.4 or later |
Since #1391 is merged
therein in particular via
523b956
both STDOUT and STDERR are redirected to the log file.
The redirection of STDOUT causes regressions for things
that are intended to run in an interactive way with a user.
Any code that "just calls" interactively working tools like
will appear as if ReaR had hang up because the user
does not see any STDOUT output on his terminal
unless proper specification to use the the original STDIN
STDOUT and STDERR when rear was launched by the user
as follows:
cf. "What to do with stdout and stderr" in
https://github.com/rear/rear/wiki/Coding-Style
Because I @jsmeix cannot check the whole code
I assume there are some places where interactive tools
are "just called" that need to be fixed.
The text was updated successfully, but these errors were encountered: