Skip to content
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

Incompatibility with rsync 3.2.4 #1247

Closed
kattjevfel opened this issue Apr 18, 2022 · 22 comments · Fixed by #1351
Closed

Incompatibility with rsync 3.2.4 #1247

kattjevfel opened this issue Apr 18, 2022 · 22 comments · Fixed by #1351

Comments

@kattjevfel
Copy link

After upgrading to rsync 3.2.4, I get the following errors:

INFO: Call rsync to take the snapshot
notify-send Back In Time (katt) : Main profile Error: rsync: [Receiver] mkdir "/home/katt/"/mnt/backups/katt/backintime/main/katt/1/new_snapshot/backup"" failed: No such file or directory (2)
WARNING: Command "rsync --recursive --times --devices --specials --hard-links --human-readable --links --perms --executability --group --owner --info=progress2 --no-inc-recursive --rsh=ssh -o ServerAliveInterval=240 -o LogLevel=Error -o IdentityFile=/home/katt/.ssh/id_rsa -p 22 --delete --delete-excluded -v -i --out-format=BACKINTIME: %i %n%L --link-dest=../../20220417-000001-246/backup --chmod=Du+wx --exclude=/home/katt/.local/share/backintime/mnt/1_145168 --exclude=/home/katt/.local/share/backintime --exclude=.local/share/backintime/mnt --include=/home/katt/ --include=/home/ --exclude=.gvfs --exclude=.thumbnails* --exclude=.local/share/[Tt]rash* --exclude=*.backup* --exclude=*~ --exclude=.Private --exclude=steamapps/common --exclude=*[Cc]ache* --exclude=/home/katt/build --exclude=/home/katt/Games --exclude=home/katt/.local/share/sddm/xorg-session.log --exclude=/home/katt/.local/var/pmbootstrap --include=/home/katt/** --exclude=* / katt@lewd.se:"/mnt/backups/katt/backintime/main/katt/1/new_snapshot/backup"" returns 11
WARNING: Command "rsync -a --delete --rsh=ssh -o ServerAliveInterval=240 -o LogLevel=Error -o IdentityFile=/home/katt/.ssh/id_rsa -p 22 /tmp/tmpne0f0gdn/ katt@lewd.se:"/mnt/backups/katt/backintime/main/katt/1/new_snapshot"" returns 3 | rsync: [Receiver] change_dir#3 "/home/katt/"/mnt/backups/katt/backintime/main/katt/1" failed: No such file or directory (2)
rsync error: errors selecting input/output files, dirs (code 3) at main.c(822) [Receiver=v3.2.3]
INFO: Nothing changed, no new snapshot necessary
WARNING: Command "rsync -a --delete --rsh=ssh -o ServerAliveInterval=240 -o LogLevel=Error -o IdentityFile=/home/katt/.ssh/id_rsa -p 22 /tmp/tmp44z9tcf9/ katt@lewd.se:"/mnt/backups/katt/backintime/main/katt/1/20220418-173836-144"" returns 3 | rsync: [Receiver] change_dir#3 "/home/katt/"/mnt/backups/katt/backintime/main/katt/1" failed: No such file or directory (2)
rsync error: errors selecting input/output files, dirs (code 3) at main.c(822) [Receiver=v3.2.3]
Traceback (most recent call last):
  File "/usr/share/backintime/common/backintime.py", line 1165, in <module>
    startApp()
  File "/usr/share/backintime/common/backintime.py", line 517, in startApp
    args.func(args)
  File "/usr/share/backintime/common/backintime.py", line 739, in backup
    ret = takeSnapshot(cfg, force)
  File "/usr/share/backintime/common/backintime.py", line 94, in takeSnapshot
    ret = snapshots.Snapshots(cfg).backup(force)
  File "/usr/share/backintime/common/snapshots.py", line 701, in backup
    self.remove(sid)
  File "/usr/share/backintime/common/snapshots.py", line 583, in remove
    shutil.rmtree(sid.path())
  File "/usr/lib/python3.10/shutil.py", line 712, in rmtree
    onerror(os.lstat, path, sys.exc_info())
  File "/usr/lib/python3.10/shutil.py", line 710, in rmtree
    orig_st = os.lstat(path)
FileNotFoundError: [Errno 2] No such file or directory: '/home/katt/.local/share/backintime/mnt/1_145168/backintime/main/katt/1/20220418-173836-144'

Downgrading rsync to 3.2.3 makes everything work just fine again.
I'm running backintime 1.3.2 on Arch Linux.

@kattjevfel
Copy link
Author

kattjevfel commented Apr 18, 2022

Bisecting rsync seems to lead to RsyncProject/rsync@6b8db0f:

6b8db0f6440b28d26ef807d17517715c47e62bd9 is the first bad commit
commit 6b8db0f6440b28d26ef807d17517715c47e62bd9
Author: Wayne Davison <wayne@opencoder.net>
Date:   Sun Jan 9 17:35:39 2022 -0800

    Add an arg-protection idiom using backslash-escapes
    
    The new default is to protect args and options from unintended shell
    interpretation using backslash escapes.  See the new `--old-args` option
    for a way to get the old-style splitting.  This idiom was chosen over
    making `--protect-args` enabled by default because it is more backward
    compatible (e.g. it works with rrsync). Fixes #272.

 NEWS.md                |  18 +++++-
 main.c                 |   6 +-
 options.c              | 143 ++++++++++++++++++++++++++++++-----------------
 packaging/cull-options | 147 +++++++++++++++++++++++++++++++++++++++++++++++++
 packaging/cull_options | 143 -----------------------------------------------
 rsync.1.md             |  87 ++++++++++++++++++-----------
 support/rrsync         |   6 +-
 7 files changed, 313 insertions(+), 237 deletions(-)
 create mode 100755 packaging/cull-options
 delete mode 100755 packaging/cull_options

I tried adding --old-args to tools.py and this workaround seem to work fine.

@buhtz
Copy link
Member

buhtz commented Apr 21, 2022

This problem sounds serious and should have high priority.
This issue is about a difference between rsync versions 3.2.4 and 3.2.3. The latter is part of current Debian stable (11). This means we can assume that the problem is still there in "fresher" GNU Linux distros and will become urgent with the next Debian release because 3.2.4 is still part of testing/unstable branch of Debian.

I don't fully understand the details about shell interpretation etc but I got an idea about it.

Is this new default behaviour of rsync 3.2.4 backward compatible with older rsync versions?
If not BIT need to check the rsync version and decide.

@kattjevfel
Copy link
Author

The --old-args is only available in rsync 3.2.4+, so BIT would need to check what version is being used.

@Piergi
Copy link

Piergi commented Apr 30, 2022

I can confirm the bug in my system as well.
Adding "--old-args" in "Expert Options / Additional options to rsync" works here.

This is actually a show-stopper: all my backups failed since the 19th of April, and as I run the backups mostly in the night, I noticed just today.

As a side-note, is it possible to make BiT send an e-mail when a backup fails?

@apolymen
Copy link

apolymen commented Jun 9, 2022

Same here (BiT 1.3.2 in Fedora 35).
Adding "--old-args" in additional options to rsync solves the issue for now.

@protist
Copy link

protist commented Jun 19, 2022

As a side-note, is it possible to make BiT send an e-mail when a backup fails?

@Piergi this would be very useful. I'm using a workaround, although it didn't work in this particular situation. On my server, I have a cron job that runs the following script.

#!/bin/sh

set -e

cd /media/HDD_name/backintime/backup_name/root/1

# This fails if there is nothing found
errors=$(bzcat last_snapshot/takesnapshot.log.bz2 | grep '^\[I\] Take snapshot (rsync: rsync error' -B3)

# This test is redundant
if [ -n "$errors" ]; then
  printf '%s\n' "Errors found in $(realpath last_snapshot)"
  echo 'Log shows:'
  echo
  printf '%s\n\n' "$errors"

  printf '%s %s %s\n' 'last_snapshot/backup contains' "$(ls last_snapshot/backup | wc -l)" 'of 19 items'
  du -hs last_snapshot/backup
  echo

  printf '%s' "Removing $(realpath last_snapshot) . . . "
  rm -r "$(realpath last_snapshot)"
  echo removed
  printf '%s' "Relinking last_snapshot . . . "
  rm last_snapshot
  ln -s "$(ls 20* -d | tail -1)" last_snapshot
  printf '%s\n' "linked to $(realpath last_snapshot)"
fi

It checks the server's log file for an rsync error, and if there had been an error in the last backup, it will remove it, relink the previous one, and mail the user. This was necessary because backintime sometimes fails to backup completely, leaving things in a broken state (despite telling the config not to do this). This would be a problem, because the subsequent backup would start from scratch with new hard links, and take up ~twice the amount of room.

However, this script didn't detect this specific bug, because there was no rsync or error file on the server at all.

@gerum100
Copy link

At least in my setup the additional option "--old-args" does not work, it seems to do and it had created backups, but the permissions file was empty, so a restore will not have the correct file owner and permissions. This could be fixed by export RSYNC_OLD_ARGS=1 before the execution of backintime.

@buhtz
Copy link
Member

buhtz commented Jun 21, 2022

Dear @gerum100 , thanks a lot. This seems like a very important information.

@protist
Copy link

protist commented Jun 24, 2022

Thanks @gerum100. I can confirm that fileinfo.bz2 is empty without this second fix. How did you achieve this exactly? Do you just prepend the generated crontab job command? Presumably this will get rewritten if you modify certain settings via the GUI.

@gerum100
Copy link

@protist If you would add it to the the crontab, it will not work for manually started backups from cmd or the gui, so I have modified the scripts itself, I add the export command directly to the /usr/bin/backintime scripts. But even their an update of backintime will propably overwrite the change, but this issue is so critical, that I hope the next update will contain a permanent solution.

@protist
Copy link

protist commented Jun 25, 2022

@gerum100 good point. Thanks for the hint. That sounds like a good idea.

@buhtz
Copy link
Member

buhtz commented Aug 22, 2022

Some new relevant infos about that.

Maybe it was fixed by Manjaro: #1272

There is also a PR about that: #1256

