fix(stat): Collection of fixes for stat(1) #9078
Conversation
CodSpeed Performance ReportMerging #9078 will not alter performanceComparing Summary
Footnotes
|
|
I mean, that's not my fault lol. |
|
GNU testsuite comparison: |
|
I think it'd be better to add tests for the device number and the percent escape issues as well. For the percent escape issue, I propose the following addition to #[test]
fn test_double_percent() {
let ts = TestScenario::new(util_name!());
let result = ts.ucmd().args(&["-c", "%%m", "/"]).succeeds();
let output = result.stdout_str().trim();
assert!(!output.is_empty(), "Escaped percent should be present");
assert_eq!(output, "%m");
}For the device number format issue, I'd add |
|
Yeah I agree, I'll add a few tests :). Ngl, I was trying to cut that corner cuz tomorrow I got an exam, but it should be quick enough. |
|
Testing the value of these flags ( |
|
GNU testsuite comparison: |
|
Those CI fails are still not my fault :) Also, I'll probably be opening a few PRs with perf improvements, passes of clippy nightly, and I'll probably refactor stat's format parser so it's less spaghetti-y; iirc stat is not the only coreutil that does use percent escaping (ls, date, du and printf come to my mind) so I feel strongly like adding a general impl to |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
a bunch of jobs are still failing |
|
GNU testsuite comparison: |
Correct me if I'm wrong, but it doesn't seem like any of the CI failures are related to the PR. Previously it was a bunch of cancelled jobs and a couple of network failures on rustup, and now it seems to be just a few unrelated failures? It's weird, because this does not touch uucore or anything that's shared across utils. |
|
@sylvestre You menace, CI is gonna be busy for hours LOL... (4h and still counting...) /jk |
Yeah, I rebased almost all pr |
|
See https://app.codecov.io/gh/uutils/coreutils/pull/9078?dropdown=coverage&src=pr&el=h1 for the tests improvements |
|
please update mknod.rs also |
There is also a makedev libc function, so it seems we can just use that. |
|
I'll get to the FreeBSD shenanigans in a minute, after I'm done writing the extra tests. |
Alonely0
force-pushed
the
stat_fix
branch
2 times, most recently
from
c8064eb to
fca942a
Compare
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
It appears that macOS CI has been down for a while lol. There are multiple PRs (including this one) which have stalled jobs for anywhere in between 4 and 15 hours because of this. Is CI self-hosted (and fixable) or are we hostage to GitHub? GitHub Status reports it's all fine... For what it's worth, last time I pushed, macOS ran successfully, and this last push was about Redox support (with conditional compilation for it), so this should be fine to merge. |
Fixes issue uutils#9072, where some AppArmor profiles designed for GNU coreutils would break under uutils because we would fetch this info unnecessarily.
Fixes issue uutils#9071, where the default options for the `Device` field where incorrect, as well as several format flags that were not working as intended: %R, %Hr, %Lr, %Hd, %Ld, %t.
Previosuly, stat would ignore the next char after escaping it; e.g., "%%m" would become "%" instead of literally "%m".
Now uucore::fs reexports libc's major(), minor() and makedev() directives.
|
It appears we ran into a filesystem race condition with atime and stime on the last runner of all the 11-hour-long CI lol (cries inside)... To avoid this in the future, I've just removed time checks and added a comment; it was naive of us to expect consistent filesystem timing anyway. |
|
GNU testsuite comparison: |
|
Should we add those two failing GNU tests ( |
3 participants
This PR fixes #9071, #9072, as well as a few bugs found along the way.