[bug-diffutils] diff: support for --no-dereference option

[Bruno Haible]

Hi,

Recently I made a backup of a root partition (with "tar cf - .")
and then wanted to compare the current state of that partition with
the backup.

The most suitable tool for this purpose appears to be "diff -r".
But symbolic links are a problem:
  - Symbolic links to targets inside the same tree could be ignored.
  - Symbolic links to targets outside the tree, given as relative filenames
    (e.g. /usr/bin/foo -> ../../etc/alternatives/foo), must not be compared
    because the two trees to be compared are not brethren.
  - Symbolic links to targets outside the tree, given as absolute filenames
    (e.g. /usr/bin/foo -> /etc/alternatives/foo), should not be compared
    because it would be a waste of time.

So one needs an option --no-dereference.

Other people have had the same wish, see
<https://lists.gnu.org/archive/html/bug-gnu-utils/2011-10/msg00010.html>
<https://lists.gnu.org/archive/html/bug-gnu-utils/2011-10/msg00014.html>

Obviously the option should be called '--no-dereference'. The short option
is '-h' for 'cp', 'chown', 'chgrp', 'chcon', 'touch'. So that's the best
candidate for 'diff' as well.

Here's a patch that implements this new feature. Patch to be applied with
"git am -3 < 0001-New-option-h-no-dereference.patch". It includes a unit test.


2012-01-06  Bruno Haible  <address@hidden>

        New option -h/--no-dereference.
        * src/diff.h (no_dereference_symlinks): New variable.
        * src/diff.c: Include xreadlink.h.
        (longopts): Add --no-dereference option.
        (main): Accept -h/--no-dereference option.
        (option_help_msgid): Mention the -h/--no-dereference option.
        (compare_files): If no_dereference_symlinks is true, use lstat()
        instead of stat(). Compare symbolic links by comparing their values.
        * bootstrap.conf (gnulib_modules): Add lstat, stat, xreadlink.
        * doc/diffutils.texi (Comparing Directories, diff Options): Mention the
        -h/--no-dereference option.
        * tests/no-dereference: New file.
        * tests/Makefile.am (TESTS): Add it.

>From f6b9e9ec8373e470dab9af2d5c4d1f1cff4c209c Mon Sep 17 00:00:00 2001
From: Bruno Haible <address@hidden>
Date: Sat, 7 Jan 2012 00:57:29 +0100
Subject: [PATCH] New option -h/--no-dereference.
 * src/diff.h (no_dereference_symlinks): New variable.
 * src/diff.c: Include xreadlink.h.
 (longopts): Add --no-dereference option.
 (main): Accept -h/--no-dereference option.
 (option_help_msgid): Mention the -h/--no-dereference option.
 (compare_files): If no_dereference_symlinks is true, use lstat()
 instead of stat(). Compare symbolic links by comparing their values.
 * bootstrap.conf (gnulib_modules): Add lstat, stat, xreadlink.
 * doc/diffutils.texi (Comparing Directories, diff Options): Mention the
 -h/--no-dereference option.
 * tests/no-dereference: New file.
 * tests/Makefile.am (TESTS): Add it.

---
 bootstrap.conf       |    3 +
 doc/diffutils.texi   |    9 +++
 src/diff.c           |   74 +++++++++++++++++++---
 src/diff.h           |    3 +
 tests/Makefile.am    |    1 +
 tests/no-dereference |  169 ++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 251 insertions(+), 8 deletions(-)
 create mode 100644 tests/no-dereference

diff --git a/bootstrap.conf b/bootstrap.conf
index 2399d08..51da0c0 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -48,6 +48,7 @@ hard-locale
 inttostr
 inttypes
 largefile
+lstat
 maintainer-makefile
 manywarnings
 mbrtowc
@@ -60,6 +61,7 @@ regex
 sh-quote
 signal
 sigprocmask
+stat
 stat-macros
 stat-time
 stdint
@@ -77,6 +79,7 @@ version-etc-fsf
 wcwidth
 xalloc
 xfreopen
+xreadlink
 xstrtoumax
 '
 
diff --git a/doc/diffutils.texi b/doc/diffutils.texi
index 5aff3c3..dc94ae2 100644
--- a/doc/diffutils.texi
+++ b/doc/diffutils.texi
@@ -1846,6 +1846,11 @@ is specified while the @option{--ignore-file-name-case} 
option is in
 effect, case is ignored when excluding file names matching the
 specified patterns.
 
+To avoid that @command{diff} follows symbolic links, use the
address@hidden (@option{-h}).  When this option is in use,
+symbolic links will be treated like a special kind of files, rather than
+comparing the target of each symbolic link.
+
 @node Adjusting Output
 @chapter Making @command{diff} Output Prettier
 
@@ -3786,6 +3791,10 @@ Headings}.
 @item address@hidden
 Compare @var{file} to each operand; @var{file} may be a directory.
 
address@hidden -h
address@hidden --no-dereference
+Act on symbolic links themselves instead of what they point to.
+
 @item --help
 Output a summary of usage and then exit.
 
diff --git a/src/diff.c b/src/diff.c
index 004654e..96cbe09 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -39,6 +39,7 @@
 #include <timespec.h>
 #include <version-etc.h>
 #include <xalloc.h>
+#include <xreadlink.h>
 #include <binary-io.h>
 
 /* The official name of this program (e.g., no `g' prefix).  */
@@ -188,6 +189,7 @@ static struct option const longopts[] =
   {"new-file", 0, 0, 'N'},
   {"new-group-format", 1, 0, NEW_GROUP_FORMAT_OPTION},
   {"new-line-format", 1, 0, NEW_LINE_FORMAT_OPTION},
+  {"no-dereference", 0, 0, 'h'},
   {"no-ignore-file-name-case", 0, 0, NO_IGNORE_FILE_NAME_CASE_OPTION},
   {"normal", 0, 0, NORMAL_OPTION},
   {"old-group-format", 1, 0, OLD_GROUP_FORMAT_OPTION},
@@ -400,10 +402,7 @@ main (int argc, char **argv)
          break;
 
        case 'h':
-         /* Split the files into chunks for faster processing.
-            Usually does not change the result.
-
-            This currently has no effect.  */
+         no_dereference_symlinks = true;
          break;
 
        case 'H':
@@ -872,6 +871,7 @@ static char const * const option_help_msgid[] = {
   N_("-l, --paginate                pass output through `pr' to paginate it"),
   "",
   N_("-r, --recursive                 recursively compare any subdirectories 
found"),
+  N_("-h, --no-dereference            don't follow symbolic links"),
   N_("-N, --new-file                  treat absent files as empty"),
   N_("    --unidirectional-new-file   treat absent first files as empty"),
   N_("    --ignore-file-name-case     ignore case when comparing file names"),
@@ -1128,7 +1128,10 @@ compare_files (struct comparison const *parent,
                  set_mtime_to_now (&cmp.file[f].stat);
                }
            }
-         else if (stat (cmp.file[f].name, &cmp.file[f].stat) != 0)
+         else if ((no_dereference_symlinks
+                   ? lstat (cmp.file[f].name, &cmp.file[f].stat)
+                   : stat (cmp.file[f].name, &cmp.file[f].stat))
+                  != 0)
            cmp.file[f].desc = ERRNO_ENCODE (errno);
        }
     }
