Repository navigation
Conversation
9768ae6 to
0cbea87
Compare
|
GNU testsuite comparison: |
528406c to
1034335
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
1034335 to
12d2419
Compare
xtqqczze
changed the title
1b13f3b to
d5c0d2b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
4abd9a1 to
e2b26c9
Compare
| } | ||
|
|
||
| #[cfg(all(unix, not(any(target_os = "aix", target_os = "redox"))))] | ||
| #[cfg(any( |
There was a problem hiding this comment.
the same 7-target cfg list now appears 5 times. could we use a cfg alias or group these fns in a module?
There was a problem hiding this comment.
Yes, ideally. I haven't yet worked out the best approach for this (see #11016), so I was going to defer that for now.
| ), | ||
| expect(clippy::unnecessary_wraps) | ||
| )] | ||
| #[allow(clippy::unnecessary_wraps)] |
There was a problem hiding this comment.
why switch from expect to allow here? we don't want to silence clippy with #[allow(...)]
There was a problem hiding this comment.
I switched to avoid the verbose cfg_attr, since the rest of the changes aren't using expect.
| ))] | ||
| { | ||
| // No method to read mounts on these platforms | ||
| Ok(Vec::new()) |
There was a problem hiding this comment.
It's perhaps worth questioning for follow-up on whether returning Ok is appropriate in this case of no available method.
e2b26c9 to
d7d8f56
Compare
Merging this PR will regress 1 benchmark
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | five_38_bit_primes |
1.7 s | 1.8 s | -6.38% |
| ⚡ | Simulation | three_39_bit_primes |
434.1 ms | 325.6 ms | +33.32% |
| ⚡ | Simulation | thirteen_39_bit_primes |
9.6 s | 8.9 s | +7.08% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing xtqqczze:uucore-fsext (de51447) with main (38336e9)
Footnotes
-
135 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
| }; | ||
| #[cfg(target_os = "haiku")] | ||
| return Self { | ||
| blocksize: statvfs.block_size().try_into().unwrap(), |
There was a problem hiding this comment.
the FsMeta methods already do these conversions for haiku, could you use statvfs.total_blocks(), avail_blocks() etc. here instead of repeating try_into().unwrap()?
There was a problem hiding this comment.
f_blocks and f_bsize are different fields, though. FsMeta doesn't expose f_bsize as a u64, so using the FsMeta helpers here wouldn't cover all of these conversions. I'd prefer to keep the explicit conversions here rather than add more refactoring on top.
The feature now compiles successfully on Haiku, Hermit and Hurd targets: ```sh RUSTC_BOOTSTRAP=1 cargo clippy -q -Zbuild-std=std,panic_abort -p uucore --features=fsext --all-targets --target=x86_64-linux-android --target=i686-linux-android --target=x86_64-pc-cygwin --target=aarch64-apple-darwin --target=x86_64-unknown-freebsd --target=x86_64-unknown-haiku --target=x86_64-unknown-hurd-gnu --target=aarch64-unknown-illumos --target=x86_64-unknown-linux-gnu --target=i686-unknown-netbsd --target=x86_64-unknown-openbsd --target=x86_64-unknown-redox --target=x86_64-pc-solaris --target=wasm32-wasip1 --target=x86_64-pc-windows-gnu ```
1bec954 to
de51447
Compare
This branch has not been deployed
2 participants
The feature now compiles successfully on Haiku, Hermit and Hurd targets:
Closes #14933