From 228524bdf3881219021c7d46347d2af5dd7bc24f Mon Sep 17 00:00:00 2001
From: deadc0de6 <info@deadc0de.ch>
Date: Fri, 29 Mar 2019 22:57:58 +0100
Subject: [PATCH 1/8] implement import external actions for #113

---
 dotdrop/config.py       |  82 ++++++++++++++++------------
 tests-ng/ext-actions.sh | 115 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 163 insertions(+), 34 deletions(-)
 create mode 100755 tests-ng/ext-actions.sh

diff --git a/dotdrop/config.py b/dotdrop/config.py
index d3a8ae5..da8167c 100644
--- a/dotdrop/config.py
+++ b/dotdrop/config.py
@@ -33,7 +33,8 @@ class Cfg:
     key_showdiff = 'showdiff'
     key_deflink = 'link_by_default'
     key_workdir = 'workdir'
-    key_include_vars = 'import_variables'
+    key_import_vars = 'import_variables'
+    key_import_actions = 'import_actions'
 
     # actions keys
     key_actions = 'actions'
@@ -216,45 +217,36 @@ class Cfg:
             self._abs_path(self.curworkdir)
 
         # load external variables/dynvariables
-        if self.key_include_vars in self.lnk_settings:
-            paths = self.lnk_settings[self.key_include_vars]
+        if self.key_import_vars in self.lnk_settings:
+            paths = self.lnk_settings[self.key_import_vars]
             self._load_ext_variables(paths, profile=profile)
 
-        # parse all actions
-        if self.key_actions in self.content:
-            if self.content[self.key_actions] is not None:
-                for k, v in self.content[self.key_actions].items():
-                    # loop through all actions
-                    if k in [self.key_actions_pre, self.key_actions_post]:
-                        # parse pre/post actions
-                        items = self.content[self.key_actions][k].items()
-                        for k2, v2 in items:
-                            if k not in self.actions:
-                                self.actions[k] = {}
-                            a = Action(k2, k, v2)
-                            self.actions[k][k2] = a
-                            if self.debug:
-                                self.log.dbg('new action: {}'.format(a))
-                    else:
-                        # parse naked actions as post actions
-                        if self.key_actions_post not in self.actions:
-                            self.actions[self.key_actions_post] = {}
-                        a = Action(k, '', v)
-                        self.actions[self.key_actions_post][k] = a
-                        if self.debug:
-                            self.log.dbg('new action: {}'.format(a))
+        # parse external actions
+        if self.key_import_actions in self.lnk_settings:
+            for path in self.lnk_settings[self.key_import_actions]:
+                if self.debug:
+                    self.log.dbg('loading actions from {}'.format(path))
+                content = self._load_yaml(path)
+                if self.key_actions in content and \
+                        content[self.key_actions] is not None:
+                    self._load_actions(content[self.key_actions])
+
+        # parse local actions
+        if self.key_actions in self.content and \
+                self.content[self.key_actions] is not None:
+            self._load_actions(self.content[self.key_actions])
 
         # parse read transformations
-        if self.key_trans_r in self.content:
-            if self.content[self.key_trans_r] is not None:
-                for k, v in self.content[self.key_trans_r].items():
-                    self.trans_r[k] = Transform(k, v)
+        if self.key_trans_r in self.content and \
+                self.content[self.key_trans_r] is not None:
+            for k, v in self.content[self.key_trans_r].items():
+                self.trans_r[k] = Transform(k, v)
 
         # parse write transformations
-        if self.key_trans_w in self.content:
-            if self.content[self.key_trans_w] is not None:
-                for k, v in self.content[self.key_trans_w].items():
-                    self.trans_w[k] = Transform(k, v)
+        if self.key_trans_w in self.content and \
+                self.content[self.key_trans_w] is not None:
+            for k, v in self.content[self.key_trans_w].items():
+                self.trans_w[k] = Transform(k, v)
 
         # parse the dotfiles
         # and construct the dict of objects per dotfile key
@@ -425,6 +417,28 @@ class Cfg:
         if self.debug:
             self.log.dbg('loaded ext dynvariables: {}'.format(dvariables))
 
