Paul Eggert wrote: > Thanks for the patch! I would like to fold something like > this in. > > Two thoughts. > > First, many programs (e.g., cp) have three options: > -H -L -P. These distinctions seem to be useful for diff. > This suggests that the new --no-dereference flag should > be -P, not -h. -h has a subtly different meaning from > -P, in programs like chown. > > Changing the patch to use -P instead of -h should be easy
But -P currently is equivalent to --unidirectional-new-file. If we reassign -P immediately, we can silently break some users' scripts. IMO we need to first deprecate the option -P and leave it that way for some time (perhaps 1 year) before it can be used for a different purpose. Find attached the revised set of patches. > Second, and more important, the output of --no-dereference > should be something that we can feed to an (augmented) > 'patch' so that it can alter a copy of the old tree, > symlinks and all, so that it looks like the new tree, > symlinks and all. This suggests that the output of > 'diff' needs to contain the symlink contents, and needs > to distinguish symlinks from regular files, so that 'patch' > can reconstruct the symlinks. Ideally, yes. But this can come later, for two reasons: 1) There are already other cases where 'diff' emits such lines that 'patch' does not understand: $ mkdir a $ ln -s /dev/tty a/foo $ mkdir b $ diff -r a b Only in a: foo $ ln -s /dev/zero b/foo $ diff -r a b File a/foo is a character special file while file b/foo is a character special file 2) 'patch' does not break through this additional lines. It simply ignores them. Bruno
>From 75736b4292dec634dd6c6f0d25f6f05468bce9d9 Mon Sep 17 00:00:00 2001 From: Bruno Haible <[email protected]> Date: Sun, 8 Jan 2012 02:29:24 +0100 Subject: [PATCH 1/2] Deprecate old option -P. * src/diff.c (UNIDIRECTIONAL_NEW_FILE_OPTION): New enumeration item. (longopts): Use it instead of '-P'. (main): Give an error message if option -P is used. --- src/diff.c | 9 +++++++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/diff.c b/src/diff.c index 004654e..c546a75 100644 --- a/src/diff.c +++ b/src/diff.c @@ -128,6 +128,7 @@ enum SUPPRESS_COMMON_LINES_OPTION, TABSIZE_OPTION, TO_FILE_OPTION, + UNIDIRECTIONAL_NEW_FILE_OPTION, /* These options must be in sequence. */ UNCHANGED_LINE_FORMAT_OPTION, @@ -210,7 +211,7 @@ static struct option const longopts[] = {"to-file", 1, 0, TO_FILE_OPTION}, {"unchanged-group-format", 1, 0, UNCHANGED_GROUP_FORMAT_OPTION}, {"unchanged-line-format", 1, 0, UNCHANGED_LINE_FORMAT_OPTION}, - {"unidirectional-new-file", 0, 0, 'P'}, + {"unidirectional-new-file", 0, 0, UNIDIRECTIONAL_NEW_FILE_OPTION}, {"unified", 2, 0, 'U'}, {"version", 0, 0, 'v'}, {"width", 1, 0, 'W'}, @@ -452,7 +453,7 @@ main (int argc, char **argv) break; case 'P': - unidirectional_new_file = true; + fatal ("option -P no longer supported, use --unidirectional-new-file for the old meaning"); break; case 'q': @@ -605,6 +606,10 @@ main (int argc, char **argv) specify_value (&to_file, optarg, "--to-file"); break; + case UNIDIRECTIONAL_NEW_FILE_OPTION: + unidirectional_new_file = true; + break; + case UNCHANGED_LINE_FORMAT_OPTION: case OLD_LINE_FORMAT_OPTION: case NEW_LINE_FORMAT_OPTION: -- 1.6.3.2
>From 1b1bf9d9663b33b4f4eb6040420cb99442a851ad Mon Sep 17 00:00:00 2001 From: Bruno Haible <[email protected]> Date: Sat, 7 Jan 2012 00:57:29 +0100 Subject: [PATCH 2/2] New option --no-dereference. * src/diff.h (no_dereference_symlinks): New variable. * src/diff.c: Include xreadlink.h. (longopts): Add --no-dereference option. (main): Accept --no-dereference option. (option_help_msgid): Mention the --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 --no-dereference option. * tests/no-dereference: New file. * tests/Makefile.am (TESTS): Add it. --- bootstrap.conf | 3 + doc/diffutils.texi | 8 +++ src/diff.c | 74 +++++++++++++++++++++- src/diff.h | 4 + tests/Makefile.am | 1 + tests/no-dereference | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 255 insertions(+), 4 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..783f084 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 +@option{--no-dereference} (@option{-P}). 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 @@ -3848,6 +3853,9 @@ file in if-then-else format. @xref{Line Group Formats}. Use @var{format} to output a line taken from just the second file in if-then-else format. @xref{Line Formats}. +@item --no-dereference +Act on symbolic links themselves instead of what they point to. + @item --old-group-format=@var{format} Use @var{format} to output a group of lines taken from just the first file in if-then-else format. @xref{Line Group Formats}. diff --git a/src/diff.c b/src/diff.c index c546a75..67b3109 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). */ @@ -120,6 +121,7 @@ enum INHIBIT_HUNK_MERGE_OPTION, LEFT_COLUMN_OPTION, LINE_FORMAT_OPTION, + NO_DEREFERENCE_OPTION, NO_IGNORE_FILE_NAME_CASE_OPTION, NORMAL_OPTION, SDIFF_MERGE_ASSIST_OPTION, @@ -189,6 +191,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, NO_DEREFERENCE_OPTION}, {"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}, @@ -565,6 +568,10 @@ main (int argc, char **argv) specify_value (&line_format[i], optarg, "--line-format"); break; + case NO_DEREFERENCE_OPTION: + no_dereference_symlinks = true; + break; + case NO_IGNORE_FILE_NAME_CASE_OPTION: ignore_file_name_case = false; break; @@ -877,6 +884,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_(" --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"), @@ -1133,7 +1141,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); } } @@ -1187,7 +1198,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; @@ -1234,8 +1248,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) { @@ -1276,6 +1292,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..05029a3 100644 --- a/src/diff.h +++ b/src/diff.h @@ -135,6 +135,10 @@ 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 + (--no-dereference). */ +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
