From 2d6cc2612782b4801ba3c3c1dcd01943e3a9e5a5 Mon Sep 17 00:00:00 2001 From: Arda Xi Date: Sun, 21 Apr 2019 19:33:41 +0200 Subject: [PATCH 1/4] Make isdir argument to _ErrorTarget optional --- src/allmydata/scripts/tahoe_backup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/allmydata/scripts/tahoe_backup.py b/src/allmydata/scripts/tahoe_backup.py index cfb83f0ec..5694b8801 100644 --- a/src/allmydata/scripts/tahoe_backup.py +++ b/src/allmydata/scripts/tahoe_backup.py @@ -365,7 +365,7 @@ class DirectoryTarget(object): class _ErrorTarget(object): - def __init__(self, path, isdir): + def __init__(self, path, isdir=False): self._path = path self._quoted_path = quote_local_unicode_path(path) self._isdir = isdir From de8229345f833317bd3bdfc75e9d3156ab57308f Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 26 Apr 2019 08:51:41 -0400 Subject: [PATCH 2/4] news fragment --- newsfragments/2950.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/2950.bugfix diff --git a/newsfragments/2950.bugfix b/newsfragments/2950.bugfix new file mode 100644 index 000000000..7558578fc --- /dev/null +++ b/newsfragments/2950.bugfix @@ -0,0 +1 @@ +`tahoe backup` no longer fails with an unhandled exception when it encounters a fifo in the backup source. \ No newline at end of file From 0ab197d9283f27581e333e4aea07107db504e54a Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Fri, 26 Apr 2019 10:30:35 -0400 Subject: [PATCH 3/4] Add a test for the problematic case Get the name in the warning right --- src/allmydata/test/cli/test_backup.py | 74 ++++++++++++++++++++++++--- 1 file changed, 67 insertions(+), 7 deletions(-) diff --git a/src/allmydata/test/cli/test_backup.py b/src/allmydata/test/cli/test_backup.py index 21527e012..7e7678b2a 100644 --- a/src/allmydata/test/cli/test_backup.py +++ b/src/allmydata/test/cli/test_backup.py @@ -14,10 +14,20 @@ from allmydata.util.namespace import Namespace from allmydata.scripts import cli, backupdb from ..common_util import StallMixin from ..no_network import GridTestMixin -from .common import CLITestMixin, parse_options +from .common import ( + CLITestMixin, + parse_options, +) +from ..common import ( + skipIf, +) timeout = 480 # deep_check takes 360s on Zandr's linksys box, others take > 240s +def _unsupported(what): + return "{} are not supported by Python on this platform.".format(what) + + class Backup(GridTestMixin, CLITestMixin, StallMixin, unittest.TestCase): def writeto(self, path, data): @@ -405,24 +415,74 @@ class Backup(GridTestMixin, CLITestMixin, StallMixin, unittest.TestCase): self.failUnless(ns.called) def test_ignore_symlinks(self): + """ + A symlink encountered in the backed-up directory is skipped with a + warning. + """ if not hasattr(os, 'symlink'): - raise unittest.SkipTest("Symlinks are not supported by Python on this platform.") + raise unittest.SkipTest(_unsupported("Symlinks")) + def make_symlink(path): + self.writeto("foo.txt", "foo") + os.symlink( + os.path.join( + os.path.dirname(path), + "foo.txt", + ), + path, + ) + + return self._ignore_something_test(u"Symlink", make_symlink) + + @skipIf(getattr(os, "mkfifo", None) is None, _unsupported("FIFOs")) + def test_ignore_fifo(self): + """ + A FIFO encountered in the backed-up directory is skipped with a warning. + """ + def make_fifo(path): + # Create the thing to ignore + os.makedirs(os.path.dirname(path)) + os.mkfifo(path) + # Also create anothing thing so the counts end up the same as + # those in the symlink test and it's easier to re-use the testing + # helper. + self.writeto("count-dummy.txt", "foo") + + return self._ignore_something_test(u"special", make_fifo) + + def _ignore_something_test(self, kind_of_thing, make_something_to_ignore): + """ + Assert that when a a certain kind of file is encountered in the backed-up + directory a warning that it is not supported is emitted and the backup + proceeds to other files with no other error. + + :param unicode kind_of_thing: The name of the kind of file that will + be ignored. This is expected to appear in the warning. + + :param make_something_to_ignore: A one-argument callable which creates + the file that is expected to be ignored. It is called with the + path at which the file must be created. + + :return Deferred: A ``Deferred`` that fires when the assertion has + been made. + """ self.basedir = os.path.dirname(self.mktemp()) self.set_up_grid(oneshare=True) source = os.path.join(self.basedir, "home") - self.writeto("foo.txt", "foo") - os.symlink(os.path.join(source, "foo.txt"), os.path.join(source, "foo2.txt")) + ignored_path = os.path.join(source, "foo2.txt") + make_something_to_ignore(ignored_path) d = self.do_cli("create-alias", "tahoe") d.addCallback(lambda res: self.do_cli("backup", "--verbose", source, "tahoe:test")) def _check((rc, out, err)): self.failUnlessReallyEqual(rc, 2) - foo2 = os.path.join(source, "foo2.txt") - self.failUnlessIn("WARNING: cannot backup symlink ", err) - self.failUnlessIn(foo2, err) + self.assertIn( + "WARNING: cannot backup {} ".format(kind_of_thing.lower()), + err, + ) + self.assertIn(ignored_path, err) fu, fr, fs, dc, dr, ds = self.count_output(out) # foo.txt From 7ac0b93148eaba9429ee162bdb91914e3d9869dc Mon Sep 17 00:00:00 2001 From: Arda Xi Date: Fri, 26 Apr 2019 21:40:21 +0200 Subject: [PATCH 4/4] Specify news fragment for 2950 --- newsfragments/2950.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/newsfragments/2950.bugfix b/newsfragments/2950.bugfix index 7558578fc..8df25e4ff 100644 --- a/newsfragments/2950.bugfix +++ b/newsfragments/2950.bugfix @@ -1 +1 @@ -`tahoe backup` no longer fails with an unhandled exception when it encounters a fifo in the backup source. \ No newline at end of file +`tahoe backup` no longer fails with an unhandled exception when it encounters a special file (device, fifo) in the backup source.