+    def _load_actions(self, dic):
+        for k, v in dic.items():
+            # loop through all actions
+            if k in [self.key_actions_pre, self.key_actions_post]:
+                # parse pre/post actions
+                items = dic[k].items()
+                for k2, v2 in items:
+                    if k not in self.actions:
+                        self.actions[k] = {}
+                    a = Action(k2, k, v2)
+                    self.actions[k][k2] = a
+                    if self.debug:
+                        self.log.dbg('new action: {}'.format(a))
+            else:
+                # parse naked actions as post actions
+                if self.key_actions_post not in self.actions:
+                    self.actions[self.key_actions_post] = {}
+                a = Action(k, '', v)
+                self.actions[self.key_actions_post][k] = a
+                if self.debug:
+                    self.log.dbg('new action: {}'.format(a))
+
     def _abs_path(self, path):
         """return absolute path of path relative to the confpath"""
         path = os.path.expanduser(path)
diff --git a/tests-ng/ext-actions.sh b/tests-ng/ext-actions.sh
new file mode 100755
index 0000000..550d69b
--- /dev/null
+++ b/tests-ng/ext-actions.sh
@@ -0,0 +1,115 @@
+#!/usr/bin/env bash
+# author: deadc0de6 (https://github.com/deadc0de6)
+# Copyright (c) 2019, deadc0de6
+#
+# test external actions
+# returns 1 in case of error
+#
+
+# exit on first error
+set -e
+
+# all this crap to get current path
+rl="readlink -f"
+if ! ${rl} "${0}" >/dev/null 2>&1; then
+  rl="realpath"
+
+  if ! hash ${rl}; then
+    echo "\"${rl}\" not found !" && exit 1
+  fi
+fi
+cur=$(dirname "$(${rl} "${0}")")
+
+#hash dotdrop >/dev/null 2>&1
+#[ "$?" != "0" ] && echo "install dotdrop to run tests" && exit 1
+
+#echo "called with ${1}"
+
+# dotdrop path can be pass as argument
+ddpath="${cur}/../"
+[ "${1}" != "" ] && ddpath="${1}"
+[ ! -d ${ddpath} ] && echo "ddpath \"${ddpath}\" is not a directory" && exit 1
+
+export PYTHONPATH="${ddpath}:${PYTHONPATH}"
+bin="python3 -m dotdrop.dotdrop"
+
+echo "dotdrop path: ${ddpath}"
+echo "pythonpath: ${PYTHONPATH}"
+
+# get the helpers
+source ${cur}/helpers
+
+echo -e "\e[96m\e[1m==> RUNNING $(basename $BASH_SOURCE) <==\e[0m"
+
+################################################################
+# this is the test
+################################################################
+
+# the action temp
+tmpa=`mktemp -d --suffix='-dotdrop-tests'`
+# the dotfile source
+tmps=`mktemp -d --suffix='-dotdrop-tests'`
+mkdir -p ${tmps}/dotfiles
+# the dotfile destination
+tmpd=`mktemp -d --suffix='-dotdrop-tests'`
+
+act="${tmps}/actions.yaml"
+cat > ${act} << _EOF
+actions:
+  pre:
+    preaction: echo 'pre' > ${tmpa}/pre
+  post:
+    postaction: echo 'post' > ${tmpa}/post
+  nakedaction: echo 'naked' > ${tmpa}/naked
+  overwrite: echo 'over' > ${tmpa}/write
+_EOF
+
+# create the config file
+cfg="${tmps}/config.yaml"
+
+cat > ${cfg} << _EOF
+config:
+  backup: true
+  create: true
+  dotpath: dotfiles
+  import_actions:
+  - ${tmps}/actions.yaml
+actions:
+  overwrite: echo 'write' > ${tmpa}/write
+dotfiles:
+  f_abc:
+    dst: ${tmpd}/abc
+    src: abc
+    actions:
+      - preaction
+      - postaction
+      - nakedaction
+      - overwrite
+profiles:
+  p1:
+    dotfiles:
+    - f_abc
+_EOF
+#cat ${cfg}
+
+# create the dotfile
+echo "test" > ${tmps}/dotfiles/abc
+
+# install
+cd ${ddpath} | ${bin} install -f -c ${cfg} -p p1
+
+# checks
+[ ! -e ${tmpa}/pre ] && exit 1
+grep pre ${tmpa}/pre >/dev/null
+[ ! -e ${tmpa}/post ] && exit 1
+grep post ${tmpa}/post >/dev/null
+[ ! -e ${tmpa}/naked ] && exit 1
+grep naked ${tmpa}/naked >/dev/null
+[ ! -e ${tmpa}/write ] && exit 1
+grep write ${tmpa}/write >/dev/null
+
+## CLEANING
+rm -rf ${tmps} ${tmpd} ${tmpa}
+
+echo "OK"
+exit 0

