Move directory check from _isRoot into _findRoot

The check being performed in the _IsRoot function to see whether the directory given is an actually existing one does not have to be performed for every directory in traversed using _findRoot. We only traverse the list upward (that is, from some directory towards the root of the file system) and once we convinced ourselve that the initial directory is valid all subsequent ones should be so as well. To that end, this change moves the directory check into the _findRoot function in order to get rid of some unnecessary work but also in anticipation of a future change which will repurpose _findRoot.

Author: d-e-s-o

btrfs-backup/src/deso/btrfs/repository.py

@@ -1,7 +1,7 @@
 # repository.py
 
 #/***************************************************************************
-# * Copyright (C) 2015 Daniel Mueller (deso@posteo.net)  *
+# * Copyright (C) 2015-2016 Daniel Mueller (deso@posteo.net) *
 # * *
 # * This program is free software: you can redistribute it and/or modify *
 # * it under the terms of the GNU General Public License as published by *
@@ -211,29 +211,24 @@ def _snapshotFiles(directory, extension, repository):
  return files
 
 
-def _isRoot(directory, repository):
- """Check if a given directory represents the root of a btrfs file system."""
- def isDir(directory):
- """Check if a directory exists."""
- try:
- # Append a trailing separator here to indicate that we are
- # checking for a directory. This way ls will fail if it is not. If
- # we left out the trailing separator and the directory actually
- # points to a file, ls would still succeed.
- func = lambda: ["/bin/ls", _trail(directory)]
- cmd = repository.command(func)
+def _isDir(directory, repository):
+ """Check if a directory exists."""
+ try:
+ # Append a trailing separator here to indicate that we are
+ # checking for a directory. This way ls will fail if it is not. If
+ # we left out the trailing separator and the directory actually
+ # points to a file, ls would still succeed.
+ func = lambda: ["/bin/ls", _trail(directory)]
+ cmd = repository.command(func)
+
+ execute(*cmd, stderr=repository.stderr)
+ return True
+ except ProcessError:
+ return False
 
- execute(*cmd, stderr=repository.stderr)
- return True
- except ProcessError:
- return False
-
- # TODO: We might want to move this invocation into _findRoot to avoid
- # unnecessary invocations, especially since they might be
- # remote.
- if not isDir(directory):
- raise FileNotFoundError("Directory \"%s\" not found." % directory)
 
+def _isRoot(directory, repository):
+ """Check if a given directory represents the root of a btrfs file system."""
  try:
  cmd = repository.command(show, directory)
  output, _ = execute(*cmd, stdout=b"", stderr=repository.stderr)
@@ -256,6 +251,9 @@ def _findRoot(directory, repository):
  """Find the root of the btrfs file system containing the given directory."""
  assert directory
 
+ if not _isDir(directory, repository):
+ raise FileNotFoundError("Directory \"%s\" not found." % directory)
+
  cur_directory = directory
  # Note that we have no guard here against an empty directory as input
  # or later because of a dirname invocation. However, the show command

btrfs-backup/src/deso/btrfs/test/testRepository.py

@@ -1,7 +1,7 @@
 # testRepository.py
 
 #/***************************************************************************
-# * Copyright (C) 2015 Daniel Mueller (deso@posteo.net)  *
+# * Copyright (C) 2015-2016 Daniel Mueller (deso@posteo.net) *
 # * *
 # * This program is free software: you can redistribute it and/or modify *
 # * it under the terms of the GNU General Public License as published by *
@@ -138,7 +138,7 @@ def doTest(path, expected):
  def testFindRootCorrectDirectory(self):
  """Verify that in case of an error _findRoot reports the proper directory."""
  directory = self._mount.path("non-existent-directory")
- regex = r"Root of btrfs.*not found.*: \"%s\"" % directory
+ regex = r"Directory.* \"%s\".*not found.*" % directory
  repo = Repository(self._mount.path())
 
  with patch("deso.btrfs.repository._isRoot") as mock_isroot: