From 8ce96ffeebe802ce0949c67087723fa8edaff13f Mon Sep 17 00:00:00 2001
From: deadc0de6 <info@deadc0de.ch>
Date: Sun, 15 Nov 2020 20:27:49 +0100
Subject: [PATCH] improve diffing

---
 dotdrop/installer.py       | 73 ++++++++++++++++++++++++--------------
 dotdrop/utils.py           |  6 ++++
 tests-ng/actions-pre.sh    | 33 ++++++++++-------
 tests-ng/chmod-install.sh  |  2 +-
 tests-ng/import-configs.sh |  2 +-
 tests-ng/update-ignore.sh  | 11 +++++-
 tests.sh                   |  8 +++--
 7 files changed, 91 insertions(+), 44 deletions(-)

diff --git a/dotdrop/installer.py b/dotdrop/installer.py
index ea3f4bf..27c8004 100644
--- a/dotdrop/installer.py
+++ b/dotdrop/installer.py
@@ -382,7 +382,7 @@ class Installer:
                 self.log.dry('would remove {} and link to {}'.format(dst, src))
                 return True, None
             if self.showdiff:
-                self._diff_before_write(src, dst, quiet=False)
+                self._show_diff_before_write(src, dst)
             msg = 'Remove "{}" for link creation?'.format(dst)
             if self.safe and not self.log.ask(msg):
                 err = 'ignoring "{}", link was not created'.format(dst)
@@ -565,43 +565,31 @@ class Installer:
             return 0, None
 
         if os.path.lexists(dst):
-            if chmod:
-                rights = chmod
-            else:
-                rights = utils.get_file_perm(src)
-            samerights = False
             try:
-                dstrights = utils.get_file_perm(dst)
-                samerights = dstrights == rights
+                os.stat(dst)
             except OSError as e:
                 if e.errno == errno.ENOENT:
                     # broken symlink
                     err = 'broken symlink {}'.format(dst)
                     return -1, err
-            if self.debug:
-                d = 'src mode {:o}, dst mode {:o}'
-                self.log.dbg(d.format(rights, dstrights))
 
-            diff = None
+            src_mode = chmod
+            if not src_mode:
+                src_mode = utils.get_file_perm(src)
             if self.diff:
-                diff = self._diff_before_write(src, dst,
-                                               content=content,
-                                               quiet=True)
-                if not diff and samerights:
+                if not self._is_different(src, dst,
+                                          content=content,
+                                          src_mode=src_mode):
                     if self.debug:
                         self.log.dbg('{} is the same'.format(dst))
                     return 1, None
-            if diff and self.safe:
+            if self.safe:
                 if self.debug:
                     self.log.dbg('change detected for {}'.format(dst))
                 if self.showdiff:
-                    if diff is None:
-                        # get diff
-                        diff = self._diff_before_write(src, dst,
-                                                       content=content,
-                                                       quiet=True)
-                        if diff:
-                            self._print_diff(src, dst, diff)
+                    # get diff
+                    self._show_diff_before_write(src, dst,
+                                                 content=content)
                 if not self.log.ask('Overwrite \"{}\"'.format(dst)):
                     self.log.warn('ignoring {}'.format(dst))
                     return 1, None
@@ -656,7 +644,40 @@ class Installer:
         tmp['_dotfile_sub_abs_dst'] = dst
         return tmp
 
