fix #431 - directory not ignored with negative pattern

.gitignore

@@ -7,6 +7,8 @@ build/
 tags
 env
 venv
+.pytest_cache
+.mypy_cache
 
 # coverage stuff
 .coverage

dotdrop/comparator.py

@@ -6,11 +6,11 @@
 """
 
 import os
-import filecmp
 
 # local imports
 from dotdrop.logger import Logger
-from dotdrop.utils import must_ignore, uniq_list, diff, \
+from dotdrop.ftree import FTreeDir
+from dotdrop.utils import must_ignore, diff, \
     get_file_perm
 
 
@@ -22,6 +22,7 @@ def __init__(self, diff_cmd='', debug=False,
         """constructor
         @diff_cmd: diff command to use
         @debug: enable debug
+        @ignore_missing_in_dotdrop: ignore missing files in dotdrop
         """
         self.diff_cmd = diff_cmd
         self.debug = debug
@@ -38,9 +39,27 @@ def compare(self, local_path, deployed_path, ignore=None, mode=None):
             ignore = []
         local_path = os.path.expanduser(local_path)
         deployed_path = os.path.expanduser(deployed_path)
+
         self.log.dbg(f'comparing \"{local_path}\" and \"{deployed_path}\"')
         self.log.dbg(f'ignore pattern(s): {ignore}')
 
+        return self._compare(local_path, deployed_path,
+                             ignore=ignore, mode=mode,
+                             recurse=True)
+
+    def _compare(self, local_path, deployed_path,
+                 ignore=None, mode=None,
+                 recurse=False):
+        if not ignore:
+            ignore = []
+
+        # test existence
+        if not os.path.exists(local_path):
+            return f'=> \"{local_path}\" does not exist on destination\n'
+        if not self.ignore_missing_in_dotdrop:
+            if not os.path.exists(deployed_path):
+                return f'=> \"{deployed_path}\" does not exist in dotdrop\n'
+
         # test type of file
         if os.path.isdir(local_path) and not os.path.isdir(deployed_path):
             ret = f'\"{local_path}\" is a dir'
@@ -51,17 +70,19 @@ def compare(self, local_path, deployed_path, ignore=None, mode=None):
             ret += f' while \"{deployed_path}\" is a dir\n'
             return ret
 
-        # test content
+        # is a file
         if not os.path.isdir(local_path):
             self.log.dbg(f'{local_path} is a file')
             ret = self._comp_file(local_path, deployed_path, ignore)
             if not ret:
                 ret = self._comp_mode(local_path, deployed_path, mode=mode)
             return ret
 
+        # is a directory
         self.log.dbg(f'\"{local_path}\" is a directory')
-
-        ret = self._comp_dir(local_path, deployed_path, ignore)
+        ret = ''
+        if recurse:
+            ret = self._comp_dir(local_path, deployed_path, ignore)
         if not ret:
             ret = self._comp_mode(local_path, deployed_path, mode=mode)
         return ret
@@ -93,7 +114,7 @@ def _comp_file(self, local_path, deployed_path, ignore):
                                debug=self.debug):
             self.log.dbg(f'ignoring diff {local_path} and {deployed_path}')
             return ''
-        return self._diff(local_path, deployed_path)
+        return self._diff(local_path, deployed_path, header=True)
 
     def _comp_dir(self, local_path, deployed_path, ignore):
         """compare a directory"""
@@ -112,90 +133,49 @@ def _comp_dir(self, local_path, deployed_path, ignore):
         if not os.path.isdir(deployed_path):
             return f'\"{deployed_path}\" is a file\n'
 
-        return self._compare_dirs(local_path, deployed_path, ignore)
+        return self._compare_dirs2(local_path, deployed_path, ignore)
 
-    def _compare_dirs(self, local_path, deployed_path, ignore):
+    def _compare_dirs2(self, local_path, deployed_path, ignore):
         """compare directories"""
         self.log.dbg(f'compare dirs {local_path} and {deployed_path}')
         ret = []
-        comp = filecmp.dircmp(local_path, deployed_path)
-
-        # handle files and subdirs only in deployed dir
-        self.log.dbg(f'files/dirs only in deployed dir: {comp.left_only}')
-        for i in comp.left_only:
-            abspath1 = os.path.join(local_path, i)
-            if os.path.isdir(abspath1):
-                abspath1 += os.path.sep
-            abspath2 = os.path.join(deployed_path, i)
-            if os.path.isdir(abspath2):
-                abspath2 += os.path.sep
-            if self.ignore_missing_in_dotdrop or \
-               must_ignore([abspath1, abspath2],
-                           ignore, debug=self.debug):
-                continue
-            ret.append(f'=> \"{i}\" does not exist on destination\n')
-
-        # handle files and subdirs only in dotpath dir
-        self.log.dbg(f'files/dirs only in dotpath dir: {comp.right_only}')
-        for i in comp.right_only:
-            abspath1 = os.path.join(local_path, i)
-            if os.path.isdir(abspath1):
-                abspath1 += os.path.sep
-            abspath2 = os.path.join(deployed_path, i)
-            if os.path.isdir(abspath2):
-                abspath2 += os.path.sep
-            if must_ignore([abspath1, abspath2],
-                           ignore, debug=self.debug):
-                continue
 
-            if not self.ignore_missing_in_dotdrop:
-                ret.append(f'=> \"{i}\" does not exist in dotdrop\n')
+        local_tree = FTreeDir(local_path, ignores=ignore, debug=self.debug)
+        deploy_tree = FTreeDir(deployed_path, ignores=ignore, debug=self.debug)
+        lonly, ronly, common = local_tree.compare(deploy_tree)
 
-        # same local_path and deployed_path but different type
-        funny = comp.common_funny
-        self.log.dbg(f'files with different types: {funny}')
-        for i in funny:
-            source_file = os.path.join(local_path, i)
-            deployed_file = os.path.join(deployed_path, i)
-            if self.ignore_missing_in_dotdrop and \
-                    not os.path.exists(source_file):
-                continue
-            if must_ignore([source_file, deployed_file],
-                           ignore, debug=self.debug):
+        for i in lonly:
+            path = os.path.join(local_path, i)
+            if os.path.isdir(path):
+                # ignore dir
                 continue
-            short = os.path.basename(source_file)
-            # file vs dir
-            ret.append(f'=> different type: \"{short}\"\n')
-
-        # content is different
-        funny = comp.diff_files
-        funny.extend(comp.funny_files)
-        funny = uniq_list(funny)
-        self.log.dbg(f'files with different content: {funny}')
-        for i in funny:
+            ret.append(f'=> \"{path}\" does not exist on destination\n')
+        if not self.ignore_missing_in_dotdrop:
+            for i in ronly:
+                path = os.path.join(deployed_path, i)
+                if os.path.isdir(path):
+                    # ignore dir
+                    continue
+                ret.append(f'=> \"{path}\" does not exist in dotdrop\n')
+
+        # test for content difference
+        # and mode difference
+        self.log.dbg(f'common files {common}')
+        for i in common:
             source_file = os.path.join(local_path, i)
             deployed_file = os.path.join(deployed_path, i)
-            if self.ignore_missing_in_dotdrop and \
-                    not os.path.exists(source_file):
-                continue
-            if must_ignore([source_file, deployed_file],
-                           ignore, debug=self.debug):
-                continue
-            ret.append(self._diff(source_file, deployed_file, header=True))
-
-        # recursively compare subdirs
-        for i in comp.common_dirs:
-            sublocal_path = os.path.join(local_path, i)
-            subdeployed_path = os.path.join(deployed_path, i)
-            ret.extend(self._comp_dir(sublocal_path, subdeployed_path, ignore))
+            subret = self._compare(source_file, deployed_file,
+                                   ignore=None, mode=None,
+                                   recurse=False)
+            ret.extend(subret)
 
         return ''.join(ret)
 
     def _diff(self, local_path, deployed_path, header=False):
         """diff two files"""
         out = diff(modified=local_path, original=deployed_path,
                    diff_cmd=self.diff_cmd, debug=self.debug)
-        if header:
+        if header and out:
             lshort = os.path.basename(local_path)
             out = f'=> diff \"{lshort}\":\n{out}'
         return out

dotdrop/dotdrop.py

@@ -22,7 +22,7 @@
 from dotdrop.importer import Importer
 from dotdrop.utils import get_tmpdir, removepath, \
     uniq_list, ignores_to_absolute, dependencies_met, \
-    adapt_workers, check_version, pivot_path
+    adapt_workers, check_version, pivot_path, dir_empty
 from dotdrop.linktypes import LinkTypes
 from dotdrop.exceptions import YamlException, \
     UndefinedException, UnmetDependency, \
@@ -626,7 +626,7 @@ def cmd_uninstall(opts):
     keys = opts.uninstall_key
 
     if keys:
-        # update only specific keys for this profile
+        # uninstall only specific keys for this profile
         dotfiles = []
         for key in uniq_list(keys):
             dotfile = opts.conf.get_dotfile(key)
@@ -726,7 +726,7 @@ def cmd_remove(opts):
             parent = os.path.dirname(dtpath)
             # remove any empty parent up to dotpath
             while parent != opts.dotpath:
-                if os.path.isdir(parent) and not os.listdir(parent):
+                if os.path.isdir(parent) and dir_empty(parent):
                     msg = f'Remove empty dir \"{parent}\"'
                     if opts.safe and not LOG.ask(msg):
                         break

dotdrop/ftree.py

@@ -0,0 +1,79 @@
+"""
+author: deadc0de6 (https://github.com/deadc0de6)
+Copyright (c) 2024, deadc0de6
+
+filesystem tree for directories
+"""
+
+
+import os
+
+# local imports
+from dotdrop.utils import must_ignore, dir_empty
+from dotdrop.logger import Logger
+
+
+class FTreeDir:
+    """
+    directory tree for comparison
+    """
+
+    def __init__(self, path, ignores=None, debug=False):
+        self.path = path
+        self.ignores = ignores
+        self.debug = debug
+        self.entries = []
+        self.log = Logger(debug=self.debug)
+        if os.path.exists(path) and os.path.isdir(path):
+            self._walk()
+
+    def _walk(self):
+        """
+        index directory
+        ignore empty directory
+        test for ignore pattern
+        """
+        for root, dirs, files in os.walk(self.path, followlinks=True):
+            for file in files:
+                fpath = os.path.join(root, file)
+                if must_ignore([fpath], ignores=self.ignores,
+                               debug=self.debug, strict=True):
+                    self.log.dbg(f'ignoring file {fpath}')
+                    continue
+                self.log.dbg(f'added file to list of {self.path}: {fpath}')
+                self.entries.append(fpath)
+            for dname in dirs:
+                dpath = os.path.join(root, dname)
+                if dir_empty(dpath):
+                    # ignore empty directory
+                    self.log.dbg(f'ignoring empty dir {dpath}')
+                    continue
+                # appending "/" allows to ensure pattern
+                # like "*/dir/*" will match the content of the directory
+                # but also the directory itself
+                dpath += os.path.sep
+                if must_ignore([dpath], ignores=self.ignores,
+                               debug=self.debug, strict=True):
+                    self.log.dbg(f'ignoring dir {dpath}')
+                    continue
+                self.log.dbg(f'added dir to list of {self.path}: {dpath}')
+                self.entries.append(dpath)
+
+    def get_entries(self):
+        """return all entries"""
+        return self.entries
+
+    def compare(self, other):
+        """
+        compare two trees and returns
+        - left_only (only in self)
+        - right_only (only in other)
+        - in_both (in both)
+        the relative path are returned
+        """
+        left = [os.path.relpath(entry, self.path) for entry in self.entries]
+        right = [os.path.relpath(entry, other.path) for entry in other.entries]
+        left_only = set(left) - set(right)
+        right_only = set(right) - set(left)
+        in_both = set(left) & set(right)
+        return list(left_only), list(right_only), list(in_both)