From e4d2d272a8c01b2b17990f5a05e4906844f877db Mon Sep 17 00:00:00 2001 From: deadc0de6 Date: Tue, 17 Nov 2020 13:42:53 +0100 Subject: [PATCH] refactor installer --- docs/config.md | 2 - dotdrop/dotdrop.py | 19 +-- dotdrop/dotfile.py | 6 +- dotdrop/installer.py | 282 +++++++++++++------------------------- dotdrop/utils.py | 1 + tests-ng/chmod-install.sh | 2 +- tests-ng/diff-cmd.sh | 6 +- tests/test_install.py | 31 +++-- 8 files changed, 132 insertions(+), 217 deletions(-) diff --git a/docs/config.md b/docs/config.md index fa5ee18..86a553e 100644 --- a/docs/config.md +++ b/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`: diff --git a/dotdrop/dotdrop.py b/dotdrop/dotdrop.py index 588be51..f9cdcc4 100644 --- a/dotdrop/dotdrop.py +++ b/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, - actionexec=pre_actions_exec, - template=dotfile.template, - chmod=dotfile.chmod) + r, err = inst.install(t, dotfile.src, dotfile.dst, + dotfile.link, + actionexec=pre_actions_exec, + 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, - actionexec=pre_actions_exec, - template=dotfile.template, - chmod=dotfile.chmod) + r, err = inst.install(t, dotfile.src, dotfile.dst, + dotfile.link, + actionexec=pre_actions_exec, + 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, diff --git a/dotdrop/dotfile.py b/dotdrop/dotfile.py index fee2727..9c96434 100644 --- a/dotdrop/dotfile.py +++ b/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() diff --git a/dotdrop/installer.py b/dotdrop/installer.py index 1dee366..4867b77 100644 --- a/dotdrop/installer.py +++ b/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, - actionexec=actionexec, - noempty=noempty, - ignore=ignore, - template=template, - chmod=chmod) + # 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 self._log_install(False, err) - return self._log_install(r, err) + self.action_executed = False - def link(self, templater, src, dst, actionexec=None, - template=True, chmod=None): - """ - set src as the link target of dst + # install to temporary dir + # and ignore any actions + if self.totemp: + r, err, _ = self.install_to_temp(templater, self.totemp, + src, dst, template=template, + chmod=chmod) + return self._log_install(r, err) - @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: rights to apply if any - - return - - True, None : success - - False, error_msg : error - - False, None : ignored - """ + isdir = os.path.isdir(src) if self.debug: - self.log.dbg('link \"{}\" to \"{}\"'.format(src, dst)) - src, dst, cont, err = self._check_paths(src, dst, chmod) - if not cont: - return self._log_install(cont, err) + self.log.dbg('install {} to {}'.format(src, dst)) + self.log.dbg('\"{}\" is a directory: {}'.format(src, isdir)) - r, err = self._link(templater, src, dst, - actionexec=actionexec, - template=template) + # TODO remove template and set templater + # to None when no template should occur + 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 - utils.chmod(dst, chmod, self.debug) - return self._log_install(r, err) + dstperms = utils.get_file_perm(dst) + if dstperms != chmod: + # 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, - 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): + 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('install to {} and symlink'.format(self.workdir)) + self.log.dbg('is a template') + 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): - return i, err + if not r and not os.path.exists(tmp): + return r, err src = tmp - b, e = self._symlink(src, dst, actionexec=actionexec) - return b, e + r, err = self._symlink(src, dst, actionexec=actionexec) + return r, err - def _link_children(self, templater, src, dst, actionexec=None, - template=True, chmod=None): + def _link_children(self, templater, src, dst, + 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 diff --git a/dotdrop/utils.py b/dotdrop/utils.py index b04be4b..52fb01c 100644 --- a/dotdrop/utils.py +++ b/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 diff --git a/tests-ng/chmod-install.sh b/tests-ng/chmod-install.sh index 72853de..5c4d363 100755 --- a/tests-ng/chmod-install.sh +++ b/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 } diff --git a/tests-ng/diff-cmd.sh b/tests-ng/diff-cmd.sh index f44e11f..639c6ae 100755 --- a/tests-ng/diff-cmd.sh +++ b/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 diff --git a/tests/test_install.py b/tests/test_install.py index 2da99be..cbac55a 100644 --- a/tests/test_install.py +++ b/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, - dst=dst_dir, actionexec=None) + installer.install(templater=MagicMock(), src=src_dir, + 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, - dst='/dev/null', actionexec=None) + res, err = installer.install(templater=MagicMock(), src=src, + 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, - dst='/dev/null', actionexec=None) + res, err = installer.install(templater=templater, src=src, + 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, - dst=dst_dir, actionexec=None) + installer.install(templater=MagicMock(), src=src_dir, + 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, - actionexec=None) + installer.install(templater=MagicMock(), src=src_dir, + 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, - actionexec=None) + installer.install(templater=templater, src=src_dir, dst=dst_dir, + linktype=LinkTypes.LINK_CHILDREN, actionexec=None) for src in srcs: dst = os.path.join(dst_dir, os.path.basename(src))