-    def _diff_before_write(self, src, dst, content=None, quiet=False):
+    def _is_different(self, src, dst, src_mode=None, content=None):
+        """
+        returns True if file is different and
+        needs to be installed
+        """
+        # check file size
+        src_size = os.stat(src).st_size
+        dst_size = os.stat(dst).st_size
+        if src_size != dst_size:
+            if self.debug:
+                self.log.dbg('size differ')
+            return True
+
+        # check file mode
+        if not src_mode:
+            src_mode = utils.get_file_perm(src)
+        dst_mode = utils.get_file_perm(dst)
+        if src_mode != dst_mode:
+            if self.debug:
+                m = 'mode differ ({:o} vs {:o})'
+                self.log.dbg(m.format(src_mode, dst_mode))
+            return True
+
+        # check file content
+        if content:
+            tmp = utils.write_to_tmpfile(content)
+            src = tmp
+        r = utils.fastdiff(src, dst)
+        if r:
+            if self.debug:
+                self.log.dbg('content differ')
+        return r
+
+    def _show_diff_before_write(self, src, dst, content=None):
         """
         diff before writing
         using a temp file if content is not None
@@ -671,7 +692,7 @@ class Installer:
         if tmp:
             utils.removepath(tmp, logger=self.log)
 
-        if not quiet and diff:
+        if diff:
             self._print_diff(src, dst, diff)
         return diff
 
diff --git a/dotdrop/utils.py b/dotdrop/utils.py
index eeaba8a..33c4a8b 100644
--- a/dotdrop/utils.py
+++ b/dotdrop/utils.py
@@ -12,6 +12,7 @@ import uuid
 import fnmatch
 import inspect
 import importlib
+import filecmp
 from shutil import rmtree, which
 
 # local import
@@ -73,6 +74,11 @@ def shell(cmd, debug=False):
     return ret == 0, out
 
 
+def fastdiff(left, right):
+    """fast compare files and returns True if different"""
+    return not filecmp.cmp(left, right, shallow=False)
+
+
 def diff(original, modified, raw=True,
          diff_cmd='', debug=False):
     """compare two files, returns '' if same"""
diff --git a/tests-ng/actions-pre.sh b/tests-ng/actions-pre.sh
index 24ae575..5f8949b 100755
--- a/tests-ng/actions-pre.sh
+++ b/tests-ng/actions-pre.sh
@@ -46,6 +46,15 @@ echo -e "$(tput setaf 6)==> RUNNING $(basename $BASH_SOURCE) <==$(tput sgr0)"
 # this is the test
 ################################################################
 
+# $1 pattern
+# $2 path
+grep_or_fail()
+{
+  set +e
+  grep "${1}" "${2}" >/dev/null 2>&1 || (echo "pattern not found in ${2}" && exit 1)
+  set -e
+}
+
 # the action temp
 tmpa=`mktemp -d --suffix='-dotdrop-tests' || mktemp -d`
 # the dotfile source
@@ -136,38 +145,36 @@ cd ${ddpath} | ${bin} install -f -c ${cfg} -p p1 -V
 
 # checks
 [ ! -e ${tmpa}/pre ] && echo 'pre action not executed' && exit 1
-grep pre ${tmpa}/pre >/dev/null
+grep_or_fail pre ${tmpa}/pre
 [ ! -e ${tmpa}/naked ] && echo 'naked action not executed'  && exit 1
-grep naked ${tmpa}/naked >/dev/null
+grep_or_fail naked ${tmpa}/naked
 
 [ ! -e ${tmpa}/multiple ] && echo 'pre action multiple not executed' && exit 1
-grep multiple ${tmpa}/multiple >/dev/null
+grep_or_fail multiple ${tmpa}/multiple
 [ "`wc -l ${tmpa}/multiple | awk '{print $1}'`" -gt "1" ] && echo 'pre action multiple executed twice' && exit 1
 
 [ ! -e ${tmpa}/pre2 ] && echo 'pre action 2 not executed' && exit 1
-grep pre2 ${tmpa}/pre2 >/dev/null
+grep_or_fail pre2 ${tmpa}/pre2
 [ ! -e ${tmpa}/naked2 ] && echo 'naked action 2 not executed'  && exit 1
-grep naked2 ${tmpa}/naked2 >/dev/null
+grep_or_fail naked2 ${tmpa}/naked2
 
 [ ! -e ${tmpa}/multiple2 ] && echo 'pre action multiple 2 not executed' && exit 1
-grep multiple2 ${tmpa}/multiple2 >/dev/null
+grep_or_fail multiple2 ${tmpa}/multiple2
 [ "`wc -l ${tmpa}/multiple2 | awk '{print $1}'`" -gt "1" ] && echo 'pre action multiple 2 executed twice' && exit 1
 [ ! -e ${tmpa}/naked3 ] && echo 'naked action 3 not executed'  && exit 1
-grep naked3 ${tmpa}/naked3 >/dev/null
+grep_or_fail naked3 ${tmpa}/naked3
 
-
-# remove the pre action result and re-run
+# remove the pre action result and re-install
 rm ${tmpa}/pre
-
-cd ${ddpath} | ${bin} install -f -c ${cfg} -p p1
-[ -e ${tmpa}/pre ] && exit 1
+cd ${ddpath} | ${bin} install -f -c ${cfg} -p p1 -V
+[ -e ${tmpa}/pre ] && echo "pre exists" && exit 1
 
 # ensure failing actions make the installation fail
 # install
 set +e
 cd ${ddpath} | ${bin} install -f -c ${cfg} -p p2 -V
 set -e
-[ -e ${tmpd}/fail ] && exit 1
+[ -e ${tmpd}/fail ] && echo "fail exists" && exit 1
 
 ## CLEANING
 rm -rf ${tmps} ${tmpd} ${tmpa}
diff --git a/tests-ng/chmod-install.sh b/tests-ng/chmod-install.sh
index 67aa009..bd0259e 100755
--- a/tests-ng/chmod-install.sh
+++ b/tests-ng/chmod-install.sh
@@ -262,7 +262,7 @@ echo "nomode" > ${tmps}/dotfiles/nomode
 chmod 600 ${tmps}/dotfiles/nomode
 echo "nomode" > ${tmpd}/nomode
 chmod 700 ${tmpd}/nomode
-cd ${ddpath} | printf 'y\n' | ${bin} install -c ${cfg} -p p2 -V f_nomode
+cd ${ddpath} | printf 'y\ny\n' | ${bin} install -c ${cfg} -p p2 -V f_nomode
 has_rights "${tmpd}/nomode" "600"
 
 ## CLEANING
diff --git a/tests-ng/import-configs.sh b/tests-ng/import-configs.sh
index 4c75d4c..2328fad 100755
--- a/tests-ng/import-configs.sh
+++ b/tests-ng/import-configs.sh
@@ -172,7 +172,7 @@ profiles:
     dotfiles:
     - f_asub
 _EOF
-cd ${ddpath} | ${bin} install -c ${cfg1} -p p2 -V
+cd ${ddpath} | ${bin} install -c ${cfg1} -p p2 -V -f
 cd ${ddpath} | ${bin} compare -c ${cfg1} -p p2 -V
 
 ## CLEANING
diff --git a/tests-ng/update-ignore.sh b/tests-ng/update-ignore.sh
index 9794e36..cf928ce 100755
--- a/tests-ng/update-ignore.sh
+++ b/tests-ng/update-ignore.sh
@@ -46,6 +46,15 @@ echo -e "$(tput setaf 6)==> RUNNING $(basename $BASH_SOURCE) <==$(tput sgr0)"
 # this is the test
 ################################################################
 
+# $1 pattern
+# $2 path
+grep_or_fail()
+{
+  set +e
+  grep "${1}" "${2}" >/dev/null 2>&1 || (echo "pattern not found in ${2}" && exit 1)
+  set -e
+}
+
 # dotdrop directory
 tmps=`mktemp -d --suffix='-dotdrop-tests' || mktemp -d`
 dt="${tmps}/dotfiles"
@@ -98,7 +107,7 @@ cd ${ddpath} | ${bin} update -f -c ${cfg} --verbose --profile=p1 --key f_abc
 #tree ${dt}
 
 # check files haven't been updated
-grep 'b' ${dt}/a/c/acfile >/dev/null
+grep_or_fail 'b' "${dt}/a/c/acfile"
 [ -e ${dt}/a/newfile ] && echo "should not have been updated" && exit 1
 
 ## CLEANING
diff --git a/tests.sh b/tests.sh
index c32ddec..05a4509 100755
--- a/tests.sh
+++ b/tests.sh
@@ -3,7 +3,8 @@
 # Copyright (c) 2017, deadc0de6
 
 # stop on first error
-set -ev
+#set -ev
+set -e
 
 # PEP8 tests
 which pycodestyle >/dev/null 2>&1
@@ -54,9 +55,12 @@ unset DOTDROP_FORCE_NODEBUG
 [ "$1" = '--python-only' ] || {
   echo "doing extended tests"
   logdir=`mktemp -d`
+  tot=`ls -1 tests-ng/*.sh | wc -l`
+  cnt=0
   for scr in tests-ng/*.sh; do
+    cnt=$((cnt + 1))
     logfile="${logdir}/`basename ${scr}`.log"
-    echo "-> running test ${scr} (logfile:${logfile})"
+    echo "-> (${cnt}/${tot}) running test ${scr} (logfile:${logfile})"
     set +e
     ${scr} > "${logfile}" 2>&1
     if [ "$?" -ne 0 ]; then