From d88f29b27631bc825fdcac445b88a196d19ef43d Mon Sep 17 00:00:00 2001
From: deadc0de6 <info@deadc0de.ch>
Date: Fri, 29 Mar 2019 23:00:22 +0100
Subject: [PATCH 2/8] allow relative load external actions

---
 dotdrop/config.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dotdrop/config.py b/dotdrop/config.py
index da8167c..4dee701 100644
--- a/dotdrop/config.py
+++ b/dotdrop/config.py
@@ -224,6 +224,7 @@ class Cfg:
         # parse external actions
         if self.key_import_actions in self.lnk_settings:
             for path in self.lnk_settings[self.key_import_actions]:
+                path = self._abs_path(path)
                 if self.debug:
                     self.log.dbg('loading actions from {}'.format(path))
                 content = self._load_yaml(path)

From 74054fbf449c59d703f22fdad137d834e48b1ebd Mon Sep 17 00:00:00 2001
From: deadc0de6 <info@deadc0de.ch>
Date: Sun, 31 Mar 2019 22:19:47 +0200
Subject: [PATCH 3/8] better handle error on dotfile installation for #111

---
 dotdrop/dotdrop.py    |  16 ++--
 dotdrop/installer.py  | 190 ++++++++++++++++++++++++------------------
 tests/test_install.py |  21 +++--
 3 files changed, 126 insertions(+), 101 deletions(-)

diff --git a/dotdrop/dotdrop.py b/dotdrop/dotdrop.py
index 7dc04cb..ca26baf 100644
--- a/dotdrop/dotdrop.py
+++ b/dotdrop/dotdrop.py
@@ -51,7 +51,7 @@ def cmd_install(o):
                      totemp=tmpdir,
                      showdiff=o.install_showdiff,
                      backup_suffix=o.install_backup_suffix)
-    installed = []
+    installed = 0
     for dotfile in dotfiles:
         preactions = []
         if not o.install_temporary and dotfile.actions \
@@ -73,13 +73,13 @@ def cmd_install(o):
                 if not tmp:
                     continue
                 src = tmp
-            r = inst.install(t, src, dotfile.dst, actions=preactions,
-                             noempty=dotfile.noempty)
+            r, err = inst.install(t, src, dotfile.dst, actions=preactions,
+                                  noempty=dotfile.noempty)
             if tmp:
                 tmp = os.path.join(o.dotpath, tmp)
                 if os.path.exists(tmp):
                     remove(tmp)
-        if len(r) > 0:
+        if r:
             if not o.install_temporary and \
                     Cfg.key_actions_post in dotfile.actions:
                 actions = dotfile.actions[Cfg.key_actions_post]
@@ -91,10 +91,12 @@ def cmd_install(o):
                         if o.debug:
                             LOG.dbg('executing post action {}'.format(action))
                         action.execute()
-        installed.extend(r)
+            installed += 1
+        elif not r and err:
+            LOG.err('installing \"{}\" failed: {}'.format(dotfile.key, err))
     if o.install_temporary:
-        LOG.log('\nInstalled to tmp \"{}\".'.format(tmpdir))
-    LOG.log('\n{} dotfile(s) installed.'.format(len(installed)))
+        LOG.log('\ninstalled to tmp \"{}\".'.format(tmpdir))
+    LOG.log('\n{} dotfile(s) installed.'.format(installed))
     return True
 
 
diff --git a/dotdrop/installer.py b/dotdrop/installer.py
index 4d371a4..2bdb38f 100644
--- a/dotdrop/installer.py
+++ b/dotdrop/installer.py
@@ -50,21 +50,27 @@ class Installer:
         self.log = Logger()
 
     def install(self, templater, src, dst, actions=[], noempty=False):
-        """install the src to dst using a template"""
+        """
+        install src to dst using a template
+        return
+        - True, None: success
+        - False, error_msg: error
+        - False, None, ignored
+        """
         if self.debug:
             self.log.dbg('install {} to {}'.format(src, dst))
         self.action_executed = False
         src = os.path.join(self.base, os.path.expanduser(src))
         if not os.path.exists(src):
