From 673cb48a387c81615a62b645bc7a5999067bf375 Mon Sep 17 00:00:00 2001 From: Sean 'Shaleh' Perry Date: Wed, 17 Jun 2026 12:43:08 -0700 Subject: [PATCH] path: fix intransitive ordering of a directory against its descendants The path comparator returned Equal when one path was a prefix of another at a separator boundary, so a directory compared equal to every file beneath it. That makes Ord intransitive: a directory equals two different descendants that do not equal each other. sort_unstable detects the broken order and panics, which showed up when many parent and child paths landed in one change set, for example copying a large tree into a watched repo. Order the ancestor directory before its descendants instead of equal. --- src/path.rs | 45 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/src/path.rs b/src/path.rs index 28c1043..c5f0ed4 100644 --- a/src/path.rs +++ b/src/path.rs @@ -146,10 +146,19 @@ fn cmp(lhs: &[u8], rhs: &[u8]) -> Ordering { // we use some unsafe let lhs_ = unsafe { slice::from_raw_parts(lhs.as_ptr(), prefix_len) }; let rhs_ = unsafe { slice::from_raw_parts(rhs.as_ptr(), prefix_len) }; + // When one path is a prefix of the other, the next byte in the longer path + // decides the order. If that byte is the separator the shorter path is an + // ancestor directory, so it sorts first. Returning Equal there would make + // a directory compare equal to every descendant, which is intransitive and + // panics sort_unstable. lhs_.cmp(rhs_).then_with(|| match diff.cmp(&0) { - Ordering::Less => PATH_SEPARATOR.cmp(unsafe { rhs.get_unchecked(prefix_len) }), + Ordering::Less => PATH_SEPARATOR + .cmp(unsafe { rhs.get_unchecked(prefix_len) }) + .then(Ordering::Less), Ordering::Equal => Ordering::Equal, - Ordering::Greater => unsafe { lhs.get_unchecked(prefix_len) }.cmp(&PATH_SEPARATOR), + Ordering::Greater => unsafe { lhs.get_unchecked(prefix_len) } + .cmp(&PATH_SEPARATOR) + .then(Ordering::Greater), }) } @@ -325,6 +334,38 @@ impl> PartialEq for CanonicalPathBuf { mod tests { use super::*; + // A directory and its descendants must order consistently. The comparator + // used to return Equal for a path versus its own child, which makes Ord + // intransitive and panics sort_unstable on large change sets. + #[test] + fn ord_is_total_across_dir_and_children() { + let p = |s| CanonicalPathBuf::assert_canonicalized(Path::new(s)); + let dir = p("/foo"); + let child_a = p("/foo/bar"); + let child_b = p("/foo/baz"); + + // A parent sorts strictly before its descendants, never Equal. + assert_eq!(dir.cmp(&child_a), Ordering::Less); + assert_eq!(dir.cmp(&child_b), Ordering::Less); + // Siblings keep their byte order. + assert_eq!(child_a.cmp(&child_b), Ordering::Less); + // Transitivity holds: dir < child_a < child_b. + assert!(dir < child_a && child_a < child_b && dir < child_b); + + // Sorting a mixed tree must not collapse distinct paths or panic. + let mut paths = [ + p("/foo/baz"), + p("/foo"), + p("/foo/bar"), + p("/foo.txt"), + p("/foo/bar/deep"), + p("/bar"), + p("/foo/baz/x"), + ]; + paths.sort_unstable(); + assert!(paths.windows(2).all(|w| w[0] < w[1])); + } + #[test] fn is_parent_of_basic() { let foo = CanonicalPathBuf::assert_canonicalized(Path::new("/foo"));