fix!: make fs::walkdir_sorted_new() sort entries by paths literally (#1928)

This follows up 7b1b5bf. Since packed-refs
appears to be sorted by full ref name, loose-refs should also be emitted in
that order.

The comparison function is copied from gix::diff::object::tree::EntryRef.
Non-utf8 file names are simply mapped to "" on Windows. We could add some
fallback, but callers can't handle such file names anyway.

Author: yuja

Cargo.lock

@@ -48,7 +48,7 @@ parallel = ["dep:crossbeam-channel", "dep:parking_lot"]
 once_cell = ["dep:once_cell"]
 ## Makes facilities of the `walkdir` crate partially available.
 ## In conjunction with the **parallel** feature, directory walking will be parallel instead behind a compatible interface.
-walkdir = ["dep:walkdir", "dep:gix-utils"]
+walkdir = ["dep:walkdir", "dep:gix-path", "dep:gix-utils"]
 #* an in-memory unidirectional pipe using `bytes` as efficient transfer mechanism.
 io-pipe = ["dep:bytes"]
 ## provide a proven and fast `crc32` implementation.
@@ -106,6 +106,7 @@ required-features = ["io-pipe"]
 gix-trace = { version = "^0.1.12", path = "../gix-trace" }
 
 # for walkdir
+gix-path = { version = "^0.10.15", path = "../gix-path", optional = true }
 gix-utils = { version = "^0.2.0", path = "../gix-utils", optional = true }
 
 # 'parallel' feature

gix-features/Cargo.toml

@@ -217,19 +217,19 @@ pub mod walkdir {
     ///
     /// Use `precompose_unicode` to represent the `core.precomposeUnicode` configuration option.
     pub fn walkdir_sorted_new(root: &Path, _: Parallelism, precompose_unicode: bool) -> WalkDir {
-        fn ft_to_number(ft: std::fs::FileType) -> usize {
-            if ft.is_file() {
-                1
-            } else {
-                2
-            }
-        }
         WalkDir {
             inner: WalkDirImpl::new(root)
                 .sort_by(|a, b| {
-                    ft_to_number(a.file_type())
-                        .cmp(&ft_to_number(b.file_type()))
-                        .then_with(|| a.file_name().cmp(b.file_name()))
+                    // Ignore non-utf8 file name on Windows, which would probably be rejected by caller.
+                    let a_name = gix_path::os_str_into_bstr(a.file_name()).unwrap_or("".as_ref());
+                    let b_name = gix_path::os_str_into_bstr(b.file_name()).unwrap_or("".as_ref());
+                    // "common." < "common/" < "common0"
+                    let common = a_name.len().min(b_name.len());
+                    a_name[..common].cmp(&b_name[..common]).then_with(|| {
+                        let a = a_name.get(common).or_else(|| a.file_type().is_dir().then_some(&b'/'));
+                        let b = b_name.get(common).or_else(|| b.file_type().is_dir().then_some(&b'/'));
+                        a.cmp(&b)
+                    })
                 })
                 .into(),
             precompose_unicode,

gix-features/src/fs.rs

@@ -31,8 +31,8 @@ mod with_namespace {
             .map(|r: gix_ref::Reference| r.name)
             .collect::<Vec<_>>();
         let expected_namespaced_refs = vec![
-            "refs/namespaces/bar/refs/multi-link",
             "refs/namespaces/bar/refs/heads/multi-link-target1",
+            "refs/namespaces/bar/refs/multi-link",
             "refs/namespaces/bar/refs/remotes/origin/multi-link-target3",
             "refs/namespaces/bar/refs/tags/multi-link-target2",
         ];
@@ -50,8 +50,8 @@ mod with_namespace {
                 .map(|r| r.name.into_inner())
                 .collect::<Vec<_>>(),
             [
-                "refs/namespaces/bar/refs/multi-link",
                 "refs/namespaces/bar/refs/heads/multi-link-target1",
+                "refs/namespaces/bar/refs/multi-link",
                 "refs/namespaces/bar/refs/tags/multi-link-target2"
             ]
         );
@@ -149,8 +149,8 @@ mod with_namespace {
         let packed = ns_store.open_packed_buffer()?;
 
         let expected_refs = vec![
-            "refs/multi-link",
             "refs/heads/multi-link-target1",
+            "refs/multi-link",
             "refs/remotes/origin/multi-link-target3",
             "refs/tags/multi-link-target2",
         ];
@@ -198,8 +198,8 @@ mod with_namespace {
                 .map(|r| r.name.into_inner())
                 .collect::<Vec<_>>(),
             [
-                "refs/multi-link",
                 "refs/heads/multi-link-target1",
+                "refs/multi-link",
                 "refs/tags/multi-link-target2",
             ],
             "loose iterators have namespace support as well"
@@ -214,8 +214,8 @@ mod with_namespace {
                     .map(|r| r.name.into_inner())
                     .collect::<Vec<_>>(),
                 [
-                    "refs/namespaces/bar/refs/multi-link",
                     "refs/namespaces/bar/refs/heads/multi-link-target1",
+                    "refs/namespaces/bar/refs/multi-link",
                     "refs/namespaces/bar/refs/tags/multi-link-target2",
                     "refs/namespaces/foo/refs/remotes/origin/HEAD"
                 ],
@@ -291,14 +291,14 @@ fn loose_iter_with_broken_refs() -> crate::Result {
         ref_paths,
         vec![
             "d1",
-            "loop-a",
-            "loop-b",
-            "multi-link",
             "heads/A",
             "heads/d1",
             "heads/dt1",
             "heads/main",
             "heads/multi-link-target1",
+            "loop-a",
+            "loop-b",
+            "multi-link",
             "remotes/origin/HEAD",
             "remotes/origin/main",
             "remotes/origin/multi-link-target3",
@@ -483,7 +483,7 @@ fn overlay_iter_reproduce_1928() -> crate::Result {
         (
             "refs/heads/a/b",
             Object(
-                Sha1(2222222222222222222222222222222222222222),
+                Sha1(bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb),
             ),
         ),
         (
@@ -492,12 +492,6 @@ fn overlay_iter_reproduce_1928() -> crate::Result {
                 Sha1(cccccccccccccccccccccccccccccccccccccccc),
             ),
         ),
-        (
-            "refs/heads/a/b",
-            Object(
-                Sha1(bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb),
-            ),
-        ),
     ]
     "#);
     Ok(())

gix-ref/tests/refs/file/store/iter.rs

@@ -24,10 +24,10 @@ fn common_values_and_names_by_path() -> crate::Result {
             "recursive-clone/submodule/.gitmodules",
             "relative-clone/.gitmodules",
             "relative-clone/submodule/.gitmodules",
-            "super/.gitmodules",
-            "super/submodule/.gitmodules",
             "super-clone/.gitmodules",
             "super-clone/submodule/.gitmodules",
+            "super/.gitmodules",
+            "super/submodule/.gitmodules",
             "top-only-clone/.gitmodules"
         ]
         .into_iter()

gix-submodule/tests/file/baseline.rs

@@ -102,10 +102,10 @@ mod iter_references {
                 "refs/heads/d1",
                 "refs/heads/dt1",
                 "refs/heads/main",
+                "refs/heads/multi-link-target1",
                 "refs/loop-a",
                 "refs/loop-b",
                 "refs/multi-link",
-                "refs/heads/multi-link-target1",
                 "refs/remotes/origin/HEAD",
                 "refs/remotes/origin/main",
                 "refs/remotes/origin/multi-link-target3",