diff: add --no-dereference

Should make test no-dereference pass.

Author: sylvestre

src/diff.rs

@@ -53,12 +53,13 @@ pub fn main(opts: Peekable<ArgsOs>) -> ExitCode {
             params.from = from;
         }
     }
-    let both_directories = params.from != "-"
-        && params.to != "-"
-        && Path::new(&params.from).is_dir()
-        && Path::new(&params.to).is_dir();
-    let outcome = if both_directories {
-        dir_diff::compare_directories(&params, &option_args)
+    let both_directories = Path::new(&params.from).is_dir() && Path::new(&params.to).is_dir();
+    // Directories, and files whose symbolic links are not followed, are
+    // compared by type before their contents.
+    let compare_types =
+        params.from != "-" && params.to != "-" && (both_directories || params.no_dereference);
+    let outcome = if compare_types {
+        dir_diff::compare(&params, &option_args)
     } else {
         compare_files(&params, None)
     };

src/dir_diff.rs

@@ -10,28 +10,31 @@ use std::borrow::Cow;
 use std::cmp::Ordering;
 use std::ffi::{OsStr, OsString};
 use std::fs::{self, Metadata};
-use std::io;
+use std::io::{self, Write};
 use std::path::{Path, PathBuf};
 
 use crate::diff::{compare_files, Outcome};
 use crate::params::{MissingFile, Params};
 use crate::utils::{
     fold_file_name_case, quote_file_name, quote_shell_argument, report_failure_to_read_input_file,
+    single_quote,
 };
 
-/// Compares the directories `params.from` and `params.to`. `option_args` are
-/// the command line options, repeated in the header printed before the
-/// differences of each pair of files.
-pub fn compare_directories(params: &Params, option_args: &[OsString]) -> Outcome {
+/// Compares `params.from` and `params.to` according to their types: the
+/// entries of directories, the targets of symbolic links that are not
+/// followed, or the contents of files. `option_args` are the command line
+/// options, repeated in the header printed before the differences of each
+/// pair of files found in directories.
+pub fn compare(params: &Params, option_args: &[OsString]) -> Outcome {
     let mut walker = DirectoryWalker {
         params,
         option_args,
         ancestors: Vec::new(),
     };
-    walker.compare_directories(&params.from, &params.to, Level::Top)
+    walker.compare_entries(&params.from, &params.to, Level::Top)
 }
 
-/// Whether directories are the operands of diff, or nested in them.
+/// Whether files are the operands of diff, or found in directories.
 #[derive(Clone, Copy, Eq, PartialEq)]
 enum Level {
     Top,
@@ -83,17 +86,17 @@ impl DirectoryWalker<'_> {
         for entry in entries {
             let entry_outcome = match entry {
                 Entry::Both(from, to) => {
-                    self.compare_entries(&join(from_dir, from), &join(to_dir, to))
+                    self.compare_entries(&join(from_dir, from), &join(to_dir, to), Level::Nested)
                 }
                 Entry::FromOnly(name) if self.params.missing_file == MissingFile::Empty => {
-                    self.compare_entries(&join(from_dir, name), &join(to_dir, name))
+                    self.compare_entries(&join(from_dir, name), &join(to_dir, name), Level::Nested)
                 }
                 Entry::FromOnly(name) => report_only_in(from_dir, name),
                 Entry::ToOnly(name) if self.params.missing_file == MissingFile::Error => {
                     report_only_in(to_dir, name)
                 }
                 Entry::ToOnly(name) => {
-                    self.compare_entries(&join(from_dir, name), &join(to_dir, name))
+                    self.compare_entries(&join(from_dir, name), &join(to_dir, name), Level::Nested)
                 }
             };
             outcome = outcome.max(entry_outcome);
@@ -148,7 +151,7 @@ impl DirectoryWalker<'_> {
 
     /// Compares two entries of the compared directories, one of which may be
     /// missing when missing files are treated as empty.
-    fn compare_entries(&mut self, from: &OsStr, to: &OsStr) -> Outcome {
+    fn compare_entries(&mut self, from: &OsStr, to: &OsStr, level: Level) -> Outcome {
         let (Ok(from_metadata), Ok(to_metadata)) = (self.metadata(from), self.metadata(to)) else {
             return Outcome::Trouble;
         };
@@ -158,8 +161,8 @@ impl DirectoryWalker<'_> {
             {
                 if same_file::is_same_file(from, to).unwrap_or(false) {
                     Outcome::Identical
-                } else if self.params.recursive {
-                    self.compare_directories(from, to, Level::Nested)
+                } else if level == Level::Top || self.params.recursive {
+                    self.compare_directories(from, to, level)
                 } else {
                     println!(
                         "Common subdirectories: {} and {}",
@@ -169,6 +172,11 @@ impl DirectoryWalker<'_> {
                     Outcome::Identical
                 }
             }
+            (Some(from_metadata), Some(to_metadata))
+                if from_metadata.is_symlink() && to_metadata.is_symlink() =>
+            {
+                self.compare_symbolic_links(from, to)
+            }
             (Some(from_metadata), Some(to_metadata))
                 if !(from_metadata.is_file() && to_metadata.is_file()) =>
             {
@@ -194,10 +202,53 @@ impl DirectoryWalker<'_> {
                     Outcome::Identical
                 }
             }
+            // The operands themselves are compared without a header, as when
+            // they are not directories.
+            _ if level == Level::Top => compare_files(self.params, None),
             _ => self.compare_files(from, to),
         }
     }
 
+    /// Compares the targets of two symbolic links.
+    fn compare_symbolic_links(&self, from: &OsStr, to: &OsStr) -> Outcome {
+        let (from_target, to_target) = match (fs::read_link(from), fs::read_link(to)) {
+            (Ok(from_target), Ok(to_target)) => (from_target, to_target),
+            (Err(error), _) => {
+                self.report_error(from, &error);
+                return Outcome::Trouble;
+            }
+            (_, Err(error)) => {
+                self.report_error(to, &error);
+                return Outcome::Trouble;
+            }
+        };
+        if from_target == to_target {
+            if self.params.report_identical_files {
+                println!(
+                    "Files {} and {} are identical",
+                    from.to_string_lossy(),
+                    to.to_string_lossy()
+                );
+            }
+            return Outcome::Identical;
+        }
+        let mut message = b"Symbolic links ".to_vec();
+        for (link, target, separator) in [
+            (from, from_target.as_os_str(), &b" and "[..]),
+            (to, to_target.as_os_str(), b" differ\n"),
+        ] {
+            message.extend(single_quote(link.as_encoded_bytes()));
+            message.extend_from_slice(b" -> ");
+            message.extend(single_quote(target.as_encoded_bytes()));
+            message.extend_from_slice(separator);
+        }
+        if io::stdout().write_all(&message).is_err() {
+            return Outcome::Trouble;
+        }
+        Outcome::Different
+    }
+
+    /// Compares two files found in the compared directories.
     fn compare_files(&self, from: &OsStr, to: &OsStr) -> Outcome {
         let params = Params {
             from: from.to_os_string(),
@@ -220,7 +271,12 @@ impl DirectoryWalker<'_> {
     /// Returns the metadata of `path`, `None` if it is missing on purpose
     /// (-N), or an error after reporting it.
     fn metadata(&self, path: &OsStr) -> Result<Option<Metadata>, ()> {
-        match fs::metadata(path) {
+        let metadata = if self.params.no_dereference {
+            fs::symlink_metadata(path)
+        } else {
+            fs::metadata(path)
+        };
+        match metadata {
             Ok(metadata) => Ok(Some(metadata)),
             Err(error)
                 if error.kind() == io::ErrorKind::NotFound
@@ -305,6 +361,10 @@ fn join(dir: &OsStr, name: &OsStr) -> OsString {
 /// Describes the type of a file, as in "File X is a fifo while ...".
 fn file_kind(metadata: &Metadata) -> &'static str {
     let file_type = metadata.file_type();
+    // Only seen with --no-dereference, as symbolic links are followed otherwise.
+    if file_type.is_symlink() {
+        return "symbolic link";
+    }
     if file_type.is_dir() {
         return "directory";
     }

src/params.rs

@@ -58,6 +58,7 @@ pub struct Params {
     /// (`-S`).
     pub starting_file: Option<OsString>,
     pub ignore_file_name_case: bool,
+    pub no_dereference: bool,
 }
 
 /// The pattern of `-p`, matching the first line of C functions.
@@ -85,6 +86,7 @@ impl Default for Params {
             recursive: false,
             starting_file: None,
             ignore_file_name_case: false,
+            no_dereference: false,
         }
     }
 }
@@ -193,6 +195,10 @@ pub fn parse_params<I: Iterator<Item = OsString>>(
             push_function_line_pattern(&mut params, &pattern.to_string_lossy())?;
             continue;
         }
+        if param == "--no-dereference" {
+            params.no_dereference = true;
+            continue;
+        }
         if param == "--ignore-file-name-case" {
             params.ignore_file_name_case = true;
             continue;
@@ -363,11 +369,20 @@ pub fn parse_params<I: Iterator<Item = OsString>>(
     let mut from_path: PathBuf = PathBuf::from(&params.from);
     let mut to_path: PathBuf = PathBuf::from(&params.to);
 
-    if from_path.is_dir() && to_path.is_file() {
+    // Symbolic links that are not followed are compared like files.
+    let is_file = |path: &Path| {
+        if params.no_dereference {
+            path.symlink_metadata()
+                .is_ok_and(|metadata| !metadata.is_dir())
+        } else {
+            path.is_file()
+        }
+    };
+    if from_path.is_dir() && is_file(&to_path) {
         let name = directory_entry(&from_path, to_path.file_name().unwrap(), &params);
         from_path.push(name);
         params.from = from_path.into_os_string();
-    } else if from_path.is_file() && to_path.is_dir() {
+    } else if is_file(&from_path) && to_path.is_dir() {
         let name = directory_entry(&to_path, from_path.file_name().unwrap(), &params);
         to_path.push(name);
         params.to = to_path.into_os_string();

src/utils.rs

@@ -114,6 +114,22 @@ pub fn write_file_header(output: &mut Vec<u8>, marker: &str, name: &OsStr, times
     output.push(b'\n');
 }
 
+/// Puts `name` between single quotes, writing a quote in it as `'\''`.
+#[must_use]
+pub fn single_quote(name: &[u8]) -> Vec<u8> {
+    let mut quoted = Vec::with_capacity(name.len() + 2);
+    quoted.push(b'\'');
+    for &byte in name {
+        if byte == b'\'' {
+            quoted.extend_from_slice(br"'\''");
+        } else {
+            quoted.push(byte);
+        }
+    }
+    quoted.push(b'\'');
+    quoted
+}
+
 /// Quotes a command line argument the way a shell would need it, for the
 /// headers that repeat the options of a directory comparison.
 #[must_use]
@@ -457,6 +473,13 @@ mod tests {
             assert_eq!(quoted_arg("don't"), "\"don't\"");
             assert_eq!(quoted_arg("don't $pay"), "'don'\\''t $pay'");
         }
+
+        #[test]
+        fn single_quotes() {
+            assert_eq!(single_quote(b"plain"), b"'plain'");
+            assert_eq!(single_quote(b"it's"), br"'it'\''s'");
+            assert_eq!(single_quote(b""), b"''");
+        }
     }
 
     mod write_line {

tests/by-util/test_diff.rs

@@ -655,6 +655,118 @@ fn directory_name_matching() -> Result<(), Box<dyn std::error::Error>> {
     Ok(())
 }
 
+#[cfg(unix)]
+#[test]
+fn no_dereference() -> Result<(), Box<dyn std::error::Error>> {
+    use std::os::unix::fs::symlink;
+
+    let tmp_dir = tempdir()?;
+    let dir = tmp_dir.path();
+    std::fs::write(dir.join("plain"), "text\n")?;
+    std::fs::write(dir.join("other"), "text\n")?;
+    symlink("plain", dir.join("to-plain"))?;
+    symlink("plain", dir.join("also-to-plain"))?;
+    symlink("other", dir.join("to-other"))?;
+    let path = |name: &str| dir.join(name).to_str().unwrap().to_string();
+
+    let mut cmd = cargo_bin_cmd!("diffutils");
+    cmd.arg("diff").arg(path("plain")).arg(path("to-other"));
+    cmd.assert().code(predicate::eq(0)).stdout("");
+
+    let mut cmd = cargo_bin_cmd!("diffutils");
+    cmd.arg("diff")
+        .arg("--no-dereference")
+        .arg(path("plain"))
+        .arg(path("to-other"));
+    cmd.assert().code(predicate::eq(1)).stdout(format!(
+        "File {} is a regular file while file {} is a symbolic link\n",
+        path("plain"),
+        path("to-other")
+    ));
+
+    let mut cmd = cargo_bin_cmd!("diffutils");
+    cmd.arg("diff")
+        .arg("--no-dereference")
+        .arg(path("to-plain"))
+        .arg(path("also-to-plain"));
+    cmd.assert().code(predicate::eq(0)).stdout("");
+
+    let mut cmd = cargo_bin_cmd!("diffutils");
+    cmd.arg("diff")
+        .arg("--no-dereference")
+        .arg(path("to-plain"))
+        .arg(path("to-other"));
+    cmd.assert().code(predicate::eq(1)).stdout(format!(
+        "Symbolic links '{}' -> 'plain' and '{}' -> 'other' differ\n",
+        path("to-plain"),
+        path("to-other")
+    ));
+
+    // In directories, dangling links are entries like any other.
+    std::fs::create_dir(dir.join("left"))?;
+    std::fs::create_dir(dir.join("right"))?;
+    symlink("missing", dir.join("left/dangling"))?;
+    symlink("../plain", dir.join("right/dangling"))?;
+    let mut cmd = cargo_bin_cmd!("diffutils");
+    cmd.arg("diff")
+        .arg("-r")
+        .arg("--no-dereference")
+        .arg(path("left"))
+        .arg(path("right"));
+    cmd.assert().code(predicate::eq(1)).stdout(format!(
+        "Symbolic links '{0}/dangling' -> 'missing' and '{1}/dangling' -> '../plain' differ\n",
+        path("left"),
+        path("right")
+    ));
+
+    let mut cmd = cargo_bin_cmd!("diffutils");
+    cmd.arg("diff")
+        .arg("-r")
+        .arg(path("left"))
+        .arg(path("right"));
+    cmd.assert()
+        .code(predicate::eq(2))
+        .stderr(predicate::str::contains(
+            "dangling: No such file or directory",
+        ));
+
+    // Links with the same target are identical files.
+    let mut cmd = cargo_bin_cmd!("diffutils");
+    cmd.arg("diff")
+        .arg("-s")
+        .arg("--no-dereference")
+        .arg(path("to-plain"))
+        .arg(path("also-to-plain"));
+    cmd.assert().code(predicate::eq(0)).stdout(format!(
+        "Files {} and {} are identical\n",
+        path("to-plain"),
+        path("also-to-plain")
+    ));
+
+    // A file compared with a directory is looked up in it, and that entry is
+    // not followed either.
+    std::fs::write(dir.join("left/plain"), "text\n")?;
+    symlink("../other", dir.join("left/to-other"))?;
+    let mut cmd = cargo_bin_cmd!("diffutils");
+    cmd.arg("diff")
+        .arg("--no-dereference")
+        .arg(path("plain"))
+        .arg(path("left"));
+    cmd.assert().code(predicate::eq(0)).stdout("");
+
+    let mut cmd = cargo_bin_cmd!("diffutils");
+    cmd.arg("diff")
+        .arg("--no-dereference")
+        .arg(path("left"))
+        .arg(path("to-other"));
+    cmd.assert().code(predicate::eq(1)).stdout(format!(
+        "Symbolic links '{}/to-other' -> '../other' and '{}' -> 'other' differ\n",
+        path("left"),
+        path("to-other")
+    ));
+    Ok(())
+}
+
 #[test]
 fn compare_file_to_directory() -> Result<(), Box<dyn std::error::Error>> {
     let tmp_dir = tempdir()?;