@aryoda
Copy link
Contributor

aryoda commented Aug 22, 2022

could be fixed by export RSYNC_OLD_ARGS=1 before the execution of backintime.

API-breaking changes are an unlucky design decision and IMHO setting the env var by default is the easiest way to solve this in BiT without knowing which version of rsync is running.

The question is if this should be done by the distribution, package maintainer or in BiT directly...

Since there may not be a common solution for each distribution BiT should set the env var in its local context.

Any opinions?

@aryoda
Copy link
Contributor

aryoda commented Aug 22, 2022

I suggest to export the env var in

by adding

export RSYNC_OLD_ARGS=1.

This should fix the problem by supporting the old and new rsync API (the old version will ignore the env var since it doesn't know it).

Any other missing scripts to set the env var?

@aryoda
Copy link
Contributor

aryoda commented Aug 22, 2022

I have created a patch file, could someone please test it with the new rsync version 3.2.4 or higher (which I don't have installed)? THX

PS: Apply the patch to the Git version by saving the patch file in the project root folder and using the terminal command:
patch -p0 -i issue_1247_rsync_API.fix.txt

issue_1247_rsync_API_fix.txt

@buhtz
Copy link
Member

buhtz commented Aug 23, 2022

I currently doing some research and looking for a solution that works for both versions of rsync without using environment variables, versions checks or other workarounds.
I'm on it...

API-breaking changes are an unlucky design decision

Technically it was API-breaking yes. But I wouldn't see it that dark. In my understanding the behavior of the early rsync version was "unusual". So 3.2.4 just corrects that.

and IMHO setting the env var by default is the easiest way to solve this in BiT without knowing which version of rsync is running.

Easy isn't always the best. I'm looking (and testing) for a better solution. But this environment variable is an option.

The question is if this should be done by the distribution, package maintainer or in BiT directly...

This is definitely upstream's job; our.

That's just my opinion here. I will come up with a possible solution on the bit-dev mailinglist and asking for review.

@buhtz
Copy link
Member

buhtz commented Aug 24, 2022

Can someone please construct me a case (folder and file structure) to reproduce this? The initial message in that issue here is not clear for me.

I am able to "reproduce" the problem using rsync 3.2.5 alone (without backintime).

But I am not able to reproduce this using backintime in the same system.

I checked the environment variables and the debug output (incl. the rsync call reported there). There is no hint that someone from the distro maintainers where activating one of the known workarounds.

I used that source folder in HOME and backed it up with backintime without any problems. I also deleted one of the snapshots. No problem.

dir with blank
├── file one
├── foobar
├── second file
└── sub dir
    └── bar.dat

It was Debian 12 (testing) with backintime 1.3.2 from that distro repo.

And again I was able to reproduce with rsync alone.

EDIT: Now I also tried the upstream version of BiT to make sure that there wasn't a patch from any distro maintainer.

@buhtz
Copy link
Member

buhtz commented Aug 28, 2022

Sorry, I got it and now can reproduce this with BIT, too. I simply forgot to configure a "ssh" job instead of a "local".

Sidenote for myself:

When implementing the new solution I have to take care of --old-args setup explicit by users. This may cause trouble or it may be overriden by -s. But we shouldn't trust that and testing doesn't help either because of the experience that the different rsync versions behave extremely different when using arguments -V, -VV or -version. Idea: remove --old-args when calling rsync and implement a warning/info in the stetting's GUI dialog that this argument is not allowed an will be removed.

There is also the problem with environment variables RSYNC_OLD_ARGS. Check it and warn about it (somewhere). But it can be overriden by --no-old-args.

Note for myself:

The following unittest's do reproduce the problem and reacting on RSYNC_OLD_ARGS

@buhtz
Copy link
Member

buhtz commented Sep 26, 2022

Just for your information and keeping you up to date.

I'm still on that problem.

The next hurdle is to create unittests for the relevant code parts. Because most of it is untested or IMHO not very well tested. I'm scared to modify that code without having unittests.

This implicates some refactoring to make the code even testable before I can write any unittest.

@buhtz
Copy link
Member

buhtz commented Oct 23, 2022

Let me give you a small update.

I modified all relevant code parts to use my approach and added some unittests. The latter are merged into master.
Next week I will start with systematic manual testing of different use cases on different distros.
After that I will offer you a branch in my forked repo for testing.

@buhtz
Copy link
Member

buhtz commented Nov 4, 2022

Would be glad if some of you could test if the PR #1351 do fix the problem.

@glyndon
Copy link

glyndon commented Nov 7, 2022

Would be glad if some of you could test if the PR #1351 do fix the problem.

Worked here (Ubuntu 22.10, rsync 3.2.5).

@emtiu emtiu unpinned this issue Nov 8, 2022
aryoda pushed a commit that referenced this issue Nov 9, 2022
…#1351)

The core problem is that BIT does call rsync in a way that is not compatible
with newer versions of rsync. The version 3.2.4 of rsync activated a new
method of argument protection. This commit fixes the rsync calls and does all
necessary modifications.

Important to know is that BIT still works with old rsync versions, too.

Fix #1247
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants