readlink: implement -f

[andrewliebenow]

No description provided.

[jgarzik]

by a quick look, it feels like ftw should be able to do this?

[fox0]

From 2a19daa37325b26bdb5af0cd1ec4f2f9a3967991 Mon Sep 17 00:00:00 2001
From: fox0 <[email protected]>
Date: Sun, 20 Oct 2024 15:09:38 +0700
Subject: [PATCH] readlink: #[cfg(feature = "full")]

---
 tree/readlink.rs | 120 ++++++++++++++++++++++++++---------------------
 1 file changed, 66 insertions(+), 54 deletions(-)

diff --git a/tree/readlink.rs b/tree/readlink.rs
index 7a35e74..0544372 100644
--- a/tree/readlink.rs
+++ b/tree/readlink.rs
@@ -13,7 +13,9 @@ use plib::PROJECT_NAME;
 use std::error::Error;
 use std::fs;
 use std::io::{stderr, stdout, ErrorKind, Write};
-use std::path::{Component, Path, PathBuf};
+#[cfg(feature = "full")]
+use std::path::Component;
+use std::path::{Path, PathBuf};
 
 /// readlink — display the contents of a symbolic link
 #[derive(Parser)]
@@ -25,6 +27,7 @@ struct Args {
 
     // Not POSIX, but implemented by BusyBox, FreeBSD, GNU Core Utilities, toybox, and others
     /// Canonicalize the provided path, resolving symbolic links repeatedly if needed. The absolute path of the resolved file is printed.
+    #[cfg(feature = "full")]
     #[arg(short = 'f')]
     canonicalize: bool,
 
@@ -43,12 +46,19 @@ struct Args {
 // Behavior of "readlink -f /non-existent-directory/non-existent-file" does not vary
 // All implementations: print nothing, exit code 1
 fn do_readlink(args: Args) -> Result<String, String> {
+    #[cfg(feature = "full")]
     let Args {
         no_newline,
         canonicalize,
         verbose,
         pathname,
     } = args;
+    #[cfg(not(feature = "full"))]
+    let Args {
+        no_newline,
+        verbose,
+        pathname,
+    } = args;
 
     let pathname_path = pathname.as_path();
 
@@ -98,10 +108,11 @@ fn do_readlink(args: Args) -> Result<String, String> {
         }
     };
 
+    #[cfg(feature = "full")]
     if canonicalize {
         let recursively_resolved_path_buf = recursive_resolve(pathname_path.to_owned())?;
 
-        match fs::canonicalize(recursively_resolved_path_buf.as_path()) {
+        return match fs::canonicalize(recursively_resolved_path_buf.as_path()) {
             Ok(pa) => format_returned_path(pa.as_path()),
             Err(er) => {
                 let mut components = recursively_resolved_path_buf.components();
@@ -140,65 +151,32 @@ fn do_readlink(args: Args) -> Result<String, String> {
                     map_io_error(&er)
                 }
             }
-        }
-    } else {
-        match fs::symlink_metadata(pathname_path) {
-            Ok(me) => {
-                if !me.is_symlink() {
-                    // POSIX says:
-                    // "If file does not name a symbolic link, readlink shall write a diagnostic message to standard error and exit with non-zero status."
-                    // However, this is violated by almost all implementations
-                    return if verbose {
-                        format_error("Not a symbolic link", None)
-                    } else {
-                        Err(String::new())
-                    };
-                }
-
-                match fs::read_link(pathname_path) {
-                    Ok(pa) => format_returned_path(pa.as_path()),
-                    Err(er) => map_io_error(&er),
-                }
-            }
-            Err(er) => map_io_error(&er),
-        }
+        };
     }
-}
-
-fn main() -> Result<(), Box<dyn std::error::Error>> {
-    // parse command line arguments
-    let args = Args::parse();
-
-    setlocale(LocaleCategory::LcAll, "");
-    textdomain(PROJECT_NAME)?;
-    bind_textdomain_codeset(PROJECT_NAME, "UTF-8")?;
-
-    let exit_code = match do_readlink(args) {
-        Ok(output) => {
-            let mut stdout_lock = stdout().lock();
-
-            write!(stdout_lock, "{output}").unwrap();
-
-            stdout_lock.flush().unwrap();
-
-            0_i32
-        }
-        Err(error_description) => {
-            if !error_description.is_empty() {
-                let mut stderr_lock = stderr().lock();
 
-                writeln!(&mut stderr_lock, "readlink: {error_description}").unwrap();
-
-                stderr_lock.flush().unwrap();
+    match fs::symlink_metadata(pathname_path) {
+        Ok(me) => {
+            if !me.is_symlink() {
+                // POSIX says:
+                // "If file does not name a symbolic link, readlink shall write a diagnostic message to standard error and exit with non-zero status."
+                // However, this is violated by almost all implementations
+                return if verbose {
+                    format_error("Not a symbolic link", None)
+                } else {
+                    Err(String::new())
+                };
             }
 
-            1_i32
+            match fs::read_link(pathname_path) {
+                Ok(pa) => format_returned_path(pa.as_path()),
+                Err(er) => map_io_error(&er),
+            }
         }
-    };
-
-    std::process::exit(exit_code);
+        Err(er) => map_io_error(&er),
+    }
 }
 
+#[cfg(feature = "full")]
 fn recursive_resolve(starting_path_buf: PathBuf) -> Result<PathBuf, String> {
     let mut current_path_buf = starting_path_buf;
 
@@ -239,3 +217,37 @@ fn recursive_resolve(starting_path_buf: PathBuf) -> Result<PathBuf, String> {
 
     Ok(current_path_buf)
 }
+
+fn main() -> Result<(), Box<dyn std::error::Error>> {
+    // parse command line arguments
+    let args = Args::parse();
+
+    setlocale(LocaleCategory::LcAll, "");
+    textdomain(PROJECT_NAME)?;
+    bind_textdomain_codeset(PROJECT_NAME, "UTF-8")?;
+
+    let exit_code = match do_readlink(args) {
+        Ok(output) => {
+            let mut stdout_lock = stdout().lock();
+
+            write!(stdout_lock, "{output}").unwrap();
+
+            stdout_lock.flush().unwrap();
+
+            0_i32
+        }
+        Err(error_description) => {
+            if !error_description.is_empty() {
+                let mut stderr_lock = stderr().lock();
+
+                writeln!(&mut stderr_lock, "readlink: {error_description}").unwrap();
+
+                stderr_lock.flush().unwrap();
+            }
+
+            1_i32
+        }
+    };
+
+    std::process::exit(exit_code);
+}
-- 
2.39.2

[jgarzik]

re @fox0: Making this option configurable is an understandable suggestion. However, in this case, wide support by FreeBSD and Linux suggests this is OK, by virtue of our rule in readme:

Popular ... options will be supported by virtue of the "don't break scripts" rule. Unpopular options will not be implemented, to prevent bloat.

This broad OS support of option -f also suggests it may appear in a future POSIX specification.

Merging the option without an ifdef (my C lingo for rust cfg) is preferable. We assume here is the option is "popular and common", and accept the cost of option and prefer fewer compile variants to test (with/without cfg).

[andrewliebenow]

by a quick look, it feels like ftw should be able to do this?

I wasn't familiar with that part of the codebase. Just took at look at this function, which I think may be what you're referring to:

posixutils-rs/ftw/src/lib.rs

Lines 483 to 585 in a08d82d

	fn process_file<'a, F, H>(
	path_stack: &'a [Rc<[libc::c_char]>],
	dir_fd: &'a FileDescriptor,
	entry_filename: Rc<[libc::c_char]>,
	follow_symlinks: bool,
	is_dot_or_double_dot: bool,
	file_handler: &mut F,
	err_reporter: &mut H,
	) -> ProcessFileResult<'a>
	where
	F: FnMut(Entry<'_>) -> Result<bool, ()>,
	H: FnMut(Entry<'_>, Error),
	{
	// Get the metadata for the file without following symlinks.
	let entry_symlink_metadata = match Metadata::new(
	dir_fd.fd,
	unsafe { CStr::from_ptr(entry_filename.as_ptr()) },
	false,
	) {
	Ok(md) => md,
	Err(e) => {
	err_reporter(
	Entry::new(dir_fd, path_stack, entry_filename, None),
	Error::new(e, ErrorKind::Stat),
	);
	return ProcessFileResult::NotProcessed;
	}
	};
	let is_symlink = entry_symlink_metadata.file_type() == FileType::SymbolicLink;
	// If `follow_symlinks` is enabled, read the location of the symlink and use the metadata of the
	// pointed file.
	let (entry_readlink, entry_metadata) = if is_symlink && follow_symlinks {
	let read_link = match read_link_at(dir_fd.fd, entry_filename.as_ptr()) {
	Ok(p) => p,
	Err(e) => {
	err_reporter(
	Entry::new(dir_fd, path_stack, entry_filename, None),
	Error::new(e, ErrorKind::ReadLink),
	);
	return ProcessFileResult::NotProcessed;
	}
	};
	match Metadata::new(
	dir_fd.fd,
	unsafe { CStr::from_ptr(entry_filename.as_ptr()) },
	true,
	) {
	Ok(md) => (Some(read_link), md),
	Err(e) => {
	if e.kind() == io::ErrorKind::NotFound {
	// Don't treat dangling links as an error, use the metadata of the original
	(Some(read_link), entry_symlink_metadata)
	} else {
	err_reporter(
	Entry::new(dir_fd, path_stack, entry_filename, None),
	Error::new(e, ErrorKind::Stat),
	);
	return ProcessFileResult::NotProcessed;
	}
	}
	}
	} else {
	(None, entry_symlink_metadata)
	};
	let mut entry = Entry::new(dir_fd, path_stack, entry_filename, Some(entry_metadata));
	entry.is_symlink = Some(is_symlink);
	entry.read_link = entry_readlink;
	let file_handler_result = file_handler(entry.clone());
	// Always skip . and .. to avoid infinite loops
	if is_dot_or_double_dot {
	return ProcessFileResult::Skipped;
	}
	match file_handler_result {
	Ok(true) => {
	let entry_metadata = entry.metadata.as_ref().unwrap();
	if entry_metadata.is_dir() {
	// Is the directory searchable?
	if entry_metadata.is_executable() {
	ProcessFileResult::ProcessedDirectory(entry)
	} else {
	// "Permission denied" error. `io::ErrorKind::PermissionDenied` uses
	// lowercase for "permission" in the error message so don't use that here.
	let e = io::Error::from_raw_os_error(libc::EACCES);
	err_reporter(entry, Error::new(e, ErrorKind::DirNotSearchable));
	ProcessFileResult::NotProcessed
	}
	} else {
	ProcessFileResult::ProcessedFile
	}
	}
	Ok(false) => {
	// `false` means skip the directory
	ProcessFileResult::Skipped
	}
	Err(_) => ProcessFileResult::NotProcessed,
	}
	}

I'm not sure how much code could be saved by trying to use that abstraction. It doesn't look super promising to me, but I can take another look if this is a concern.