-            self.log.err('source dotfile does not exist: {}'.format(src))
-            return []
+            err = 'source dotfile does not exist: {}'.format(src)
+            return False, err
         dst = os.path.expanduser(dst)
         if self.totemp:
             dst = self._pivot_path(dst, self.totemp)
         if utils.samefile(src, dst):
             # symlink loop
-            self.log.err('dotfile points to itself: {}'.format(dst))
-            return []
+            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))
@@ -76,15 +82,21 @@ class Installer:
                                  actions=actions, noempty=noempty)
 
     def link(self, templater, src, dst, actions=[]):
-        """set src as the link target of dst"""
+        """
+        set src as the link target of dst
+        return
+        - True, None: success
+        - False, error_msg: error
+        - False, None, ignored
+        """
         if self.debug:
             self.log.dbg('link {} to {}'.format(src, dst))
         self.action_executed = False
         src = os.path.normpath(os.path.join(self.base,
                                             os.path.expanduser(src)))
         if not os.path.exists(src):
-            self.log.err('source dotfile does not exist: {}'.format(src))
-            return []
+            err = 'source dotfile does not exist: {}'.format(src)
+            return False, err
         dst = os.path.normpath(os.path.expanduser(dst))
         if self.totemp:
             # ignore actions
@@ -95,14 +107,20 @@ class Installer:
                 self.log.dbg('dotfile is a template')
                 self.log.dbg('install to {} and symlink'.format(self.workdir))
             tmp = self._pivot_path(dst, self.workdir, striphome=True)
-            i = self.install(templater, src, tmp, actions=actions)
+            i, err = self.install(templater, src, tmp, actions=actions)
             if not i and not os.path.exists(tmp):
-                return []
+                return i, err
             src = tmp
         return self._link(src, dst, actions=actions)
 
     def link_children(self, templater, src, dst, actions=[]):
-        """link all dotfiles in a given directory"""
+        """
+        link all dotfiles in a given directory
+        return
+        - True, None: success
+        - False, error_msg: error
+        - False, None, ignored
+        """
         if self.debug:
             self.log.dbg('link_children {} to {}'.format(src, dst))
         self.action_executed = False
@@ -110,17 +128,16 @@ class Installer:
 
         # Fail if source doesn't exist
         if not os.path.exists(parent):
-            self.log.err('source dotfile does not exist: {}'.format(parent))
-            return []
+            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))
 
-            self.log.err('source dotfile is not a directory: {}'
-                         .format(parent))
-            return []
+            err = 'source dotfile is not a directory: {}'.format(parent)
+            return False, err
 
         dst = os.path.normpath(os.path.expanduser(dst))
         if not os.path.lexists(dst):
@@ -134,9 +151,8 @@ class Installer:
             ]).format(dst)
 
             if self.safe and not self.log.ask(msg):
-                msg = 'ignoring "{}", nothing installed'
-                self.log.warn(msg.format(dst))
-                return []
+                err = 'ignoring "{}", nothing installed'.format(dst)
+                return False, err
             os.unlink(dst)
             os.mkdir(dst)
 
@@ -171,54 +187,52 @@ class Installer:
             if len(result):
                 actions = []
 
-        return (src, dst)
+        return True, None
 
     def _link(self, src, dst, actions=[]):
         """set src as a link target of dst"""
         overwrite = not self.safe
         if os.path.lexists(dst):
             if os.path.realpath(dst) == os.path.realpath(src):
-                if self.debug:
-                    self.log.dbg('ignoring "{}", link exists'.format(dst))
-                return []
+                err = 'ignoring "{}", link exists'.format(dst)
+                return False, err
             if self.dry:
                 self.log.dry('would remove {} and link to {}'.format(dst, src))
-                return []
+                return True, None
             msg = 'Remove "{}" for link creation?'.format(dst)
             if self.safe and not self.log.ask(msg):
-                msg = 'ignoring "{}", link was not created'
-                self.log.warn(msg.format(dst))
-                return []
+                err = 'ignoring "{}", link was not created'.format(dst)
+                return False, err
             overwrite = True
             try:
                 utils.remove(dst)
             except OSError as e:
-                self.log.err('something went wrong with {}: {}'.format(src, e))
-                return []
+                err = 'something went wrong with {}: {}'.format(src, e)
+                return False, err
         if self.dry:
             self.log.dry('would link {} to {}'.format(dst, src))
-            return []
+            return True, None
         base = os.path.dirname(dst)
         if not self._create_dirs(base):
