Skip to content

Commit 7502b4a

Browse files
ByronByron
authored
Merge pull request #1931 from yuja/push-klrqpplwxrkx
make `fs::walkdir_sorted_new()` sort entries by paths literally (#1928)
2 parents 19f50d0 + 7e6e751 commit 7502b4a

File tree

8 files changed

+88
-24
lines changed

8 files changed

+88
-24
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎gix-features/Cargo.toml‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ parallel = ["dep:crossbeam-channel", "dep:parking_lot"]
4848
once_cell = ["dep:once_cell"]
4949
## Makes facilities of the `walkdir` crate partially available.
5050
## In conjunction with the **parallel** feature, directory walking will be parallel instead behind a compatible interface.
51-
walkdir = ["dep:walkdir", "dep:gix-utils"]
51+
walkdir = ["dep:walkdir", "dep:gix-path", "dep:gix-utils"]
5252
#* an in-memory unidirectional pipe using `bytes` as efficient transfer mechanism.
5353
io-pipe = ["dep:bytes"]
5454
## provide a proven and fast `crc32` implementation.
@@ -106,6 +106,7 @@ required-features = ["io-pipe"]
106106
gix-trace = { version = "^0.1.12", path = "../gix-trace" }
107107

108108
# for walkdir
109+
gix-path = { version = "^0.10.15", path = "../gix-path", optional = true }
109110
gix-utils = { version = "^0.2.0", path = "../gix-utils", optional = true }
110111

111112
# 'parallel' feature

‎gix-features/src/fs.rs‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44
//! along with runtime costs for maintaining a global [`rayon`](https://docs.rs/rayon) thread pool.
55
//!
66
//! For information on how to use the [`WalkDir`] type, have a look at
7-
//! * [`jwalk::WalkDir`](https://docs.rs/jwalk/0.5.1/jwalk/type.WalkDir.html) if `parallel` feature is enabled
8-
//! * [walkdir::WalkDir](https://docs.rs/walkdir/2.3.1/walkdir/struct.WalkDir.html) otherwise
7+
// TODO: Move all this to `gix-fs` in a breaking change.
98

109
#[cfg(feature = "walkdir")]
1110
mod shared {
@@ -217,19 +216,32 @@ pub mod walkdir {
217216
///
218217
/// Use `precompose_unicode` to represent the `core.precomposeUnicode` configuration option.
219218
pub fn walkdir_sorted_new(root: &Path, _: Parallelism, precompose_unicode: bool) -> WalkDir {
220-
fn ft_to_number(ft: std::fs::FileType) -> usize {
221-
if ft.is_file() {
222-
1
223-
} else {
224-
2
225-
}
226-
}
227219
WalkDir {
228220
inner: WalkDirImpl::new(root)
229221
.sort_by(|a, b| {
230-
ft_to_number(a.file_type())
231-
.cmp(&ft_to_number(b.file_type()))
232-
.then_with(|| a.file_name().cmp(b.file_name()))
222+
let storage_a;
223+
let storage_b;
224+
let a_name = match gix_path::os_str_into_bstr(a.file_name()) {
225+
Ok(f) => f,
226+
Err(_) => {
227+
storage_a = a.file_name().to_string_lossy();
228+
storage_a.as_ref().into()
229+
}
230+
};
231+
let b_name = match gix_path::os_str_into_bstr(b.file_name()) {
232+
Ok(f) => f,
233+
Err(_) => {
234+
storage_b = b.file_name().to_string_lossy();
235+
storage_b.as_ref().into()
236+
}
237+
};
238+
// "common." < "common/" < "common0"
239+
let common = a_name.len().min(b_name.len());
240+
a_name[..common].cmp(&b_name[..common]).then_with(|| {
241+
let a = a_name.get(common).or_else(|| a.file_type().is_dir().then_some(&b'/'));
242+
let b = b_name.get(common).or_else(|| b.file_type().is_dir().then_some(&b'/'));
243+
a.cmp(&b)
244+
})
233245
})
234246
.into(),
235247
precompose_unicode,

‎gix-ref/tests/fixtures/generated-archives/make_repo_for_1928_repro.tar‎

52.5 KB
Binary file not shown.

‎gix-ref/tests/fixtures/make_repo_for_1928_repro.sh‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
#!/usr/bin/env bash
2+
set -eu -o pipefail
3+
4+
git init -q
5+
6+
mkdir -p .git/refs/heads/a
7+
cat <<EOF >.git/packed-refs
8+
# pack-refs with: peeled fully-peeled sorted
9+
1111111111111111111111111111111111111111 refs/heads/a-
10+
2222222222222222222222222222222222222222 refs/heads/a/b
11+
3333333333333333333333333333333333333333 refs/heads/a0
12+
EOF
13+
14+
mkdir -p .git/refs/heads/a
15+
echo aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa >.git/refs/heads/a-
16+
echo bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb >.git/refs/heads/a/b
17+
echo cccccccccccccccccccccccccccccccccccccccc >.git/refs/heads/a0

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

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ mod with_namespace {
3131
.map(|r: gix_ref::Reference| r.name)
3232
.collect::<Vec<_>>();
3333
let expected_namespaced_refs = vec![
34-
"refs/namespaces/bar/refs/multi-link",
3534
"refs/namespaces/bar/refs/heads/multi-link-target1",
35+
"refs/namespaces/bar/refs/multi-link",
3636
"refs/namespaces/bar/refs/remotes/origin/multi-link-target3",
3737
"refs/namespaces/bar/refs/tags/multi-link-target2",
3838
];
@@ -50,8 +50,8 @@ mod with_namespace {
5050
.map(|r| r.name.into_inner())
5151
.collect::<Vec<_>>(),
5252
[
53-
"refs/namespaces/bar/refs/multi-link",
5453
"refs/namespaces/bar/refs/heads/multi-link-target1",
54+
"refs/namespaces/bar/refs/multi-link",
5555
"refs/namespaces/bar/refs/tags/multi-link-target2"
5656
]
5757
);
@@ -149,8 +149,8 @@ mod with_namespace {
149149
let packed = ns_store.open_packed_buffer()?;
150150

151151
let expected_refs = vec![
152-
"refs/multi-link",
153152
"refs/heads/multi-link-target1",
153+
"refs/multi-link",
154154
"refs/remotes/origin/multi-link-target3",
155155
"refs/tags/multi-link-target2",
156156
];
@@ -198,8 +198,8 @@ mod with_namespace {
198198
.map(|r| r.name.into_inner())
199199
.collect::<Vec<_>>(),
200200
[
201-
"refs/multi-link",
202201
"refs/heads/multi-link-target1",
202+
"refs/multi-link",
203203
"refs/tags/multi-link-target2",
204204
],
205205
"loose iterators have namespace support as well"
@@ -214,8 +214,8 @@ mod with_namespace {
214214
.map(|r| r.name.into_inner())
215215
.collect::<Vec<_>>(),
216216
[
217-
"refs/namespaces/bar/refs/multi-link",
218217
"refs/namespaces/bar/refs/heads/multi-link-target1",
218+
"refs/namespaces/bar/refs/multi-link",
219219
"refs/namespaces/bar/refs/tags/multi-link-target2",
220220
"refs/namespaces/foo/refs/remotes/origin/HEAD"
221221
],
@@ -291,14 +291,14 @@ fn loose_iter_with_broken_refs() -> crate::Result {
291291
ref_paths,
292292
vec![
293293
"d1",
294-
"loop-a",
295-
"loop-b",
296-
"multi-link",
297294
"heads/A",
298295
"heads/d1",
299296
"heads/dt1",
300297
"heads/main",
301298
"heads/multi-link-target1",
299+
"loop-a",
300+
"loop-b",
301+
"multi-link",
302302
"remotes/origin/HEAD",
303303
"remotes/origin/main",
304304
"remotes/origin/multi-link-target3",
@@ -464,6 +464,39 @@ fn overlay_iter_reproduce_1850() -> crate::Result {
464464
Ok(())
465465
}
466466

467+
#[test]
468+
fn overlay_iter_reproduce_1928() -> crate::Result {
469+
let store = store_at("make_repo_for_1928_repro.sh")?;
470+
let ref_names = store
471+
.iter()?
472+
.all()?
473+
.map(|r| r.map(|r| (r.name.as_bstr().to_owned(), r.target)))
474+
.collect::<Result<Vec<_>, _>>()?;
475+
insta::assert_debug_snapshot!(ref_names, @r#"
476+
[
477+
(
478+
"refs/heads/a-",
479+
Object(
480+
Sha1(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa),
481+
),
482+
),
483+
(
484+
"refs/heads/a/b",
485+
Object(
486+
Sha1(bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb),
487+
),
488+
),
489+
(
490+
"refs/heads/a0",
491+
Object(
492+
Sha1(cccccccccccccccccccccccccccccccccccccccc),
493+
),
494+
),
495+
]
496+
"#);
497+
Ok(())
498+
}
499+
467500
#[test]
468501
fn overlay_iter_with_prefix_wont_allow_absolute_paths() -> crate::Result {
469502
let store = store_with_packed_refs()?;

‎gix-submodule/tests/file/baseline.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ fn common_values_and_names_by_path() -> crate::Result {
2424
"recursive-clone/submodule/.gitmodules",
2525
"relative-clone/.gitmodules",
2626
"relative-clone/submodule/.gitmodules",
27-
"super/.gitmodules",
28-
"super/submodule/.gitmodules",
2927
"super-clone/.gitmodules",
3028
"super-clone/submodule/.gitmodules",
29+
"super/.gitmodules",
30+
"super/submodule/.gitmodules",
3131
"top-only-clone/.gitmodules"
3232
]
3333
.into_iter()

‎gix/tests/gix/repository/reference.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,10 @@ mod iter_references {
102102
"refs/heads/d1",
103103
"refs/heads/dt1",
104104
"refs/heads/main",
105+
"refs/heads/multi-link-target1",
105106
"refs/loop-a",
106107
"refs/loop-b",
107108
"refs/multi-link",
108-
"refs/heads/multi-link-target1",
109109
"refs/remotes/origin/HEAD",
110110
"refs/remotes/origin/main",
111111
"refs/remotes/origin/multi-link-target3",

0 commit comments

Comments
 (0)