Error on rsync transmission never triggers a fatal message
In the current rsync handler code, we have:
$nice $rsync "${rsync_options[@]}" $filelist_flag $excludes $batch_option $orig $dest_path | tee -a $log
if [ "$?" != "0" ]; then
fatal "Rsync error when trying to transfer $SECTION"
fi
The problem is that due to piping the rsync output to "tee" program the handler will never trigger the fatal message:
When running the following commands we get an exit status 1:
rsync = --non-existing-bla --syntax-error-coming up echo $?
But we get 0 when piping to @tee@:
rsync = --non-existing-bla --syntax-error-coming up | tee test_file echo $?
(from redmine: created on 2012-04-17, closed on 2012-05-15)
Activity
I think we have three possibilities here:
Use the @mispipe@ tool from "moreutils":http://packages.debian.org/source/unstable/moreutils package as suggested "here":http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=654192#128, with the downside of having another dependency for backupninja just used just for a single test.
Dropping the @tee -a $log@.
Using a @set -o pipefail@ bash statament to ensure that a non-zero exit status is always forwarded to @$?@.
I'm more inclined to the third sollution as doesn't break any existing functionality and would not conflict with any changes from current pending patches.
The following example returns @1@ as expected:
#!/bin/bash set -o pipefail rsync = --non-existing-bla --syntax-error-coming up | tee test_file echo $?
So I just did commit "c43bac59":https://git.sarava.org/?p=backupninja.git;a=commit;h=c43bac5911f8d97044b505466f21ed87f43d84fc at "bug/3892":https://git.sarava.org/?p=backupninja.git;a=shortlog;h=refs/heads/bug/3892 branch to address this.
(from redmine: written on 2012-05-10)
rhatto wrote:
So I just did commit "c43bac59":https://git.sarava.org/?p=backupninja.git;a=commit;h=c43bac5911f8d97044b505466f21ed87f43d84fc at "bug/3892":https://git.sarava.org/?p=backupninja.git;a=shortlog;h=refs/heads/bug/3892 branch to address this.
Do you think I should also disable @pipefail@ after issuing the command or I should also check if @pipefail@ is set in beforehand and restore it's default value after I use it? Maybe that's the way to go as all backupninja actions are sourced.
(from redmine: written on 2012-05-10)
rhatto wrote:
Do you think I should also disable @pipefail@ after issuing the command or I should also check if @pipefail@ is set in beforehand and restore it's default value after I use it? Maybe that's the way to go as all backupninja actions are sourced.
Commit "fbf25018":https://git.sarava.org/?p=backupninja.git;a=commit;h=fbf25018bea6b5cb630ee15b401b385d5e9f7084 does that.
(from redmine: written on 2012-05-11)
I agree using pipefail is the best option.
However, I prefer set pipefail / unset pipefail being tightly wrapped around the smallest possible amount of code that actually needs it, rather than globally as your branch does: we need to increase chances, if not to make sure, the initial state is reset if things go wrong somehow.
(from redmine: written on 2012-05-13)
intrigeri wrote:
I agree using pipefail is the best option.
However, I prefer set pipefail / unset pipefail being tightly wrapped around the smallest possible amount of code that actually needs it, rather than globally as your branch does: we need to increase chances, if not to make sure, the initial state is reset if things go wrong somehow.
Commit "118d7587":https://git.sarava.org/?p=backupninja.git;a=commit;h=118d7587b6d4ad34224b7bf7370bdfbe941ffe3c restricts pipefail handling just for the rsync run.
(from redmine: written on 2012-05-13)
In the end, @set_pipefail@ / @restore_pipefail@ look overkill, don't they?
(from redmine: written on 2012-05-13)
intrigeri wrote:
In the end, @set_pipefail@ / @restore_pipefail@ look overkill, don't they?
Yes, but they keep the overall coding style. If you want, I can present an alternative commit where the set/restore pipefail are handled in the main procedure, but it would mix inner details with other generic function calls IMO.
(from redmine: written on 2012-05-14)
rhatto wrote:
Yes, but they keep the overall coding style.
Fair enough. Note that I was not suggesting to move the current pipefail set/restore code to the main procedure, but merely to replace the calls to these functions by @set -o pipefail@ / @set +o pipefail@ and be done with it.
I tried merging your branch, but it conflicts with current master. Please rebase.
(from redmine: written on 2012-05-15)
intrigeri wrote:
rhatto wrote:
Yes, but they keep the overall coding style.
Fair enough. Note that I was not suggesting to move the current pipefail set/restore code to the main procedure, but merely to replace the calls to these functions by @set -o pipefail@ / @set +o pipefail@ and be done with it.
I tried merging your branch, but it conflicts with current master. Please rebase.
I just "updated":https://git.sarava.org/?p=backupninja.git;a=commit;h=5b319008da2a19fe762b699abe4fc8a5c888286e this branch so it can merge with master.
(from redmine: written on 2012-05-15)
Merged, thanks.
(from redmine: written on 2012-05-15)
- micah Status changed to closed
Status changed to closed