-            self.log.err('creating directory for {}'.format(dst))
-            return []
-        if not self._exec_pre_actions(actions):
-            return []
+            err = 'creating directory for {}'.format(dst)
+            return False, err
+        r, e = self._exec_pre_actions(actions)
+        if not r:
+            return False, e
         # re-check in case action created the file
         if os.path.lexists(dst):
             msg = 'Remove "{}" for link creation?'.format(dst)
             if self.safe and not overwrite and not self.log.ask(msg):
-                msg = 'ignoring "{}", link was not created'
-                self.log.warn(msg.format(dst))
-                return []
+                err = 'ignoring "{}", link was not created'.format(dst)
+                return False, err
             try:
                 utils.remove(dst)
             except OSError as e:
-                self.log.err('something went wrong with {}: {}'.format(src, e))
-                return []
+                err = 'something went wrong with {}: {}'.format(src, e)
+                return False, err
         os.symlink(src, dst)
         self.log.sub('linked {} to {}'.format(dst, src))
-        return [(src, dst)]
+        return True, None
 
     def _handle_file(self, templater, src, dst, actions=[], noempty=False):
         """install src to dst when is a file"""
@@ -227,52 +241,61 @@ class Installer:
             self.log.dbg('ignore empty: {}'.format(noempty))
         if utils.samefile(src, dst):
             # symlink loop
-            self.log.err('dotfile points to itself: {}'.format(dst))
-            return []
+            err = 'dotfile points to itself: {}'.format(dst)
+            return False, err
         content = templater.generate(src)
         if noempty and utils.content_empty(content):
-            self.log.warn('ignoring empty template: {}'.format(src))
-            return []
+            self.log.dbg('ignoring empty template: {}'.format(src))
+            return False, None
         if content is None:
-            self.log.err('generate from template {}'.format(src))
-            return []
+            err = 'empty template {}'.format(src)
+            return False, err
         if not os.path.exists(src):
-            self.log.err('source dotfile does not exist: {}'.format(src))
-            return []
+            err = 'source dotfile does not exist: {}'.format(src)
+            return False, err
         st = os.stat(src)
-        ret = self._write(src, dst, content, st.st_mode, actions=actions)
+        ret, err = self._write(src, dst, content, st.st_mode, actions=actions)
         if ret < 0:
-            self.log.err('installing {} to {}'.format(src, dst))
-            return []
+            return False, err
         if ret > 0:
             if self.debug:
                 self.log.dbg('ignoring {}'.format(dst))
-            return []
+            return False, None
         if ret == 0:
             if not self.dry and not self.comparing:
                 self.log.sub('copied {} to {}'.format(src, dst))
-            return [(src, dst)]
-        return []
+            return True, None
+        err = 'installing {} to {}'.format(src, dst)
+        return False, err
 
     def _handle_dir(self, templater, src, dst, actions=[], noempty=False):
         """install src to dst when is a directory"""
         if self.debug:
             self.log.dbg('install dir {}'.format(src))
             self.log.dbg('ignore empty: {}'.format(noempty))
-        ret = []
+        ret = True, None
         if not self._create_dirs(dst):
-            return []
+            err = 'creating directory for {}'.format(dst)
+            return False, err
         # handle all files in dir
         for entry in os.listdir(src):
             f = os.path.join(src, entry)
             if not os.path.isdir(f):
-                res = self._handle_file(templater, f, os.path.join(dst, entry),
-                                        actions=actions, noempty=noempty)
-                ret.extend(res)
+                res, err = self._handle_file(templater, f,
+                                             os.path.join(dst, entry),
+                                             actions=actions,
+                                             noempty=noempty)
+                if not res:
+                    ret = res, err
+                    break
             else:
-                res = self._handle_dir(templater, f, os.path.join(dst, entry),
-                                       actions=actions, noempty=noempty)
-                ret.extend(res)
+                res, err = self._handle_dir(templater, f,
+                                            os.path.join(dst, entry),
+                                            actions=actions,
+                                            noempty=noempty)
+                if not res:
+                    ret = res, err
+                    break
         return ret
 
     def _fake_diff(self, dst, content):
