sort: open the output file at the beginning

This causes us to print an error message about an invalid output file
right at the start of the invocation, but *after*  verifying other arguments.

src/uu/sort/src/ext_sort.rs

@@ -28,6 +28,7 @@ use crate::merge::ClosedTmpFile;
 use crate::merge::WriteableCompressedTmpFile;
 use crate::merge::WriteablePlainTmpFile;
 use crate::merge::WriteableTmpFile;
+use crate::Output;
 use crate::{
     chunks::{self, Chunk},
     compare_by, merge, sort_by, GlobalSettings,
@@ -38,7 +39,11 @@ use tempfile::TempDir;
 const START_BUFFER_SIZE: usize = 8_000;
 
 /// Sort files by using auxiliary files for storing intermediate chunks (if needed), and output the result.
-pub fn ext_sort(files: &mut impl Iterator<Item = Box<dyn Read + Send>>, settings: &GlobalSettings) {
+pub fn ext_sort(
+    files: &mut impl Iterator<Item = Box<dyn Read + Send>>,
+    settings: &GlobalSettings,
+    output: Output,
+) {
     let (sorted_sender, sorted_receiver) = std::sync::mpsc::sync_channel(1);
     let (recycled_sender, recycled_receiver) = std::sync::mpsc::sync_channel(1);
     thread::spawn({
@@ -51,6 +56,7 @@ pub fn ext_sort(files: &mut impl Iterator<Item = Box<dyn Read + Send>>, settings
             settings,
             sorted_receiver,
             recycled_sender,
+            output,
         );
     } else {
         reader_writer::<_, WriteablePlainTmpFile>(
@@ -58,6 +64,7 @@ pub fn ext_sort(files: &mut impl Iterator<Item = Box<dyn Read + Send>>, settings
             settings,
             sorted_receiver,
             recycled_sender,
+            output,
         );
     }
 }
@@ -67,6 +74,7 @@ fn reader_writer<F: Iterator<Item = Box<dyn Read + Send>>, Tmp: WriteableTmpFile
     settings: &GlobalSettings,
     receiver: Receiver<Chunk>,
     sender: SyncSender<Chunk>,
+    output: Output,
 ) {
     let separator = if settings.zero_terminated {
         b'\0'
@@ -96,7 +104,7 @@ fn reader_writer<F: Iterator<Item = Box<dyn Read + Send>>, Tmp: WriteableTmpFile
                 settings,
                 Some((tmp_dir, tmp_dir_size)),
             );
-            merger.write_all(settings);
+            merger.write_all(settings, output);
         }
         ReadResult::SortedSingleChunk(chunk) => {
             if settings.unique {
@@ -106,9 +114,10 @@ fn reader_writer<F: Iterator<Item = Box<dyn Read + Send>>, Tmp: WriteableTmpFile
                             == Ordering::Equal
                     }),
                     settings,
+                    output,
                 );
             } else {
-                print_sorted(chunk.lines().iter(), settings);
+                print_sorted(chunk.lines().iter(), settings, output);
             }
         }
         ReadResult::SortedTwoChunks([a, b]) => {
@@ -128,9 +137,10 @@ fn reader_writer<F: Iterator<Item = Box<dyn Read + Send>>, Tmp: WriteableTmpFile
                         })
                         .map(|(line, _)| line),
                     settings,
