Merge lp:~jelmer/brz/just-read-mtab into lp:brz

[Jelmer Vernooij]

Read filesystem metadata from mtab directly rather than going via psutil.

Read filesystem metadata from mtab directly rather than going via psutil.

[Martin Packman (gz)]

Looks good, see a few inline comments. May be worth filtering out `path.startswith(('/sys/', '/run/'))` which I have quite a few of and are never(?) going to be interesting for our purposes.

review: Approve

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/brz/job/brz-land/377/

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/brz/job/brz-land/381/

=== modified file 'breezy/osutils.py'
--- breezy/osutils.py	2019-06-16 01:03:51 +0000
+++ breezy/osutils.py	2019-06-16 15:36:24 +0000
@@ -2634,27 +2634,77 @@
     return False
 
 
+def read_mtab(path):
+    """Read an fstab-style file and extract mountpoint+filesystem information.
+
+    :param path: Path to read from
+    :yield: Tuples with mountpoints (as bytestrings) and filesystem names
+    """
+    with open(path, 'rb') as f:
+        for line in f:
+            if line.startswith(b'#'):
+                continue
+            cols = line.split()
+            if len(cols) < 3:
+                continue
+            yield cols[1], cols[2].decode('ascii', 'replace')
+
+
+MTAB_PATH = '/etc/mtab'
+
+class FilesystemFinder(object):
+    """Find the filesystem for a particular path."""
+
+    def __init__(self, mountpoints):
+        def key(x):
+            return len(x[0])
+        self._mountpoints = sorted(mountpoints, key=key, reverse=True)
+
+    @classmethod
+    def from_mtab(cls):
+        """Create a FilesystemFinder from an mtab-style file.
+
+        Note that this will silenty ignore mtab if it doesn't exist or can not
+        be opened.
+        """
+        # TODO(jelmer): Use inotify to be notified when /etc/mtab changes and
+        # we need to re-read it.
+        try:
+            return cls(read_mtab(MTAB_PATH))
+        except EnvironmentError as e:
+            trace.mutter('Unable to read mtab: %s', e)
+            return cls([])
+
+    def find(self, path):
+        """Find the filesystem used by a particular path.
+
+        :param path: Path to find (bytestring or text type)
+        :return: Filesystem name (as text type) or None, if the filesystem is
+            unknown.
+        """
+        for mountpoint, filesystem in self._mountpoints:
+            if is_inside(mountpoint, path):
+                return filesystem
+        return None
+
+
+_FILESYSTEM_FINDER = None
+
+
 def get_fs_type(path):
     """Return the filesystem type for the partition a path is in.
 
     :param path: Path to search filesystem type for
     :return: A FS type, as string. E.g. "ext2"
     """
-    # TODO(jelmer): It would be nice to avoid an extra dependency here, but the only
-    # alternative is reading platform-specific files under /proc :(
-    try:
-        import psutil
-    except ImportError as e:
-        raise errors.DependencyNotPresent('psutil', e)
+    global _FILESYSTEM_FINDER
+    if _FILESYSTEM_FINDER is None:
+        _FILESYSTEM_FINDER = FilesystemFinder.from_mtab()
 
-    if not PY3 and not isinstance(path, str):
+    if not isinstance(path, bytes):
         path = path.encode(_fs_enc)
 
-    for part in sorted(psutil.disk_partitions(), key=lambda x: len(x.mountpoint), reverse=True):
-        if is_inside(part.mountpoint, path):
-            return part.fstype
-    # Unable to parse the file? Since otherwise at least the entry for / should match..
-    return None
+    return _FILESYSTEM_FINDER.find(path)
 
 
 if PY3:

=== modified file 'breezy/tests/test_osutils.py'
--- breezy/tests/test_osutils.py	2019-06-16 01:03:51 +0000
+++ breezy/tests/test_osutils.py	2019-06-16 15:36:24 +0000
@@ -62,8 +62,6 @@
 
 term_ios_feature = features.ModuleAvailableFeature('termios')
 
-psutil_feature = features.ModuleAvailableFeature('psutil')
-
 
 def _already_unicode(s):
     return s
@@ -2356,8 +2354,45 @@
         self.assertIsInstance(osutils.supports_symlinks(self.test_dir), bool)
 
 
+class MtabReader(tests.TestCaseInTempDir):
+
+    def test_read_mtab(self):
+        self.build_tree_contents([('mtab', """\
+/dev/mapper/blah--vg-root / ext4 rw,relatime,errors=remount-ro 0 0
+/dev/mapper/blah--vg-home /home vfat rw,relatime 0 0
+# comment
+
+iminvalid
+""")])
+        self.assertEqual(
+            list(osutils.read_mtab('mtab')),
+            [(b'/', 'ext4'),
+             (b'/home', 'vfat')])
+
+
 class GetFsTypeTests(tests.TestCaseInTempDir):
 
-    def test_returns_string(self):
-        self.requireFeature(psutil_feature)
-        self.assertIsInstance(osutils.get_fs_type(self.test_dir), str)
+    def test_returns_string_or_none(self):
+        ret = osutils.get_fs_type(self.test_dir)
+        self.assertTrue(isinstance(ret, text_type) or ret is None)
+
+    def test_returns_most_specific(self):
+        self.overrideAttr(
+            osutils, '_FILESYSTEM_FINDER',
+            osutils.FilesystemFinder(
+                [(b'/', 'ext4'), (b'/home', 'vfat'),
+                 (b'/home/jelmer', 'ext2')]))
+        self.assertEqual(osutils.get_fs_type(b'/home/jelmer/blah'), 'ext2')
+        self.assertEqual(osutils.get_fs_type('/home/jelmer/blah'), 'ext2')
+        self.assertEqual(osutils.get_fs_type(b'/home/jelmer'), 'ext2')
+        self.assertEqual(osutils.get_fs_type(b'/home/martin'), 'vfat')
+        self.assertEqual(osutils.get_fs_type(b'/home'), 'vfat')
+        self.assertEqual(osutils.get_fs_type(b'/other'), 'ext4')
+
+    def test_returns_none(self):
+        self.overrideAttr(
+            osutils, '_FILESYSTEM_FINDER',
+            osutils.FilesystemFinder([]))
+        self.assertIs(osutils.get_fs_type('/home/jelmer/blah'), None)
+        self.assertIs(osutils.get_fs_type(b'/home/jelmer/blah'), None)
+        self.assertIs(osutils.get_fs_type('/home/jelmer'), None)

=== modified file 'doc/en/release-notes/brz-3.1.txt'
--- doc/en/release-notes/brz-3.1.txt	2019-06-15 19:09:08 +0000
+++ doc/en/release-notes/brz-3.1.txt	2019-06-16 15:36:24 +0000
@@ -30,6 +30,11 @@
  * The 'quilt' plugin, extracted from brz-debian, is now
    bundled. (Jelmer Vernooij)
 
+* Directly read mtab rather than using psutil when trying to figure out
+  filesystem types. This removes a dependency that not all users may
+  have installed and speeds up import time since psutil brings in
+  various other modules. (Jelmer Vernooij)
+
 Improvements
 ************