Repository: Backblaze/B2_Command_Line_Tool

Issue:
Broken symlink break sync
I had this issue where one of my sysmlinks was broken and b2 tool broke, this is the stack trace:

```
Traceback (most recent call last):                                                
  File "/usr/local/bin/b2", line 9, in <module>
    load_entry_point('b2==0.5.4', 'console_scripts', 'b2')()
  File "/usr/local/lib/python2.7/dist-packages/b2/console_tool.py", line 861, in main
    exit_status = ct.run_command(decoded_argv)
  File "/usr/local/lib/python2.7/dist-packages/b2/console_tool.py", line 789, in run_command
    return command.run(args)
  File "/usr/local/lib/python2.7/dist-packages/b2/console_tool.py", line 609, in run
    max_workers=max_workers
  File "/usr/local/lib/python2.7/dist-packages/b2/sync.py", line 877, in sync_folders
    source_folder, dest_folder, args, now_millis, reporter
  File "/usr/local/lib/python2.7/dist-packages/b2/sync.py", line 777, in make_folder_sync_actions
    for (source_file, dest_file) in zip_folders(source_folder, dest_folder):
  File "/usr/local/lib/python2.7/dist-packages/b2/sync.py", line 646, in zip_folders
    current_a = next_or_none(iter_a)
  File "/usr/local/lib/python2.7/dist-packages/b2/sync.py", line 620, in next_or_none
    return six.advance_iterator(iterator)
  File "/usr/local/lib/python2.7/dist-packages/b2/sync.py", line 499, in all_files
    yield self._make_file(relative_path)
  File "/usr/local/lib/python2.7/dist-packages/b2/sync.py", line 553, in _make_file
    mod_time = int(round(os.path.getmtime(full_path) * 1000))
  File "/usr/lib/python2.7/genericpath.py", line 54, in getmtime
    return os.stat(filename).st_mtime
OSError: [Errno 2] No such file or directory: '/media/2a9074d0-4788-45ab-bfae-fc46427c69fa/PersonalData/some-broken-symlink'

```

Discussion:

Patch:
diff --git a/b2/sync.py b/b2/sync.py
index c3c4ad9..cffdc81 100644
--- a/b2/sync.py
+++ b/b2/sync.py
@@ -67,12 +67,15 @@ class SyncReport(object):
         self.closed = False
         self.lock = threading.Lock()
         self._update_progress()
+        self.warnings = []
 
     def close(self):
         with self.lock:
             if not self.no_progress:
                 self._print_line('', False)
             self.closed = True
+            for warning in self.warnings:
+                self._print_line(warning, True)
 
     def __enter__(self):
         return self
@@ -185,6 +188,9 @@ class SyncReport(object):
             self.transfer_bytes += byte_delta
             self._update_progress()
 
+    def local_access_error(self, path):
+        self.warnings.append('WARNING: %s could not be accessed (broken symlink?)' % (path,))
+
 
 class SyncFileReporter(AbstractProgressListener):
     """
@@ -453,13 +459,17 @@ class AbstractFolder(object):
     """
 
     @abstractmethod
-    def all_files(self):
+    def all_files(self, reporter):
         """
         Returns an iterator over all of the files in the folder, in
         the order that B2 uses.
 
         No matter what the folder separator on the local file system
         is, "/" is used in the returned file names.
+
+        If a file is found, but does not exist (for example due to
+        a broken symlink or a race), reporter will be informed about
+        each such problem.
         """
 
     @abstractmethod
@@ -494,9 +504,9 @@ class LocalFolder(AbstractFolder):
     def folder_type(self):
         return 'local'
 
-    def all_files(self):
+    def all_files(self, reporter):
         prefix_len = len(self.root) + 1  # include trailing '/' in prefix length
-        for relative_path in self._walk_relative_paths(prefix_len, self.root):
+        for relative_path in self._walk_relative_paths(prefix_len, self.root, reporter):
             yield self._make_file(relative_path)
 
     def make_full_path(self, file_name):
@@ -514,7 +524,7 @@ class LocalFolder(AbstractFolder):
         elif not os.path.isdir(self.root):
             raise Exception('%s is not a directory' % (self.root,))
 