@@ -284,13 +307,13 @@ class Installer:
 
     def _write(self, src, dst, content, rights, actions=[]):
         """write content to file
-        return  0 for success,
-                1 when already exists
-               -1 when error"""
+        return  0, None:  for success,
+                1, None:  when already exists
+               -1, err: when error"""
         overwrite = not self.safe
         if self.dry:
             self.log.dry('would install {}'.format(dst))
-            return 0
+            return 0, None
         if os.path.lexists(dst):
             samerights = False
             try:
@@ -298,12 +321,12 @@ class Installer:
             except OSError as e:
                 if e.errno == errno.ENOENT:
                     # broken symlink
-                    self.log.err('broken symlink {}'.format(dst))
-                    return -1
+                    err = 'broken symlink {}'.format(dst)
+                    return -1, err
             if self.diff and self._fake_diff(dst, content) and samerights:
                 if self.debug:
                     self.log.dbg('{} is the same'.format(dst))
-                return 1
+                return 1, None
             if self.safe:
                 if self.debug:
                     self.log.dbg('change detected for {}'.format(dst))
@@ -311,32 +334,33 @@ class Installer:
                     self._diff_before_write(src, dst, content)
                 if not self.log.ask('Overwrite \"{}\"'.format(dst)):
                     self.log.warn('ignoring {}'.format(dst))
-                    return 1
+                    return 1, None
                 overwrite = True
         if self.backup and os.path.lexists(dst):
             self._backup(dst)
         base = os.path.dirname(dst)
         if not self._create_dirs(base):
-            self.log.err('creating directory for {}'.format(dst))
-            return -1
-        if not self._exec_pre_actions(actions):
-            return -1
+            err = 'creating directory for {}'.format(dst)
+            return -1, err
+        r, e = self._exec_pre_actions(actions)
+        if not r:
+            return -1, e
         if self.debug:
             self.log.dbg('write content to {}'.format(dst))
         # re-check in case action created the file
         if self.safe and not overwrite and os.path.lexists(dst):
             if not self.log.ask('Overwrite \"{}\"'.format(dst)):
                 self.log.warn('ignoring {}'.format(dst))
-                return 1
+                return 1, None
         # write the file
         try:
             with open(dst, 'wb') as f:
                 f.write(content)
         except NotADirectoryError as e:
-            self.log.err('opening dest file: {}'.format(e))
-            return -1
+            err = 'opening dest file: {}'.format(e)
+            return -1, err
         os.chmod(dst, rights)
-        return 0
+        return 0, None
 
     def _diff_before_write(self, src, dst, src_content):
         """diff before writing when using --showdiff - not efficient"""
@@ -355,7 +379,7 @@ class Installer:
     def _create_dirs(self, directory):
         """mkdir -p <directory>"""
         if not self.create and not os.path.exists(directory):
-            return False
+            return False,
         if os.path.exists(directory):
             return True
         if self.dry:
@@ -390,7 +414,7 @@ class Installer:
     def _exec_pre_actions(self, actions):
         """execute pre-actions if any"""
         if self.action_executed:
-            return True
+            return True, None
         for action in actions:
             if self.dry:
                 self.log.dry('would execute action: {}'.format(action))
@@ -398,10 +422,10 @@ class Installer:
                 if self.debug:
                     self.log.dbg('executing pre action {}'.format(action))
                 if not action.execute():
-                    self.log.err('pre-action {} failed'.format(action.key))
-                    return False
+                    err = 'pre-action \"{}\" failed'.format(action.key)
+                    return False, err
         self.action_executed = True
-        return True
+        return True, None
 
     def _install_to_temp(self, templater, src, dst, tmpdir):
         """install a dotfile to a tempdir"""
diff --git a/tests/test_install.py b/tests/test_install.py
index d844322..0eb2972 100644
--- a/tests/test_install.py
+++ b/tests/test_install.py
@@ -266,12 +266,12 @@ exec bspwm
         logger = MagicMock()
         installer.log.err = logger
 
-        res = installer.link_children(templater=MagicMock(), src=src,
-                                      dst='/dev/null', actions=[])
+        res, err = installer.link_children(templater=MagicMock(), src=src,
+                                           dst='/dev/null', actions=[])
 
-        self.assertEqual(res, [])
-        logger.assert_called_with('source dotfile does not exist: {}'
-                                  .format(src))
+        self.assertFalse(res)
+        e = 'source dotfile does not exist: {}'.format(src)
+        self.assertEqual(err, e)
 
     def test_fails_when_src_file(self):
         """test fails when src file"""
@@ -288,14 +288,13 @@ exec bspwm
         installer.log.err = logger
 
         # pass src file not src dir
-        res = installer.link_children(templater=templater, src=src,
-                                      dst='/dev/null', actions=[])
+        res, err = installer.link_children(templater=templater, src=src,
+                                           dst='/dev/null', actions=[])
 
         # ensure nothing performed
-        self.assertEqual(res, [])
-        # ensure logger logged error
-        logger.assert_called_with('source dotfile is not a directory: {}'
-                                  .format(src))
+        self.assertFalse(res)
+        e = 'source dotfile is not a directory: {}'.format(src)
+        self.assertEqual(err, e)
 
     def test_creates_dst(self):
         """test creates dst"""

From 19a62bd08233ceaab7594fb1a0b7a5f7f3f486d6 Mon Sep 17 00:00:00 2001
From: deadc0de6 <info@deadc0de.ch>
Date: Sun, 31 Mar 2019 22:29:09 +0200
Subject: [PATCH 4/8] improve tests

---
 tests-ng/ignore-empty.sh | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests-ng/ignore-empty.sh b/tests-ng/ignore-empty.sh
index aa45fca..834c578 100755
--- a/tests-ng/ignore-empty.sh
+++ b/tests-ng/ignore-empty.sh
@@ -83,8 +83,8 @@ echo "not empty" >> ${tmps}/dotfiles/d1/notempty
 cd ${ddpath} | ${bin} install -f -c ${cfg} -p p1 -V
 
 # test existence
-[ -e ${tmpd}/d1/empty ] && exit 1
-[ ! -e ${tmpd}/d1/notempty ] && exit 1
+[ -e ${tmpd}/d1/empty ] && echo 'empty should not exist' && exit 1
+[ ! -e ${tmpd}/d1/notempty ] && echo 'not empty should exist' && exit 1
 
 # through the dotfile
 cat > ${cfg} << _EOF
@@ -112,8 +112,8 @@ rm -rf ${tmpd}/*
 cd ${ddpath} | ${bin} install -f -c ${cfg} -p p1 -V
 
 # test existence
-[ -e ${tmpd}/d1/empty ] && exit 1
-[ ! -e ${tmpd}/d1/notempty ] && exit 1
+[ -e ${tmpd}/d1/empty ] && echo 'empty should not exist' && exit 1
+[ ! -e ${tmpd}/d1/notempty ] && echo 'not empty should exist' && exit 1
 
 ## CLEANING
 rm -rf ${tmps} ${tmpd}

From e8643ec8a34b2ae611b75d0db1633997bbb2e5e4 Mon Sep 17 00:00:00 2001
From: deadc0de6 <info@deadc0de.ch>
Date: Sun, 31 Mar 2019 22:33:03 +0200
Subject: [PATCH 5/8] fix tests

---
 tests-ng/ignore-empty.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests-ng/ignore-empty.sh b/tests-ng/ignore-empty.sh
index 834c578..4d33f12 100755
--- a/tests-ng/ignore-empty.sh
+++ b/tests-ng/ignore-empty.sh
@@ -77,7 +77,7 @@ _EOF
 # create the dotfile
 mkdir -p ${tmps}/dotfiles/d1
 echo "{{@@ var1 @@}}" > ${tmps}/dotfiles/d1/empty
-echo "not empty" >> ${tmps}/dotfiles/d1/notempty
+echo "not empty" > ${tmps}/dotfiles/d1/notempty
 
 # install
 cd ${ddpath} | ${bin} install -f -c ${cfg} -p p1 -V

From c9e11ffc842103a9fc04b394ce190c4498d12e98 Mon Sep 17 00:00:00 2001
From: deadc0de6 <info@deadc0de.ch>
Date: Sun, 31 Mar 2019 22:42:11 +0200
Subject: [PATCH 6/8] fix empty installation

---
 dotdrop/installer.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/dotdrop/installer.py b/dotdrop/installer.py
index 2bdb38f..203ef35 100644
--- a/dotdrop/installer.py
+++ b/dotdrop/installer.py
@@ -175,8 +175,8 @@ class Installer:
                     self.log.dbg('install to {} and symlink'
                                  .format(self.workdir))
                 tmp = self._pivot_path(dst, self.workdir, striphome=True)
-                i = self.install(templater, src, tmp, actions=actions)
-                if not i and not os.path.exists(tmp):
+                r, e = self.install(templater, src, tmp, actions=actions)
+                if not r and e and not os.path.exists(tmp):
                     continue
                 src = tmp
 
@@ -285,7 +285,7 @@ class Installer:
                                              os.path.join(dst, entry),
                                              actions=actions,
                                              noempty=noempty)
-                if not res:
+                if not res and err:
                     ret = res, err
                     break
             else:
@@ -293,7 +293,7 @@ class Installer:
                                             os.path.join(dst, entry),
                                             actions=actions,
                                             noempty=noempty)
-                if not res:
+                if not res and err:
                     ret = res, err
                     break
         return ret

From 672d8c63a7648a3e72fae04e158133add75bd8d9 Mon Sep 17 00:00:00 2001
From: deadc0de6 <info@deadc0de.ch>
Date: Sun, 31 Mar 2019 23:16:08 +0200
Subject: [PATCH 7/8] fix installing directory dotfile

---
 dotdrop/installer.py | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/dotdrop/installer.py b/dotdrop/installer.py
index 203ef35..574c1e4 100644
--- a/dotdrop/installer.py
+++ b/dotdrop/installer.py
@@ -273,7 +273,8 @@ class Installer:
         if self.debug:
             self.log.dbg('install dir {}'.format(src))
             self.log.dbg('ignore empty: {}'.format(noempty))
-        ret = True, None
+        # default to nothing installed and no error
+        ret = False, None
         if not self._create_dirs(dst):
             err = 'creating directory for {}'.format(dst)
             return False, err
@@ -281,21 +282,31 @@ class Installer:
         for entry in os.listdir(src):
             f = os.path.join(src, entry)
             if not os.path.isdir(f):
+                # is file
                 res, err = self._handle_file(templater, f,
                                              os.path.join(dst, entry),
                                              actions=actions,
                                              noempty=noempty)
                 if not res and err:
+                    # error occured
                     ret = res, err
                     break
+                elif res:
+                    # something got installed
+                    ret = True, None
             else:
+                # is directory
                 res, err = self._handle_dir(templater, f,
                                             os.path.join(dst, entry),
                                             actions=actions,
                                             noempty=noempty)
                 if not res and err:
+                    # error occured
                     ret = res, err
                     break
+                elif res:
+                    # something got installed
+                    ret = True, None
         return ret
 
     def _fake_diff(self, dst, content):

From 15dd13f8b4b400e84131e5d779958a4ca3a05a4c Mon Sep 17 00:00:00 2001
From: deadc0de6 <info@deadc0de.ch>
Date: Tue, 2 Apr 2019 21:07:49 +0200
Subject: [PATCH 8/8] migrate to yaml.safe_dump

---
 dotdrop/config.py    | 9 ++++++---
 tests/test_config.py | 6 ++++--
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/dotdrop/config.py b/dotdrop/config.py
index d3a8ae5..2b7e351 100644
--- a/dotdrop/config.py
+++ b/dotdrop/config.py
@@ -569,8 +569,9 @@ class Cfg:
         """writes the config to file"""
         ret = False
         with open(path, 'w') as f:
-            ret = yaml.dump(content, f,
-                            default_flow_style=False, indent=2)
+            ret = yaml.safe_dump(content, f,
+                                 default_flow_style=False,
+                                 indent=2)
         return ret
 
     def _norm_key_elem(self, elem):
@@ -827,7 +828,9 @@ class Cfg:
         self.lnk_settings[self.key_dotpath] = self.curdotpath
         self.lnk_settings[self.key_workdir] = self.curworkdir
         # dump
-        ret = yaml.dump(self.content, default_flow_style=False, indent=2)
+        ret = yaml.safe_dump(self.content,
+                             default_flow_style=False,
+                             indent=2)
         ret = ret.replace('{}', '')
         # restore paths
         self.lnk_settings[self.key_dotpath] = dotpath
diff --git a/tests/test_config.py b/tests/test_config.py
index 460d6c7..a3112f3 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -79,7 +79,8 @@ class TestConfig(unittest.TestCase):
 
         # save the new config
         with open(confpath, 'w') as f:
-            yaml.dump(content, f, default_flow_style=False, indent=2)
+            yaml.safe_dump(content, f, default_flow_style=False,
+                           indent=2)
 
         # do the tests
         conf = Cfg(confpath)
@@ -109,7 +110,8 @@ class TestConfig(unittest.TestCase):
 
         # save the new config
         with open(confpath, 'w') as f:
-            yaml.dump(content, f, default_flow_style=False, indent=2)
+            yaml.safe_dump(content, f, default_flow_style=False,
+                           indent=2)
 
         # do the tests
         conf = Cfg(confpath)