fix(tsort): achieve GNU compatibility with alphabetical DFS ordering and reverse successor processing

should fix tests/misc/tsort.pl

Author: sylvestre

src/uu/tsort/src/tsort.rs

@@ -17,6 +17,7 @@ use uucore::translate;
 
 mod options {
     pub const FILE: &str = "file";
+    pub const WARN: &str = "warn";
 }
 
 #[derive(Debug, Error)]
@@ -49,11 +50,28 @@ impl UError for LoopNode<'_> {}
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let matches = uucore::clap_localization::handle_clap_result(uu_app(), args)?;
 
-    let input = matches
-        .get_one::<OsString>(options::FILE)
-        .expect("Value is required by clap");
+    // Check for extra operands
+    let files: Vec<_> = matches
+        .get_many::<OsString>(options::FILE)
+        .map(|v| v.collect())
+        .unwrap_or_default();
+
+    if files.len() > 1 {
+        return Err(uucore::error::USimpleError::new(
+            1,
+            format!(
+                "extra operand {}\nTry '{} --help' for more information.",
+                files[1].to_string_lossy().quote(),
+                uucore::util_name()
+            ),
+        ));
+    }
+
+    let input = files.first().expect("Value is required by clap");
+    // Accept -w/--warn flag for GNU compatibility (doesn't change behavior)
+    let _warn = matches.get_flag(options::WARN);
 