-    def _walk_relative_paths(self, prefix_len, dir_path):
+    def _walk_relative_paths(self, prefix_len, dir_path, reporter):
         """
         Yields all of the file names anywhere under this folder, in the
         order they would appear in B2.
@@ -535,16 +545,21 @@ class LocalFolder(AbstractFolder):
                 )
             full_path = os.path.join(dir_path, name)
             relative_path = full_path[prefix_len:]
-            if os.path.isdir(full_path):
-                name += six.u('/')
-                dirs.add(name)
-            names[name] = (full_path, relative_path)
+            # Skip broken symlinks or other inaccessible files
+            if not os.path.exists(full_path):
+                if reporter is not None:
+                    reporter.local_access_error(full_path)
+            else:
+                if os.path.isdir(full_path):
+                    name += six.u('/')
+                    dirs.add(name)
+                names[name] = (full_path, relative_path)
 
         # Yield all of the answers
         for name in sorted(names):
             (full_path, relative_path) = names[name]
             if name in dirs:
-                for rp in self._walk_relative_paths(prefix_len, full_path):
+                for rp in self._walk_relative_paths(prefix_len, full_path, reporter):
                     yield rp
             else:
                 yield relative_path
@@ -573,7 +588,7 @@ class B2Folder(AbstractFolder):
         self.bucket = api.get_bucket_by_name(bucket_name)
         self.prefix = '' if self.folder_name == '' else self.folder_name + '/'
 
-    def all_files(self):
+    def all_files(self, reporter):
         current_name = None
         current_versions = []
         for (file_version_info, folder_name) in self.bucket.ls(
@@ -625,7 +640,7 @@ def next_or_none(iterator):
         return None
 
 
-def zip_folders(folder_a, folder_b, exclusions=tuple()):
+def zip_folders(folder_a, folder_b, reporter, exclusions=tuple()):
     """
     An iterator over all of the files in the union of two folders,
     matching file names.
@@ -637,8 +652,10 @@ def zip_folders(folder_a, folder_b, exclusions=tuple()):
     :param folder_b: A Folder object.
     """
 
-    iter_a = (f for f in folder_a.all_files() if not any(ex.match(f.name) for ex in exclusions))
-    iter_b = folder_b.all_files()
+    iter_a = (
+        f for f in folder_a.all_files(reporter) if not any(ex.match(f.name) for ex in exclusions)
+    )
+    iter_b = folder_b.all_files(reporter)
 
     current_a = next_or_none(iter_a)
     current_b = next_or_none(iter_b)
@@ -810,7 +827,7 @@ def make_folder_sync_actions(source_folder, dest_folder, args, now_millis, repor
         ('b2', 'local'), ('local', 'b2')
     ]:
         raise NotImplementedError("Sync support only local-to-b2 and b2-to-local")
-    for (source_file, dest_file) in zip_folders(source_folder, dest_folder, exclusions):
+    for (source_file, dest_file) in zip_folders(source_folder, dest_folder, reporter, exclusions):
         if source_folder.folder_type() == 'local':
             if source_file is not None:
                 reporter.update_compare(1)
@@ -863,7 +880,9 @@ def count_files(local_folder, reporter):
     """
     Counts all of the files in a local folder.
     """
-    for _ in local_folder.all_files():
+    # Don't pass in a reporter to all_files.  Broken symlinks will be reported
+    # during the next pass when the source and dest files are compared.
+    for _ in local_folder.all_files(None):
         reporter.update_local(1)
     reporter.end_local()
 


Test patch:
diff --git a/test/test_sync.py b/test/test_sync.py
index ad2b140..9102b6e 100644
--- a/test/test_sync.py
+++ b/test/test_sync.py
@@ -37,36 +37,58 @@ def write_file(path, contents):
         f.write(contents)
 
 
-def create_files(root_dir, relative_paths):
-    for relative_path in relative_paths:
-        full_path = os.path.join(root_dir, relative_path)
-        write_file(full_path, b'')
+class TestLocalFolder(unittest.TestCase):
+    NAMES = [
+        six.u('.dot_file'), six.u('hello.'), six.u('hello/a/1'), six.u('hello/a/2'),
+        six.u('hello/b'), six.u('hello0'), six.u('\u81ea\u7531')
+    ]
 
+    def setUp(self):
+        self.reporter = MagicMock()
+
+    @classmethod
+    def _create_files(cls, root_dir, relative_paths):
+        for relative_path in relative_paths:
+            full_path = os.path.join(root_dir, relative_path)
+            write_file(full_path, b'')
+
+    def _prepare_folder(self, root_dir, broken_symlink=False):
+        self._create_files(root_dir, self.NAMES)
+        if broken_symlink:
+            os.symlink(
+                os.path.join(root_dir, 'non_existant_file'), os.path.join(root_dir, 'bad_symlink')
+            )
+        return LocalFolder(root_dir)
 
-class TestLocalFolder(unittest.TestCase):
     def test_slash_sorting(self):
         # '/' should sort between '.' and '0'
-        names = [
-            six.u('.dot_file'), six.u('hello.'), six.u('hello/a/1'), six.u('hello/a/2'),
-            six.u('hello/b'), six.u('hello0'), six.u('\u81ea\u7531')
-        ]
         with TempDir() as tmpdir:
-            create_files(tmpdir, names)
-            folder = LocalFolder(tmpdir)
-            actual_names = list(f.name for f in folder.all_files())
-            self.assertEqual(names, actual_names)
+            folder = self._prepare_folder(tmpdir)
+            actual_names = list(f.name for f in folder.all_files(self.reporter))
+            self.assertEqual(self.NAMES, actual_names)
+            self.reporter.local_access_error.assert_not_called()
+
+    def test_broken_symlink(self):
+        with TempDir() as tmpdir:
+            folder = self._prepare_folder(tmpdir, broken_symlink=True)
+            for f in folder.all_files(self.reporter):
+                pass  # just generate all the files
+            self.reporter.local_access_error.assert_called_once_with(
+                os.path.join(tmpdir, 'bad_symlink')
+            )
 
 
 class TestB2Folder(unittest.TestCase):
     def setUp(self):
         self.bucket = MagicMock()
         self.api = MagicMock()
+        self.reporter = MagicMock()
         self.api.get_bucket_by_name.return_value = self.bucket
         self.b2_folder = B2Folder('bucket-name', 'folder', self.api)
 
     def test_empty(self):
         self.bucket.ls.return_value = []
-        self.assertEqual([], list(self.b2_folder.all_files()))
+        self.assertEqual([], list(self.b2_folder.all_files(self.reporter)))
 
     def test_multiple_versions(self):
         # Test two files, to cover the yield within the loop, and
@@ -102,7 +124,7 @@ class TestB2Folder(unittest.TestCase):
             [
                 "File(a.txt, [FileVersion('a2', 'folder/a.txt', 2000, 'upload'), FileVersion('a1', 'folder/a.txt', 1000, 'upload')])",
                 "File(b.txt, [FileVersion('b2', 'folder/b.txt', 2000, 'upload'), FileVersion('b1', 'folder/b.txt', 1000, 'upload')])",
-            ], [str(f) for f in self.b2_folder.all_files()]
+            ], [str(f) for f in self.b2_folder.all_files(self.reporter)]
         )
 
 
@@ -111,7 +133,7 @@ class FakeFolder(AbstractFolder):
         self.f_type = f_type
         self.files = files
 
-    def all_files(self):
+    def all_files(self, reporter):
         return iter(self.files)
 
     def folder_type(self):
@@ -150,16 +172,19 @@ class TestParseSyncFolder(unittest.TestCase):
 
 
 class TestZipFolders(unittest.TestCase):
+    def setUp(self):
+        self.reporter = MagicMock()
+
     def test_empty(self):
         folder_a = FakeFolder('b2', [])
         folder_b = FakeFolder('b2', [])
-        self.assertEqual([], list(zip_folders(folder_a, folder_b)))
+        self.assertEqual([], list(zip_folders(folder_a, folder_b, self.reporter)))
 
     def test_one_empty(self):
         file_a1 = File("a.txt", [FileVersion("a", "a", 100, "upload", 10)])
         folder_a = FakeFolder('b2', [file_a1])
         folder_b = FakeFolder('b2', [])
-        self.assertEqual([(file_a1, None)], list(zip_folders(folder_a, folder_b)))
+        self.assertEqual([(file_a1, None)], list(zip_folders(folder_a, folder_b, self.reporter)))
 
     def test_two(self):
         file_a1 = File("a.txt", [FileVersion("a", "a", 100, "upload", 10)])
@@ -174,9 +199,22 @@ class TestZipFolders(unittest.TestCase):
             [
                 (file_a1, None), (file_a2, file_b1), (file_a3, None), (None, file_b2),
                 (file_a4, None)
-            ], list(zip_folders(folder_a, folder_b))
+            ], list(zip_folders(folder_a, folder_b, self.reporter))
         )
 
+    def test_pass_reporter_to_folder(self):
+        """
+        Check that the zip_folders() function passes the reporter through
+        to both folders.
+        """
+        folder_a = MagicMock()
+        folder_b = MagicMock()
+        folder_a.all_files = MagicMock(return_value=iter([]))
+        folder_b.all_files = MagicMock(return_value=iter([]))
+        self.assertEqual([], list(zip_folders(folder_a, folder_b, self.reporter)))
+        folder_a.all_files.assert_called_once_with(self.reporter)
+        folder_b.all_files.assert_called_once_with(self.reporter)
+
 
 class FakeArgs(object):
     """
diff --git a/test_b2_command_line.py b/test_b2_command_line.py
index 8d23678..0628248 100644
--- a/test_b2_command_line.py
+++ b/test_b2_command_line.py
@@ -200,6 +200,8 @@ class CommandLine(object):
                 sys.exit(1)
         if expected_pattern is not None:
             if re.search(expected_pattern, stdout) is None:
+                print('STDOUT:')
+                print(stdout)
                 error_and_exit('did not match pattern: ' + expected_pattern)
         return stdout
 
@@ -469,8 +471,12 @@ def _sync_test_using_dir(b2_tool, bucket_name, dir_):
         write_file(p('a'), b'hello')
         write_file(p('b'), b'hello')
         write_file(p('c'), b'hello')
+        os.symlink('broken', p('d'))
 
-        b2_tool.should_succeed(['sync', '--noProgress', dir_path, b2_sync_point])
+        b2_tool.should_succeed(
+            ['sync', '--noProgress', dir_path, b2_sync_point],
+            expected_pattern="/d could not be accessed"
+        )
         file_versions = b2_tool.list_file_versions(bucket_name)
         should_equal(
             [