@@ -1182,7 +1185,10 @@ compare_files (struct comparison const *parent,
       if (STREQ (fnm, "-"))
        fatal ("cannot compare `-' to a directory");
 
-      if (stat (filename, &cmp.file[dir_arg].stat) != 0)
+      if ((no_dereference_symlinks
+          ? lstat (filename, &cmp.file[dir_arg].stat)
+          : stat (filename, &cmp.file[dir_arg].stat))
+         != 0)
        {
          perror_with_name (filename);
          status = EXIT_TROUBLE;
@@ -1229,8 +1235,10 @@ compare_files (struct comparison const *parent,
     }
   else if ((DIR_P (0) | DIR_P (1))
           || (parent
-              && (! S_ISREG (cmp.file[0].stat.st_mode)
-                  || ! S_ISREG (cmp.file[1].stat.st_mode))))
+              && !((S_ISREG (cmp.file[0].stat.st_mode)
+                    || S_ISLNK (cmp.file[0].stat.st_mode))
+                   && (S_ISREG (cmp.file[1].stat.st_mode)
+                       || S_ISLNK  (cmp.file[1].stat.st_mode)))))
     {
       if (cmp.file[0].desc == NONEXISTENT || cmp.file[1].desc == NONEXISTENT)
        {
@@ -1271,6 +1279,56 @@ compare_files (struct comparison const *parent,
          status = EXIT_FAILURE;
        }
     }
+  else if (S_ISLNK (cmp.file[0].stat.st_mode)
+          || S_ISLNK (cmp.file[1].stat.st_mode))
+    {
+      /* We get here only if we use lstat(), not stat().  */
+      assert (no_dereference_symlinks);
+
+      if (S_ISLNK (cmp.file[0].stat.st_mode)
+         && S_ISLNK (cmp.file[1].stat.st_mode))
+       {
+         /* Compare the values of the symbolic links.  */
+         char *link_value[2] = { NULL, NULL };
+
+         for (f = 0; f < 2; f++)
+           {
+             link_value[f] = xreadlink (cmp.file[f].name);
+             if (link_value[f] == NULL)
+               {
+                 perror_with_name (cmp.file[f].name);
+                 status = EXIT_TROUBLE;
+                 break;
+               }
+           }
+         if (status == EXIT_SUCCESS)
+           {
+             if (strcmp (link_value[0], link_value[1]) != 0)
+               {
+                 message ("Symbolic links %s and %s differ\n",
+                          cmp.file[0].name, cmp.file[1].name);
+                 /* This is a difference.  */
+                 status = EXIT_FAILURE;
+               }
+           }
+         for (f = 0; f < 2; f++)
+           free (link_value[f]);
+       }
+      else
+       {
+         /* We have two files that are not to be compared, because
+            one of them is a symbolic link and the other one is not.  */
+
+         message5 ("File %s is a %s while file %s is a %s\n",
+                   file_label[0] ? file_label[0] : cmp.file[0].name,
+                   file_type (&cmp.file[0].stat),
+                   file_label[1] ? file_label[1] : cmp.file[1].name,
+                   file_type (&cmp.file[1].stat));
+
+         /* This is a difference.  */
+         status = EXIT_FAILURE;
+       }
+    }
   else if (files_can_be_treated_as_binary
           && S_ISREG (cmp.file[0].stat.st_mode)
           && S_ISREG (cmp.file[1].stat.st_mode)
diff --git a/src/diff.h b/src/diff.h
index 795cc0c..e77343a 100644
--- a/src/diff.h
+++ b/src/diff.h
@@ -135,6 +135,9 @@ XTERN bool ignore_case;
 /* Ignore differences in case of letters in file names.  */
 XTERN bool ignore_file_name_case;
 
+/* Act on symbolic links themselves rather than on their target (-h).  */
+XTERN bool no_dereference_symlinks;
+
 /* File labels for `-c' output headers (--label).  */
 XTERN char *file_label[2];
 
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 9952d67..2f6ad53 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -8,6 +8,7 @@ TESTS = \
   help-version \
   function-line-vs-leading-space \
   label-vs-func        \
+  no-dereference \
   no-newline-at-eof \
   stdin
 
diff --git a/tests/no-dereference b/tests/no-dereference
new file mode 100644
index 0000000..1426beb
--- /dev/null
+++ b/tests/no-dereference
@@ -0,0 +1,169 @@
+#!/bin/sh
+# Test the --no-dereference option.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+
+echo 'Simple contents' > regular1
+echo 'Sample contents' > regular2
+echo 'Sample contents' > regular3
+ln -s regular1 symlink1
+ln -s regular1 symlink1bis
+ln -s regular2 symlink2
+ln -s regular3 symlink3
+
+# Non-recursive comparisons.
+
+# Test case 3: Compare regular file with regular file.
+diff --no-dereference regular1 regular2 > out
+test $? = 1 || fail=1
+cat <<EOF > expected || framework_failure_
+1c1
+< Simple contents
+---
+> Sample contents
+EOF
+compare expected out || fail=1
+
+# Test case 4: Compare regular file with symbolic link.
+diff --no-dereference regular1 symlink1 > out
+test $? = 1 || fail=1
+cat <<EOF > expected || framework_failure_
+File regular1 is a regular file while file symlink1 is a symbolic link
+EOF
+compare expected out || fail=1
+
+# Test case 5: Compare symbolic link with regular file.
+diff --no-dereference symlink1 regular1 > out
+test $? = 1 || fail=1
+cat <<EOF > expected || framework_failure_
+File symlink1 is a symbolic link while file regular1 is a regular file
+EOF
+compare expected out || fail=1
+
+# Test case 6: Compare symbolic links with same value.
+diff --no-dereference symlink1 symlink1bis > out
+test $? = 0 || fail=1
+compare /dev/null out || fail=1
+
+# Test case 7: Compare symbolic links with different value and different target
+# contents.
+diff --no-dereference symlink1 symlink2 > out
+test $? = 1 || fail=1
+cat <<EOF > expected || framework_failure_
+Symbolic links symlink1 and symlink2 differ
+EOF
+compare expected out || fail=1
+
+# Test case 8: Compare symbolic links with different value and same target
+# contents.
+diff --no-dereference symlink2 symlink3 > out
+test $? = 1 || fail=1
+cat <<EOF > expected || framework_failure_
+Symbolic links symlink2 and symlink3 differ
+EOF
+compare expected out || fail=1
+
+# Recursive comparisons.
+
+# Test case 1: Compare symbolic link with nonexistent file.
+mkdir subdir1a
+mkdir subdir1b
+ln -s nonexistent subdir1a/foo
+ln -s ../regular1 subdir1a/bar
+diff -r --no-dereference subdir1a subdir1b > out
+test $? = 1 || fail=1
+cat <<EOF > expected || framework_failure_
+Only in subdir1a: bar
+Only in subdir1a: foo
+EOF
+compare expected out || fail=1
+
+# Test case 1: Compare nonexistent file with symbolic link.
+mkdir subdir2a
+mkdir subdir2b
+ln -s nonexistent subdir2b/foo
+ln -s ../regular1 subdir2b/bar
+diff -r --no-dereference subdir2a subdir2b > out
+test $? = 1 || fail=1
+cat <<EOF > expected || framework_failure_
+Only in subdir2b: bar
+Only in subdir2b: foo
+EOF
+compare expected out || fail=1
+
+# Test case 3: Compare regular file with regular file.
+mkdir subdir3a
+mkdir subdir3b
+cp regular1 subdir3a/foo
+cp regular2 subdir3b/foo
+diff -r --no-dereference subdir3a subdir3b > out
+test $? = 1 || fail=1
+cat <<EOF > expected || framework_failure_
+diff -r --no-dereference subdir3a/foo subdir3b/foo
+1c1
+< Simple contents
+---
+> Sample contents
+EOF
+compare expected out || fail=1
+
+# Test case 4: Compare regular file with symbolic link.
+mkdir subdir4a
+mkdir subdir4b
+cp regular1 subdir4a/foo
+ln -s ../regular1 subdir4b/foo
+diff -r --no-dereference subdir4a subdir4b > out
+test $? = 1 || fail=1
+cat <<EOF > expected || framework_failure_
+File subdir4a/foo is a regular file while file subdir4b/foo is a symbolic link
+EOF
+compare expected out || fail=1
+
+# Test case 5: Compare symbolic link with regular file.
+mkdir subdir5a
+mkdir subdir5b
+ln -s ../regular1 subdir5a/foo
+cp regular1 subdir5b/foo
+diff -r --no-dereference subdir5a subdir5b > out
+test $? = 1 || fail=1
+cat <<EOF > expected || framework_failure_
+File subdir5a/foo is a symbolic link while file subdir5b/foo is a regular file
+EOF
+compare expected out || fail=1
+
+# Test case 6: Compare symbolic links with same value.
+mkdir subdir6a
+mkdir subdir6b
+ln -s ../regular1 subdir6a/foo
+ln -s ../regular1 subdir6b/foo
+diff -r --no-dereference subdir6a subdir6b > out
+test $? = 0 || fail=1
+compare /dev/null out || fail=1
+
+# Test case 7: Compare symbolic links with different value and different target
+# contents.
+mkdir subdir7a
+mkdir subdir7b
+ln -s ../regular1 subdir7a/foo
+ln -s ../regular2 subdir7b/foo
+diff -r --no-dereference subdir7a subdir7b > out
+test $? = 1 || fail=1
+cat <<EOF > expected || framework_failure_
+Symbolic links subdir7a/foo and subdir7b/foo differ
+EOF
+compare expected out || fail=1
+
+# Test case 8: Compare symbolic links with different value and same target
+# contents.
+mkdir subdir8a
+mkdir subdir8b
+ln -s ../regular2 subdir8a/foo
+ln -s ../regular3 subdir8b/foo
+diff -r --no-dereference subdir8a subdir8b > out
+test $? = 1 || fail=1
+cat <<EOF > expected || framework_failure_
+Symbolic links subdir8a/foo and subdir8b/foo differ
+EOF
+compare expected out || fail=1
+
+Exit $fail
-- 
1.6.3.2

0001-New-option-h-no-dereference.patch
Description: Text Data