From d4375c5d043cb015757f40e93705716588048a64 Mon Sep 17 00:00:00 2001 From: deadc0de6 Date: Wed, 22 Jan 2020 17:41:48 +0100 Subject: [PATCH 1/5] adding ability to provide diff command for #203 --- dotdrop/comparator.py | 10 ++-- dotdrop/dotdrop.py | 10 ++-- dotdrop/installer.py | 7 ++- dotdrop/options.py | 4 +- dotdrop/settings.py | 7 ++- dotdrop/utils.py | 9 ++- tests-ng/diff-cmd.sh | 125 ++++++++++++++++++++++++++++++++++++++++++ tests/helpers.py | 1 - 8 files changed, 153 insertions(+), 20 deletions(-) create mode 100755 tests-ng/diff-cmd.sh diff --git a/dotdrop/comparator.py b/dotdrop/comparator.py index 7cab104..8cd86cb 100644 --- a/dotdrop/comparator.py +++ b/dotdrop/comparator.py @@ -15,12 +15,12 @@ from dotdrop.utils import must_ignore, uniq_list, diff class Comparator: - def __init__(self, diffopts='', debug=False): + def __init__(self, diff_cmd='', debug=False): """constructor - @diffopts: switches to pass to unix diff + @diff_cmd: diff command to use @debug: enable debug """ - self.diffopts = diffopts + self.diff_cmd = diff_cmd self.debug = debug self.log = Logger() @@ -122,9 +122,9 @@ class Comparator: return ''.join(ret) def _diff(self, left, right, header=False): - """diff using the unix tool diff""" + """diff two files""" out = diff(modified=left, original=right, raw=False, - opts=self.diffopts, debug=self.debug) + diff_cmd=self.diff_cmd, debug=self.debug) if header: lshort = os.path.basename(left) out = '=> diff \"{}\":\n{}'.format(lshort, out) diff --git a/dotdrop/dotdrop.py b/dotdrop/dotdrop.py index 3dbe377..57efdf9 100644 --- a/dotdrop/dotdrop.py +++ b/dotdrop/dotdrop.py @@ -96,7 +96,8 @@ def cmd_install(o): diff=o.install_diff, debug=o.debug, totemp=tmpdir, showdiff=o.install_showdiff, - backup_suffix=o.install_backup_suffix) + backup_suffix=o.install_backup_suffix, + diff_cmd=o.diff_command) installed = 0 tvars = t.add_tmp_vars() @@ -211,8 +212,9 @@ def cmd_compare(o, tmp): inst = Installer(create=o.create, backup=o.backup, dry=o.dry, base=o.dotpath, workdir=o.workdir, debug=o.debug, - backup_suffix=o.install_backup_suffix) - comp = Comparator(diffopts=o.compare_dopts, debug=o.debug) + backup_suffix=o.install_backup_suffix, + diff_cmd=o.diff_command) + comp = Comparator(diff_cmd=o.diff_command, debug=o.debug) for dotfile in selected: if o.debug: @@ -362,7 +364,7 @@ def cmd_importer(o): if os.path.exists(srcf): overwrite = True if o.safe: - c = Comparator(debug=o.debug) + c = Comparator(debug=o.debug, diff_cmd=o.diff_command) diff = c.compare(srcf, dst) if diff != '': # files are different, dunno what to do diff --git a/dotdrop/installer.py b/dotdrop/installer.py index 2c5936a..c61cf98 100644 --- a/dotdrop/installer.py +++ b/dotdrop/installer.py @@ -19,7 +19,7 @@ class Installer: def __init__(self, base='.', create=True, backup=True, dry=False, safe=False, workdir='~/.config/dotdrop', debug=False, diff=True, totemp=None, showdiff=False, - backup_suffix='.dotdropbak'): + backup_suffix='.dotdropbak', diff_cmd=''): """constructor @base: directory path where to search for templates @create: create directory hierarchy if missing when installing @@ -32,6 +32,7 @@ class Installer: @totemp: deploy to this path instead of dotfile dst if not None @showdiff: show the diff before overwriting (or asking for) @backup_suffix: suffix for dotfile backup file + @diff_cmd: diff command to use """ self.create = create self.backup = backup @@ -44,6 +45,7 @@ class Installer: self.totemp = totemp self.showdiff = showdiff self.backup_suffix = backup_suffix + self.diff_cmd = diff_cmd self.comparing = False self.action_executed = False self.log = Logger() @@ -437,7 +439,8 @@ class Installer: if content: tmp = utils.write_to_tmpfile(content) src = tmp - diff = utils.diff(modified=src, original=dst, raw=False) + diff = utils.diff(modified=src, original=dst, raw=False, + diff_cmd=self.diff_cmd) if tmp: utils.remove(tmp, quiet=True) diff --git a/dotdrop/options.py b/dotdrop/options.py index 8489ca9..de7babc 100644 --- a/dotdrop/options.py +++ b/dotdrop/options.py @@ -55,7 +55,7 @@ Usage: dotdrop import [-Vbdf] [-c ] [-p ] [-l ] ... dotdrop compare [-Vb] [-c ] [-p ] - [-o ] [-C ...] [-i ...] + [-C ...] [-i ...] dotdrop update [-VbfdkP] [-c ] [-p ] [-i ...] [...] dotdrop remove [-Vbfdk] [-c ] [-p ] [...] @@ -70,7 +70,6 @@ Options: -c --cfg= Path to the config. -C --file= Path of dotfile to compare. -i --ignore= Pattern to ignore. - -o --dopts= Diff options [default: ]. -l --link= "link_on_import" (nolink|link|link_children). -n --nodiff Do not diff when installing. -t --temp Install to a temporary directory for review. @@ -231,7 +230,6 @@ class Options(AttrMonitor): if a.kind == Action.post] self.install_ignore = self.instignore # "compare" specifics - self.compare_dopts = self.args['--dopts'] self.compare_focus = self.args['--file'] self.compare_ignore = self.args['--ignore'] self.compare_ignore.extend(self.cmpignore) diff --git a/dotdrop/settings.py b/dotdrop/settings.py index e375163..bfc3ae6 100644 --- a/dotdrop/settings.py +++ b/dotdrop/settings.py @@ -33,6 +33,7 @@ class Settings(DictParser): key_minversion = 'minversion' key_func_file = 'func_file' key_filter_file = 'filter_file' + key_diff_command = 'diff_command' # import keys key_import_actions = 'import_actions' @@ -47,7 +48,8 @@ class Settings(DictParser): link_on_import=LinkTypes.NOLINK, longkey=False, upignore=[], cmpignore=[], instignore=[], workdir='~/.config/dotdrop', showdiff=False, - minversion=None, func_file=[], filter_file=[]): + minversion=None, func_file=[], filter_file=[], + diff_command='diff -r {0} {1}'): self.backup = backup self.banner = banner self.create = create @@ -69,6 +71,7 @@ class Settings(DictParser): self.minversion = minversion self.func_file = func_file self.filter_file = filter_file + self.diff_command = diff_command def _serialize_seq(self, name, dic): """serialize attribute 'name' into 'dic'""" @@ -77,7 +80,6 @@ class Settings(DictParser): def serialize(self): """Return key-value pair representation of the settings""" - # Tedious, but less error-prone than introspection dic = { self.key_backup: self.backup, self.key_banner: self.banner, @@ -91,6 +93,7 @@ class Settings(DictParser): self.key_showdiff: self.showdiff, self.key_workdir: self.workdir, self.key_minversion: self.minversion, + self.key_diff_command: self.diff_command, } self._serialize_seq(self.key_default_actions, dic) self._serialize_seq(self.key_import_actions, dic) diff --git a/dotdrop/utils.py b/dotdrop/utils.py index 893af71..1fae268 100644 --- a/dotdrop/utils.py +++ b/dotdrop/utils.py @@ -70,9 +70,12 @@ def shell(cmd, debug=False): return ret == 0, out -def diff(original, modified, raw=True, opts='', debug=False): - """call unix diff to compare two files""" - cmd = 'diff -r {} \"{}\" \"{}\"'.format(opts, original, modified) +def diff(original, modified, raw=True, + diff_cmd='diff -r {0} {1}', debug=False): + """compare two files""" + if not diff_cmd: + diff_cmd = 'diff -r {0} {1}' + cmd = diff_cmd.format(original, modified) _, out = run(shlex.split(cmd), raw=raw, debug=debug) return out diff --git a/tests-ng/diff-cmd.sh b/tests-ng/diff-cmd.sh new file mode 100755 index 0000000..1956c26 --- /dev/null +++ b/tests-ng/diff-cmd.sh @@ -0,0 +1,125 @@ +#!/usr/bin/env bash +# author: deadc0de6 (https://github.com/deadc0de6) +# Copyright (c) 2017, deadc0de6 +# +# test diff cmd +# 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 "$(tput setaf 6)==> RUNNING $(basename $BASH_SOURCE) <==$(tput sgr0)" + +################################################################ +# this is the test +################################################################ + +# dotdrop directory +basedir=`mktemp -d --suffix='-dotdrop-tests' || mktemp -d` +echo "[+] dotdrop dir: ${basedir}" +echo "[+] dotpath dir: ${basedir}/dotfiles" + +# the dotfile to be imported +tmpd=`mktemp -d --suffix='-dotdrop-tests' || mktemp -d` + +# some files +echo "original" > ${tmpd}/singlefile + +# create the config file +cfg="${basedir}/config.yaml" +cat > ${cfg} << _EOF +config: + backup: true + create: true + dotpath: dotfiles +dotfiles: +profiles: +_EOF + +# import +echo "[+] import" +cd ${ddpath} | ${bin} import -c ${cfg} ${tmpd}/singlefile + +# modify the file +echo "modified" > ${tmpd}/singlefile + +# normal diff +echo "[+] comparing with normal diff" +set +e +cd ${ddpath} | ${bin} compare -c ${cfg} 2>&1 | grep -v '=>' > ${tmpd}/normal +diff -r ${tmpd}/singlefile ${basedir}/dotfiles/${tmpd}/singlefile > ${tmpd}/real +set -e + +# verify +#cat ${tmpd}/normal +#cat ${tmpd}/real +diff ${tmpd}/normal ${tmpd}/real || exit 1 + +# adding unified diff +cfg2="${basedir}/config2.yaml" +sed '/dotpath: dotfiles/a \ \ diff_command: "diff -u {0} {1}"' ${cfg} > ${cfg2} +#cat ${cfg2} + +# unified diff +echo "[+] comparing with unified diff" +set +e +cd ${ddpath} | ${bin} compare -c ${cfg2} 2>&1 | grep -v '=>' | grep -v '^+++\|^---' > ${tmpd}/unified +diff -u ${tmpd}/singlefile ${basedir}/dotfiles/${tmpd}/singlefile | grep -v '^+++\|^---' > ${tmpd}/real +set -e + +# verify +#cat ${tmpd}/unified +#cat ${tmpd}/real +diff ${tmpd}/unified ${tmpd}/real || exit 1 + +# adding fake diff +cfg3="${basedir}/config3.yaml" +sed '/dotpath: dotfiles/a \ \ diff_command: "echo fakediff"' ${cfg} > ${cfg3} +cat ${cfg3} + +# fake diff +echo "[+] comparing with fake diff" +set +e +cd ${ddpath} | ${bin} compare -c ${cfg3} 2>&1 | grep -v '=>' > ${tmpd}/fake +set -e + +# verify +cat ${tmpd}/fake +grep fakediff ${tmpd}/fake || exit 1 + +## CLEANING +rm -rf ${basedir} ${tmpd} + +echo "OK" +exit 0 diff --git a/tests/helpers.py b/tests/helpers.py index 997a487..cc6b958 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -157,7 +157,6 @@ def load_options(confpath, profile): o.import_link = LinkTypes.NOLINK o.install_showdiff = True o.debug = True - o.compare_dopts = '' o.variables = {} return o From 98911550df3b7efe1f9189111a850fbf6127e5f7 Mon Sep 17 00:00:00 2001 From: deadc0de6 Date: Thu, 23 Jan 2020 08:27:44 +0100 Subject: [PATCH 2/5] fix quotes in arguments for #203 --- dotdrop/settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotdrop/settings.py b/dotdrop/settings.py index bfc3ae6..0bef2b3 100644 --- a/dotdrop/settings.py +++ b/dotdrop/settings.py @@ -49,7 +49,7 @@ class Settings(DictParser): upignore=[], cmpignore=[], instignore=[], workdir='~/.config/dotdrop', showdiff=False, minversion=None, func_file=[], filter_file=[], - diff_command='diff -r {0} {1}'): + diff_command='diff -r "{0}" "{1}"'): self.backup = backup self.banner = banner self.create = create From 40a9872f56a07af934257e3f689d75fe0b36632c Mon Sep 17 00:00:00 2001 From: deadc0de6 Date: Sun, 26 Jan 2020 11:21:14 +0100 Subject: [PATCH 3/5] quote diff command arguments (#203) --- dotdrop/utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dotdrop/utils.py b/dotdrop/utils.py index 1fae268..df073e6 100644 --- a/dotdrop/utils.py +++ b/dotdrop/utils.py @@ -71,10 +71,10 @@ def shell(cmd, debug=False): def diff(original, modified, raw=True, - diff_cmd='diff -r {0} {1}', debug=False): + diff_cmd='diff -r "{0}" "{1}"', debug=False): """compare two files""" if not diff_cmd: - diff_cmd = 'diff -r {0} {1}' + diff_cmd = 'diff -r "{0}" "{1}"' cmd = diff_cmd.format(original, modified) _, out = run(shlex.split(cmd), raw=raw, debug=debug) return out From 704f8deab6ac872b35699fc910a3a61ed66c4522 Mon Sep 17 00:00:00 2001 From: Sighery Date: Sun, 26 Jan 2020 12:28:28 +0100 Subject: [PATCH 4/5] Use alternative list-based diff_cmd escaping The previous escaping method of wrapping the arguments with double quotes would eventually fail in some cases, since Linux allows pretty much any character for a filename. Using a different quoting character, like the single quote would have brought you back to the exact same issue. As soon as any part of the path contained your escape quote character, the code would break. The code would use `shlex`, a parser for Linux shells. However, this wasn't working since even that wouldn't know where your argument began and ended, since it wasn't escaped properly. Meaning, a string like: `diff -r /home/test/.config/Code - OSS/t"t't.test mytestfile.test` Would then break any of the quoting solutions. And shlex, since it wouldn't know where arguments start and end, it would think an argument ends at `home/test/.config/Code`, since the spaces haven't been escaped. But escaping the spaces with quote characters is not a good idea since any parts of the path with those quote arguments would then again break shlex and it wouldn't be able to tell when your argument starts and ends. The solution for that is to, before we replace our diff template string with the given files arguments, we can just split it by whitespace, and manually replace the `{0}` and `{1}` placeholders. This allows us to keep the separation with a Python list. What does this mean? That when you then call `subprocess.Popen` with this list, `subprocess` knows where all your arguments start and end, even if they themselves are not properly escaped. But since it's all split in a list, `subprocess` has a concept of what is a single argument and would apply the needed escaping to each individual argument. --- dotdrop/settings.py | 2 +- dotdrop/utils.py | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/dotdrop/settings.py b/dotdrop/settings.py index 0bef2b3..bfc3ae6 100644 --- a/dotdrop/settings.py +++ b/dotdrop/settings.py @@ -49,7 +49,7 @@ class Settings(DictParser): upignore=[], cmpignore=[], instignore=[], workdir='~/.config/dotdrop', showdiff=False, minversion=None, func_file=[], filter_file=[], - diff_command='diff -r "{0}" "{1}"'): + diff_command='diff -r {0} {1}'): self.backup = backup self.banner = banner self.create = create diff --git a/dotdrop/utils.py b/dotdrop/utils.py index df073e6..1842d30 100644 --- a/dotdrop/utils.py +++ b/dotdrop/utils.py @@ -71,12 +71,19 @@ def shell(cmd, debug=False): def diff(original, modified, raw=True, - diff_cmd='diff -r "{0}" "{1}"', debug=False): + diff_cmd='diff -r {0} {1}', debug=False): """compare two files""" if not diff_cmd: - diff_cmd = 'diff -r "{0}" "{1}"' - cmd = diff_cmd.format(original, modified) - _, out = run(shlex.split(cmd), raw=raw, debug=debug) + diff_cmd = 'diff -r {0} {1}' + + replacements = { + "{0}": original, + "{original}": original, + "{1}": modified, + "{modified}": modified, + } + cmd = [replacements.get(x, x) for x in diff_cmd.split()] + _, out = run(cmd, raw=raw, debug=debug) return out From 99150cec8887fd50b34c4a65d0b4911a890f7937 Mon Sep 17 00:00:00 2001 From: deadc0de6 Date: Sun, 26 Jan 2020 19:37:52 +0100 Subject: [PATCH 5/5] remove "import shlex" to pass tests --- dotdrop/utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/dotdrop/utils.py b/dotdrop/utils.py index 1842d30..689b4db 100644 --- a/dotdrop/utils.py +++ b/dotdrop/utils.py @@ -9,7 +9,6 @@ import subprocess import tempfile import os import uuid -import shlex import fnmatch import inspect import importlib