refactor installer

docs/config.md

@@ -77,8 +77,6 @@ On `import` the following rules are applied:
 On `install` the following rules are applied:
 
 * if `chmod` is specified in the dotfile, it will be applied to the installed dotfile
-* if no `chmod` is specified and the file exists on the filesystem with different permissions than the file in the `dotpath`
-  then the user needs to confirm the dotfile installation (unless `-f --force` is used)
 
 On `update`:
 

dotdrop/dotdrop.py

@@ -198,17 +198,19 @@ def _dotfile_install(o, dotfile, tmpdir=None):
 
     if hasattr(dotfile, 'link') and dotfile.link == LinkTypes.LINK:
         # link
-        r, err = inst.link(t, dotfile.src, dotfile.dst,
+        r, err = inst.install(t, dotfile.src, dotfile.dst,
-                           actionexec=pre_actions_exec,
+                              dotfile.link,
-                           template=dotfile.template,
+                              actionexec=pre_actions_exec,
-                           chmod=dotfile.chmod)
+                              template=dotfile.template,
+                              chmod=dotfile.chmod)
     elif hasattr(dotfile, 'link') and \
             dotfile.link == LinkTypes.LINK_CHILDREN:
         # link_children
-        r, err = inst.link_children(t, dotfile.src, dotfile.dst,
+        r, err = inst.install(t, dotfile.src, dotfile.dst,
-                                    actionexec=pre_actions_exec,
+                              dotfile.link,
-                                    template=dotfile.template,
+                              actionexec=pre_actions_exec,
-                                    chmod=dotfile.chmod)
+                              template=dotfile.template,
+                              chmod=dotfile.chmod)
     else:
         # nolink
         src = dotfile.src
@@ -221,6 +223,7 @@ def _dotfile_install(o, dotfile, tmpdir=None):
         ignores = list(set(o.install_ignore + dotfile.instignore))
         ignores = patch_ignores(ignores, dotfile.dst, debug=o.debug)
         r, err = inst.install(t, src, dotfile.dst,
+                              LinkTypes.NOLINK,
                               actionexec=pre_actions_exec,
                               noempty=dotfile.noempty,
                               ignore=ignores,

dotdrop/dotfile.py

@@ -115,7 +115,8 @@ class Dotfile(DictParser):
         msg += ', dst:\"{}\"'.format(self.dst)
         msg += ', link:\"{}\"'.format(str(self.link))
         msg += ', template:{}'.format(self.template)
-        msg += ', chmod:{}'.format(self.chmod)
+        if self.chmod:
+            msg += ', chmod:{:o}'.format(self.chmod)
         return msg
 
     def prt(self):
@@ -126,7 +127,8 @@ class Dotfile(DictParser):
         out += '\n{}dst: \"{}\"'.format(indent, self.dst)
         out += '\n{}link: \"{}\"'.format(indent, str(self.link))
         out += '\n{}template: \"{}\"'.format(indent, str(self.template))
-        out += '\n{}chmod: \"{}\"'.format(indent, str(self.chmod))
+        if self.chmod:
+            out += '\n{}chmod: \"{:o}\"'.format(indent, self.chmod)
 
         out += '\n{}pre-action:'.format(indent)
         some = self.get_pre_actions()

dotdrop/installer.py

@@ -11,6 +11,7 @@ import shutil
 
 # local imports
 from dotdrop.logger import Logger
+from dotdrop.linktypes import LinkTypes
 from dotdrop.templategen import Templategen
 import dotdrop.utils as utils
 from dotdrop.exceptions import UndefinedException
@@ -63,16 +64,17 @@ class Installer:
     # public methods
     ########################################################
 
-    def install(self, templater, src, dst,
+    def install(self, templater, src, dst, linktype,
                 actionexec=None, noempty=False,
                 ignore=[], template=True,
                 chmod=None):
         """
-        install src to dst using a templater
+        install src to dst
 
         @templater: the templater object
         @src: dotfile source path in dotpath
         @dst: dotfile destination path in the FS
+        @linktype: linktypes.LinkTypes
         @actionexec: action executor callback
         @noempty: render empty template flag
         @ignore: pattern to ignore when installing
@@ -85,77 +87,79 @@ class Installer:
         - False, None       : ignored
         """
         if self.debug:
-            self.log.dbg('installing \"{}\" to \"{}\"'.format(src, dst))
+            msg = 'installing \"{}\" to \"{}\" (link: {})'
+            self.log.dbg(msg.format(src, dst, str(linktype)))
         src, dst, cont, err = self._check_paths(src, dst, chmod)
         if not cont:
             return self._log_install(cont, err)
 
-        r, err = self._install(templater, src, dst,
+        # check source file exists
-                               actionexec=actionexec,
+        src = os.path.join(self.base, src)
-                               noempty=noempty,
+        if not os.path.exists(src):
-                               ignore=ignore,
+            err = 'source dotfile does not exist: {}'.format(src)
-                               template=template,
+            return self._log_install(False, err)
-                               chmod=chmod)
 
-        return self._log_install(r, err)
+        self.action_executed = False
 
-    def link(self, templater, src, dst, actionexec=None,
+        # install to temporary dir
-             template=True, chmod=None):
+        # and ignore any actions
-        """
+        if self.totemp:
-        set src as the link target of dst
+            r, err, _ = self.install_to_temp(templater, self.totemp,
+                                             src, dst, template=template,
+                                             chmod=chmod)
+            return self._log_install(r, err)
 
-        @templater: the templater
+        isdir = os.path.isdir(src)
-        @src: dotfile source path in dotpath
-        @dst: dotfile destination path in the FS
-        @actionexec: action executor callback
-        @template: template this dotfile
-        @chmod: rights to apply if any
-
-        return
-        - True, None        : success
-        - False, error_msg  : error
-        - False, None       : ignored
-        """
         if self.debug:
-            self.log.dbg('link \"{}\" to \"{}\"'.format(src, dst))
+            self.log.dbg('install {} to {}'.format(src, dst))
-        src, dst, cont, err = self._check_paths(src, dst, chmod)
+            self.log.dbg('\"{}\" is a directory: {}'.format(src, isdir))
-        if not cont:
-            return self._log_install(cont, err)
 
-        r, err = self._link(templater, src, dst,
+        # TODO remove template and set templater
-                            actionexec=actionexec,
+        # to None when no template should occur
-                            template=template)
+        if linktype == LinkTypes.NOLINK:
+            # normal file
+            if isdir:
+                r, err = self._copy_dir(templater, src, dst,
+                                        actionexec=actionexec,
+                                        noempty=noempty, ignore=ignore,
+                                        template=template,
+                                        chmod=chmod)
+            else:
+                r, err = self._copy_file(templater, src, dst,
+                                         actionexec=actionexec,
+                                         noempty=noempty, ignore=ignore,
+                                         template=template,
+                                         chmod=chmod)
+        elif linktype == LinkTypes.LINK:
+            # symlink
+            r, err = self._link(templater, src, dst,
+                                actionexec=actionexec,
+                                template=template)
+        elif linktype == LinkTypes.LINK_CHILDREN:
+            # symlink direct children
+            if not isdir:
+                if self.debug:
+                    msg = 'symlink children of {} to {}'
+                    self.log.dbg(msg.format(src, dst))
+                err = 'source dotfile is not a directory: {}'.format(src)
+                r = False
+            else:
+                r, err = self._link_children(templater, src, dst,
+                                             actionexec=actionexec,
+                                             template=template)
+
+        # handle chmod
         if chmod and not self.dry:
-            # apply mode
+            dstperms = utils.get_file_perm(dst)
-            utils.chmod(dst, chmod, self.debug)
+            if dstperms != chmod:
-        return self._log_install(r, err)
+                # apply mode
+                self.log.sub('chmod {} to {:o}'.format(dst, chmod))
+                if utils.chmod(dst, chmod, debug=self.debug):
+                    r = True
+                else:
+                    r = False
+                    err = 'chmod failed'
 
-    def link_children(self, templater, src, dst, actionexec=None,
-                      template=True, chmod=None):
-        """
-        link all files under a given directory
-
-        @templater: the templater
-        @src: dotfile source path in dotpath
-        @dst: dotfile destination path in the FS
-        @actionexec: action executor callback
-        @template: template this dotfile
-        @chmod: file mode to apply
-
-        return
-        - True, None: success
-        - False, error_msg: error
-        - False, None, ignored
-        """
-        if self.debug:
-            self.log.dbg('link_children \"{}\" to \"{}\"'.format(src, dst))
-        src, dst, cont, err = self._check_paths(src, dst, chmod)
-        if not cont:
-            return self._log_install(cont, err)
-
-        r, err = self._link_children(templater, src, dst,
-                                     actionexec=actionexec,
-                                     template=template, chmod=chmod)
         return self._log_install(r, err)
 
     def install_to_temp(self, templater, tmpdir, src, dst,
@@ -190,10 +194,13 @@ class Installer:
         self.diff = False
         createsaved = self.create
         self.create = True
+        totemp = self.totemp
+        self.totemp = None
 
         # install the dotfile to a temp directory
         tmpdst = self._pivot_path(dst, tmpdir)
         ret, err = self.install(templater, src, tmpdst,
+                                LinkTypes.NOLINK,
                                 template=template, chmod=chmod)
         if self.debug:
             if ret:
@@ -204,101 +211,35 @@ class Installer:
         self.diff = diffsaved
         self.create = createsaved
         self.comparing = False
+        self.totemp = totemp
+
         return ret, err, tmpdst
 
     ########################################################
     # low level accessors for public methods
     ########################################################
 
-    def _install(self, templater, src, dst,
+    def _link(self, templater, src, dst, actionexec=None, template=True):
-                 actionexec=None, noempty=False,
-                 ignore=[], template=True,
-                 chmod=None):
-        """install link:nolink"""
-        self.action_executed = False
-
-        # check source file exists
-        src = os.path.join(self.base, src)
-        if not os.path.exists(src):
-            err = 'source dotfile does not exist: {}'.format(src)
-            return False, err
-
-        # check dst is valid
-        if self.totemp:
-            dst = self._pivot_path(dst, self.totemp)
-        if utils.samefile(src, dst):
-            # symlink loop
-            err = 'dotfile points to itself: {}'.format(dst)
-            return False, err
-
-        isdir = os.path.isdir(src)
-        if self.debug:
-            self.log.dbg('install {} to {}'.format(src, dst))
-            self.log.dbg('\"{}\" is a directory: {}'.format(src, isdir))
-        if isdir:
-            b, e = self._copy_dir(templater, src, dst,
-                                  actionexec=actionexec,
-                                  noempty=noempty, ignore=ignore,
-                                  template=template,
-                                  chmod=chmod)
-            return b, e
-        b, e = self._copy_file(templater, src, dst,
-                               actionexec=actionexec,
-                               noempty=noempty, ignore=ignore,
-                               template=template,
-                               chmod=chmod)
-        return b, e
-
-    def _link(self, templater, src, dst, actionexec=None,
-              template=True):
         """install link:link"""
-        self.action_executed = False
-        src = os.path.join(self.base, src)
-        if not os.path.exists(src):
-            err = 'source dotfile does not exist: {}'.format(src)
-            return False, err
-        if self.totemp:
-            # ignore actions
-            b, e = self.install(templater, src, dst, actionexec=None,
-                                template=template)
-            return b, e
-
         if template and Templategen.is_template(src):
             if self.debug:
-                self.log.dbg('dotfile is a template')
+                self.log.dbg('is a template')
-                self.log.dbg('install to {} and symlink'.format(self.workdir))
+                self.log.dbg('install to {}'.format(self.workdir))
             tmp = self._pivot_path(dst, self.workdir, striphome=True)
-            i, err = self.install(templater, src, tmp, actionexec=actionexec,
+            r, err = self.install(templater, src, tmp,
+                                  LinkTypes.NOLINK,
+                                  actionexec=actionexec,
                                   template=template)
-            if not i and not os.path.exists(tmp):
+            if not r and not os.path.exists(tmp):
-                return i, err
+                return r, err
             src = tmp
-        b, e = self._symlink(src, dst, actionexec=actionexec)
+        r, err = self._symlink(src, dst, actionexec=actionexec)
-        return b, e
+        return r, err
 
-    def _link_children(self, templater, src, dst, actionexec=None,
+    def _link_children(self, templater, src, dst,
-                       template=True, chmod=None):
+                       actionexec=None, template=True):
         """install link:link_children"""
-        if not dst or not src:
-            if self.debug:
-                self.log.dbg('empty dst for {}'.format(src))
-            return True, None
-        self.action_executed = False
         parent = os.path.join(self.base, src)
-
-        # Fail if source doesn't exist
-        if not os.path.exists(parent):
-            err = 'source dotfile does not exist: {}'.format(parent)
-            return False, err
-
-        # Fail if source not a directory
-        if not os.path.isdir(parent):
-            if self.debug:
-                self.log.dbg('symlink children of {} to {}'.format(src, dst))
-
-            err = 'source dotfile is not a directory: {}'.format(parent)
-            return False, err
-
         if not os.path.lexists(dst):
             self.log.sub('creating directory "{}"'.format(dst))
             os.makedirs(dst)
@@ -313,7 +254,7 @@ class Installer:
                 err = 'ignoring "{}", not installed'.format(dst)
                 return False, err
             os.unlink(dst)
-            os.mkdir(dst)
+            self._create_dirs(dst)
 
         children = os.listdir(parent)
         srcs = [os.path.normpath(os.path.join(parent, child))
@@ -331,11 +272,12 @@ class Installer:
 
             if template and Templategen.is_template(subsrc):
                 if self.debug:
-                    self.log.dbg('dotfile is a template')
+                    self.log.dbg('child is a template')
                     self.log.dbg('install to {} and symlink'
                                  .format(self.workdir))
                 tmp = self._pivot_path(subdst, self.workdir, striphome=True)
                 r, e = self.install(templater, subsrc, tmp,
+                                    LinkTypes.NOLINK,
                                     actionexec=actionexec,
                                     template=template)
                 if not r and e and not os.path.exists(tmp):
@@ -352,10 +294,6 @@ class Installer:
                 if err:
                     return ret, err
 
-        if chmod and not self.dry:
-            # apply mode
-            utils.chmod(dst, chmod, debug=self.debug)
-
         return installed > 0, None
 
     ########################################################
@@ -430,6 +368,11 @@ class Installer:
             self.log.dbg('template: {}'.format(template))
             self.log.dbg('no empty: {}'.format(noempty))
 
+        # check no loop
+        if utils.samefile(src, dst):
+            err = 'dotfile points to itself: {}'.format(dst)
+            return False, err
+
         if utils.must_ignore([src, dst], ignore, debug=self.debug):
             if self.debug:
                 self.log.dbg('ignoring install of {} to {}'.format(src, dst))
@@ -483,7 +426,7 @@ class Installer:
         if ret == 0:
             # success
             if not self.dry and not self.comparing:
-                self.log.sub('copied {} to {}'.format(src, dst))
+                self.log.sub('install {} to {}'.format(src, dst))
             return True, None
         # error
         err = 'installing {} to {}'.format(src, dst)
@@ -499,8 +442,6 @@ class Installer:
         ret = False, None
 
         # create the directory anyway
-        if self.debug:
-            self.log.dbg('mkdir -p {}'.format(dst))
         if not self._create_dirs(dst):
             err = 'creating directory for {}'.format(dst)
             return False, err
@@ -542,10 +483,6 @@ class Installer:
                 elif res:
                     # something got installed
                     ret = True, None
-
-        if chmod and not self.dry:
-            # apply mode
-            utils.chmod(dst, chmod, debug=self.debug)
         return ret
 
     def _write(self, src, dst, content=None,
@@ -629,9 +566,6 @@ class Installer:
                 shutil.copymode(src, dst)
             except Exception as e:
                 return -1, str(e)
-
-        if chmod:
-            utils.chmod(dst, chmod, debug=self.debug)
         return 0, None
 
     ########################################################
@@ -714,6 +648,7 @@ class Installer:
             return True
         if self.debug:
             self.log.dbg('mkdir -p {}'.format(directory))
+        self.log.sub('create directory {}'.format(directory))
 
         os.makedirs(directory, exist_ok=True)
         return os.path.exists(directory)
@@ -762,32 +697,6 @@ class Installer:
                 self.log.dbg('install: IGNORED')
         return boolean, err
 
-    def _check_chmod(self, src, dst, chmod):
-        """
-        check chmod needs change
-        returns True to continue installation
-        """
-        if chmod:
-            return True
-        if not os.path.exists(dst):
-            return True
-        if not os.path.exists(src):
-            return True
-        sperms = utils.get_file_perm(src)
-        dperms = utils.get_file_perm(dst)
-        if sperms == dperms:
-            return True
-        elif self.safe:
-            # this only happens if no
-            # chmod is provided
-            # and dst/src modes differ
-            if self.safe:
-                msg = 'Set mode {:o} to \"{}\"'
-                msg = msg.format(sperms, dst)
-                if not self.log.ask(msg):
-                    return False
-        return True
-
     def _check_paths(self, src, dst, chmod):
         """
         check and normalize param
@@ -807,9 +716,4 @@ class Installer:
         dst = os.path.expanduser(dst)
         dst = os.path.normpath(dst)
 
-        # check chmod
-        if not self._check_chmod(src, dst, chmod):
-            err = 'ignoring "{}", not installed'.format(dst)
-            return None, None, False, err
-
         return src, dst, True, None

dotdrop/utils.py

@@ -330,3 +330,4 @@ def chmod(path, mode, debug=False):
     if debug:
         LOG.dbg('chmod {} {}'.format(oct(mode), path))
     os.chmod(path, mode)
+    return get_file_perm(path) == mode

tests-ng/chmod-install.sh

@@ -17,7 +17,7 @@ has_rights()
   echo "testing ${1} is ${2}"
   [ ! -e "$1" ] && echo "`basename $1` does not exist" && exit 1
   local mode=`stat -L -c '%a' "$1"`
-  [ "${mode}" != "$2" ] && echo "bad mode for `basename $1`" && exit 1
+  [ "${mode}" != "$2" ] && echo "bad mode for `basename $1` (${mode} VS expected ${2})" && exit 1
   true
 }
 

tests-ng/diff-cmd.sh

@@ -81,7 +81,7 @@ echo "modified" > ${tmpd}/singlefile
 # default diff (unified)
 echo "[+] comparing with default diff (unified)"
 set +e
-cd ${ddpath} | ${bin} compare -c ${cfg} 2>&1 | grep -v '=>' | grep -v 'dotfile(s) compared' | sed '$d' | grep -v '^+++\|^---' > ${tmpd}/normal
+cd ${ddpath} | ${bin} compare -c ${cfg} 2>&1 | grep -v '=>' | grep -v '\->' | grep -v 'dotfile(s) compared' | sed '$d' | grep -v '^+++\|^---' > ${tmpd}/normal
 diff -u -r ${tmpd}/singlefile ${basedir}/dotfiles/${tmpd}/singlefile | grep -v '^+++\|^---' > ${tmpd}/real
 set -e
 
@@ -96,7 +96,7 @@ sed '/dotpath: dotfiles/a \ \ diff_command: "diff -r {0} {1}"' ${cfg} > ${cfg2}
 # normal diff
 echo "[+] comparing with normal diff"
 set +e
-cd ${ddpath} | ${bin} compare -c ${cfg2} 2>&1 | grep -v '=>' | grep -v 'dotfile(s) compared' | sed '$d' > ${tmpd}/unified
+cd ${ddpath} | ${bin} compare -c ${cfg2} 2>&1 | grep -v '=>' | grep -v '\->' | grep -v 'dotfile(s) compared' | sed '$d' > ${tmpd}/unified
 diff -r ${tmpd}/singlefile ${basedir}/dotfiles/${tmpd}/singlefile > ${tmpd}/real
 set -e
 
@@ -113,7 +113,7 @@ sed '/dotpath: dotfiles/a \ \ diff_command: "echo fakediff"' ${cfg} > ${cfg3}
 # fake diff
 echo "[+] comparing with fake diff"
 set +e
-cd ${ddpath} | ${bin} compare -c ${cfg3} 2>&1 | grep -v '=>' | grep -v 'dotfile(s) compared' | sed '$d' > ${tmpd}/fake
+cd ${ddpath} | ${bin} compare -c ${cfg3} 2>&1 | grep -v '=>' | grep -v '\->' | grep -v 'dotfile(s) compared' | sed '$d' > ${tmpd}/fake
 set -e
 
 # verify

tests/test_install.py

@@ -349,8 +349,9 @@ exec bspwm
         srcs = [create_random_file(src_dir)[0] for _ in range(3)]
 
         installer = Installer()
-        installer.link_children(templater=MagicMock(), src=src_dir,
+        installer.install(templater=MagicMock(), src=src_dir,
-                                dst=dst_dir, actionexec=None)
+                          dst=dst_dir, linktype=LinkTypes.LINK_CHILDREN,
+                          actionexec=None)
 
         # Ensure all destination files point to source
         for src in srcs:
@@ -365,8 +366,10 @@ exec bspwm
         # logger = MagicMock()
         # installer.log.err = logger
 
-        res, err = installer.link_children(templater=MagicMock(), src=src,
+        res, err = installer.install(templater=MagicMock(), src=src,
-                                           dst='/dev/null', actionexec=None)
+                                     dst='/dev/null',
+                                     linktype=LinkTypes.LINK_CHILDREN,
+                                     actionexec=None)
 
         self.assertFalse(res)
         e = 'source dotfile does not exist: {}'.format(src)
@@ -387,8 +390,10 @@ exec bspwm
         # installer.log.err = logger
 
         # pass src file not src dir
-        res, err = installer.link_children(templater=templater, src=src,
+        res, err = installer.install(templater=templater, src=src,
-                                           dst='/dev/null', actionexec=None)
+                                     dst='/dev/null',
+                                     linktype=LinkTypes.LINK_CHILDREN,
+                                     actionexec=None)
 
         # ensure nothing performed
         self.assertFalse(res)
@@ -410,8 +415,9 @@ exec bspwm
         self.assertFalse(os.path.exists(dst_dir))
 
         installer = Installer()
-        installer.link_children(templater=MagicMock(), src=src_dir,
+        installer.install(templater=MagicMock(), src=src_dir,
-                                dst=dst_dir, actionexec=None)
+                          dst=dst_dir, linktype=LinkTypes.LINK_CHILDREN,
+                          actionexec=None)
 
         # ensure dst dir created
         self.assertTrue(os.path.exists(dst_dir))
@@ -442,8 +448,9 @@ exec bspwm
         installer.safe = True
         installer.log.ask = ask
 
-        installer.link_children(templater=MagicMock(), src=src_dir, dst=dst,
+        installer.install(templater=MagicMock(), src=src_dir,
-                                actionexec=None)
+                          dst=dst, linktype=LinkTypes.LINK_CHILDREN,
+                          actionexec=None)
 
         # ensure destination now a directory
         self.assertTrue(os.path.isdir(dst))
@@ -476,8 +483,8 @@ exec bspwm
         # make templategen treat everything as a template
         mocked_templategen.is_template.return_value = True
 
-        installer.link_children(templater=templater, src=src_dir, dst=dst_dir,
+        installer.install(templater=templater, src=src_dir, dst=dst_dir,
-                                actionexec=None)
+                          linktype=LinkTypes.LINK_CHILDREN, actionexec=None)
 
         for src in srcs:
             dst = os.path.join(dst_dir, os.path.basename(src))