+                    output,
                 );
             } else {
-                print_sorted(merged_iter.map(|(line, _)| line), settings);
+                print_sorted(merged_iter.map(|(line, _)| line), settings, output);
             }
         }
         ReadResult::EmptyInput => {

src/uu/sort/src/merge.rs

@@ -25,7 +25,7 @@ use tempfile::TempDir;
 
 use crate::{
     chunks::{self, Chunk, RecycledChunk},
-    compare_by, GlobalSettings,
+    compare_by, GlobalSettings, Output,
 };
 
 /// Merge pre-sorted `Box<dyn Read>`s.
@@ -238,8 +238,8 @@ pub struct FileMerger<'a> {
 
 impl<'a> FileMerger<'a> {
     /// Write the merged contents to the output file.
-    pub fn write_all(&mut self, settings: &GlobalSettings) {
-        let mut out = settings.out_writer();
+    pub fn write_all(&mut self, settings: &GlobalSettings, output: Output) {
+        let mut out = output.into_write();
         self.write_all_to(settings, &mut out);
     }
 

src/uu/sort/src/sort.rs

@@ -37,7 +37,7 @@ use std::env;
 use std::ffi::OsStr;
 use std::fs::File;
 use std::hash::{Hash, Hasher};
-use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Read, Write};
+use std::io::{stdin, stdout, BufRead, BufReader, Read, Write};
 use std::ops::Range;
 use std::path::Path;
 use std::path::PathBuf;
@@ -146,6 +146,29 @@ impl SortMode {
     }
 }
 
+pub struct Output {
+    file: Option<File>,
+}
+
+impl Output {
+    fn new(name: Option<&str>) -> Self {
+        Self {
+            file: name.map(|name| {
+                File::create(name).unwrap_or_else(|e| {
+                    crash!(2, "open failed: {}: {}", name, strip_errno(&e.to_string()))
+                })
+            }),
+        }
+    }
+
+    fn into_write(self) -> Box<dyn Write> {
+        match self.file {
+            Some(file) => Box::new(file),
+            None => Box::new(stdout()),
+        }
+    }
+}
+
 #[derive(Clone)]
 pub struct GlobalSettings {
     mode: SortMode,
@@ -156,7 +179,6 @@ pub struct GlobalSettings {
     ignore_non_printing: bool,
     merge: bool,
     reverse: bool,
-    output_file: Option<String>,
     stable: bool,
     unique: bool,
     check: bool,
@@ -209,19 +231,6 @@ impl GlobalSettings {
         }
     }
 
-    fn out_writer(&self) -> BufWriter<Box<dyn Write>> {
-        match self.output_file {
-            Some(ref filename) => match File::create(Path::new(&filename)) {
-                Ok(f) => BufWriter::new(Box::new(f) as Box<dyn Write>),
-                Err(e) => {
-                    show_error!("{0}: {1}", filename, e.to_string());
-                    crash!(2, "Could not open output file");
-                }
-            },
-            None => BufWriter::new(Box::new(stdout()) as Box<dyn Write>),
-        }
-    }
-
     /// Precompute some data needed for sorting.
     /// This function **must** be called before starting to sort, and `GlobalSettings` may not be altered
     /// afterwards.
@@ -253,7 +262,6 @@ impl Default for GlobalSettings {
             ignore_non_printing: false,
             merge: false,
             reverse: false,
-            output_file: None,
             stable: false,
             unique: false,
             check: false,
@@ -1053,7 +1061,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 
     settings.ignore_leading_blanks = matches.is_present(options::IGNORE_LEADING_BLANKS);
 
-    settings.output_file = matches.value_of(options::OUTPUT).map(String::from);
     settings.reverse = matches.is_present(options::REVERSE);
     settings.stable = matches.is_present(options::STABLE);
     settings.unique = matches.is_present(options::UNIQUE);
@@ -1099,9 +1106,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         );
     }
 
+    let output = Output::new(matches.value_of(options::OUTPUT));
+
     settings.init_precomputed();
 
-    exec(&files, &settings)
+    exec(&files, &settings, output)
 }
 
 pub fn uu_app() -> App<'static, 'static> {
@@ -1334,10 +1343,10 @@ pub fn uu_app() -> App<'static, 'static> {
         .arg(Arg::with_name(options::FILES).multiple(true).takes_value(true))
 }
 
-fn exec(files: &[String], settings: &GlobalSettings) -> i32 {
+fn exec(files: &[String], settings: &GlobalSettings, output: Output) -> i32 {
     if settings.merge {
         let mut file_merger = merge::merge(files.iter().map(open), settings);
-        file_merger.write_all(settings);
+        file_merger.write_all(settings, output);
     } else if settings.check {
         if files.len() > 1 {
             crash!(2, "only one file allowed with -c");
@@ -1346,7 +1355,7 @@ fn exec(files: &[String], settings: &GlobalSettings) -> i32 {
     } else {
         let mut lines = files.iter().map(open);
 
-        ext_sort(&mut lines, settings);
+        ext_sort(&mut lines, settings, output);
     }
     0
 }
@@ -1618,8 +1627,12 @@ fn month_compare(a: &str, b: &str) -> Ordering {
     }
 }
 
-fn print_sorted<'a, T: Iterator<Item = &'a Line<'a>>>(iter: T, settings: &GlobalSettings) {
-    let mut writer = settings.out_writer();
+fn print_sorted<'a, T: Iterator<Item = &'a Line<'a>>>(
+    iter: T,
+    settings: &GlobalSettings,
+    output: Output,
+) {
+    let mut writer = output.into_write();
     for line in iter {
         line.print(&mut writer, settings);
     }

tests/by-util/test_sort.rs

@@ -959,3 +959,30 @@ fn test_key_takes_one_arg() {
         .succeeds()
         .stdout_is_fixture("keys_open_ended.expected");
 }
+
+#[test]
+fn test_verifies_out_file() {
+    let inputs = ["" /* no input */, "some input"];
+    for &input in &inputs {
+        new_ucmd!()
+            .args(&["-o", "nonexistent_dir/nonexistent_file"])
+            .pipe_in(input)
+            .fails()
+            .status_code(2)
+            .stderr_only(
+                #[cfg(not(windows))]
+                "sort: open failed: nonexistent_dir/nonexistent_file: No such file or directory",
+                #[cfg(windows)]
+                "sort: open failed: nonexistent_dir/nonexistent_file: The system cannot find the path specified.",
+            );
+    }
+}
+
+#[test]
+fn test_verifies_out_file_after_keys() {
+    new_ucmd!()
+        .args(&["-o", "nonexistent_dir/nonexistent_file", "-k", "0"])
+        .fails()
+        .status_code(2)
+        .stderr_contains("failed to parse key");
+}