-    let data = if input == "-" {
+    let data = if input.to_string_lossy() == "-" {
         let stdin = std::io::stdin();
         std::io::read_to_string(stdin)?
     } else {
@@ -96,12 +114,20 @@ pub fn uu_app() -> Command {
         .override_usage(format_usage(&translate!("tsort-usage")))
         .about(translate!("tsort-about"))
         .infer_long_args(true)
+        .arg(
+            Arg::new(options::WARN)
+                .short('w')
+                .long("warn")
+                .help("warn about cycles, but continue")
+                .action(clap::ArgAction::SetTrue),
+        )
         .arg(
             Arg::new(options::FILE)
                 .default_value("-")
                 .hide(true)
                 .value_parser(clap::value_parser!(OsString))
-                .value_hint(clap::ValueHint::FilePath),
+                .value_hint(clap::ValueHint::FilePath)
+                .num_args(0..),
         )
 }
 
@@ -132,6 +158,7 @@ impl<'input> Node<'input> {
 struct Graph<'input> {
     name: String,
     nodes: HashMap<&'input str, Node<'input>>,
+    insertion_order: Vec<&'input str>,
 }
 
 #[derive(Clone, Copy, PartialEq, Eq)]
@@ -145,14 +172,26 @@ impl<'input> Graph<'input> {
         Self {
             name,
             nodes: HashMap::default(),
+            insertion_order: Vec::new(),
         }
     }
 
     fn add_edge(&mut self, from: &'input str, to: &'input str) {
-        let from_node = self.nodes.entry(from).or_default();
+        // Track insertion order for deterministic output
+        if let Entry::Vacant(e) = self.nodes.entry(from) {
+            self.insertion_order.push(from);
+            e.insert(Node::default());
+        }
+
+        let from_node = self.nodes.get_mut(from).unwrap();
         if from != to {
             from_node.add_successor(to);
-            let to_node = self.nodes.entry(to).or_default();
+
+            if let Entry::Vacant(e) = self.nodes.entry(to) {
+                self.insertion_order.push(to);
+                e.insert(Node::default());
+            }
+            let to_node = self.nodes.get_mut(to).unwrap();
             to_node.predecessor_count += 1;
         }
     }
@@ -178,23 +217,20 @@ impl<'input> Graph<'input> {
             })
             .collect();
 
-        // To make sure the resulting ordering is deterministic we
-        // need to order independent nodes.
-        //
-        // FIXME: this doesn't comply entirely with the GNU coreutils
-        // implementation.
+        // Sort independent nodes alphabetically to match GNU coreutils behavior
         independent_nodes_queue.make_contiguous().sort_unstable();
 
         while !self.nodes.is_empty() {
             // Get the next node (breaking any cycles necessary to do so).
             let v = self.find_next_node(&mut independent_nodes_queue);
             println!("{v}");
             if let Some(node_to_process) = self.nodes.remove(v) {
-                for successor_name in node_to_process.successor_names {
+                // Process successors in reverse order and add freed nodes to the back
+                for successor_name in node_to_process.successor_names.iter().rev() {
                     let successor_node = self.nodes.get_mut(successor_name).unwrap();
                     successor_node.predecessor_count -= 1;
                     if successor_node.predecessor_count == 0 {
-                        // If we find nodes without any other prerequisites, we add them to the queue.
+                        // Add to the back of the queue (FIFO)
                         independent_nodes_queue.push_back(successor_name);
                     }
                 }
@@ -245,6 +281,8 @@ impl<'input> Graph<'input> {
     }
 
     fn detect_cycle(&self) -> Vec<&'input str> {
+        // Sort the nodes alphabetically to make this function deterministic
+        // and match GNU coreutils behavior.
         let mut nodes: Vec<_> = self.nodes.keys().collect();
         nodes.sort_unstable();
 

tests/by-util/test_tsort.rs

@@ -77,7 +77,7 @@ fn test_multiple_arguments() {
         .arg("call_graph.txt")
         .arg("invalid_file")
         .fails()
-        .stderr_contains("unexpected argument 'invalid_file' found");
+        .stderr_contains("extra operand 'invalid_file'");
 }
 
 #[test]
@@ -119,7 +119,7 @@ fn test_two_cycles() {
     new_ucmd!()
         .pipe_in("a b b c c b b d d b")
         .fails_with_code(1)
-        .stdout_is("a\nb\nc\nd\n")
+        .stdout_is("a\nb\nd\nc\n")
         .stderr_is("tsort: -: input contains a loop:\ntsort: b\ntsort: c\ntsort: -: input contains a loop:\ntsort: b\ntsort: d\n");
 }
 
@@ -153,3 +153,72 @@ fn test_loop_for_iterative_dfs_correctness() {
         .fails_with_code(1)
         .stderr_contains("tsort: -: input contains a loop:\ntsort: B\ntsort: C");
 }
+
+#[test]
+fn test_warn_flag_accepted() {
+    // Test that -w/--warn flag is accepted without error
+    // This is for GNU test suite compatibility (cycle-3 test uses -w)
+    // The flag doesn't change behavior, just needs to be parseable
+    new_ucmd!()
+        .arg("-w")
+        .pipe_in("a b\nb c\nc a")
+        .fails_with_code(1)
+        .stderr_contains("input contains a loop:");
+}
+
+#[test]
+fn test_warn_flag_long() {
+    // Test that --warn is also accepted
+    new_ucmd!()
+        .arg("--warn")
+        .pipe_in("a b\nb c")
+        .succeeds()
+        .stdout_is("a\nb\nc\n");
+}
+
+#[test]
+fn test_cycle_detection_alphabetical_order() {
+    // Verify cycles are reported starting from alphabetically first node
+    // Input: t->b, t->s, s->t creates cycle [s,t]
+    // Cycle should be detected starting from 's' (alphabetically first)
+    new_ucmd!()
+        .pipe_in("t b t s s t")
+        .fails_with_code(1)
+        .stdout_is("s\nt\nb\n")
+        .stderr_contains("tsort: s\ntsort: t");
+}
+
+#[test]
+fn test_tree_branching_order() {
+    // Test the interleaving behavior when dependency chains branch
+    // Input creates:  a→b→c→d→e→f→g
+    //                      ↓
+    //                      x→y→z
+    // Should output: a, b, c, x, d, y, e, z, f, g
+    new_ucmd!()
+        .pipe_in("a b b c c d d e e f f g c x x y y z")
+        .succeeds()
+        .stdout_is("a\nb\nc\nx\nd\ny\ne\nz\nf\ng\n");
+}
+
+#[test]
+fn test_multiple_cycles_alphabetical() {
+    // Test multiple cycles are detected in alphabetical order
+    // Input: a→b→a (cycle 1), a→c→a (cycle 2)
+    // Both cycles should start from 'a' (alphabetically first in each)
+    new_ucmd!()
+        .pipe_in("a a a b a c c a b a")
+        .fails_with_code(1)
+        .stderr_contains("tsort: a\ntsort: b")
+        .stderr_contains("tsort: a\ntsort: c");
+}
+
+#[test]
+fn test_alphabetical_frontier_ordering() {
+    // Test that initial independent nodes are processed alphabetically
+    // All nodes have no dependencies, should output alphabetically
+    new_ucmd!()
+        .pipe_in("d d c c b b a a")
+        .succeeds()
+        .stdout_is("a\nb\nc\nd\n");
+}

tests/fixtures/tsort/call_graph.expected

@@ -1,17 +1,17 @@
 main
-parse_options
-tail_file
 tail_forever
-tail
+tail_file
+parse_options
 recheck
+tail
 write_header
-tail_lines
-tail_bytes
 pretty_name
-start_lines
-file_lines
-pipe_lines
-xlseek
-start_bytes
+tail_bytes
+tail_lines
 pipe_bytes
+start_bytes
+xlseek
+pipe_lines
+file_lines
+start_lines
 dump_remainder