readlink: implement -f

[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 <15684995+fox0@users.noreply.github.com>
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:

https://github.com/rustcoreutils/posixutils-rs/blob/a08d82dff64d97e752909d413a0cbaba40bf59c9/ftw/src/lib.rs#L483-L585

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.