date: report errors while reading date files by ddy314 · Pull Request #14783 · uutils/coreutils

ddy314 · 2026-09-21T12:50:25Z

Propagate file/stdin read failures instead of treating them as EOF. Flush completed output, report the input and read error, and exit unsuccessfully. Invalid date lines still allow processing to continue.

Fixes #12672.

Validation: 164 date tests passed, 7 ignored; 37 date unit tests passed, including a reader failing after valid and invalid lines. Clippy and rustfmt passed.

AI-assisted with Codex.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 · 1

Open (2)

This integration test is likely to be flaky across Linux environments because /proc/self/mem… · New
input.clone() is unnecessary here since input is already owned and not used afterward in this… · New

What changed in this PR

Propagates input read failures in date (file/stdin) instead of treating them as EOF, while continuing past invalid date lines; adds user-facing error reporting and tests to validate behavior.

Changes:

Introduce DateInputError to distinguish invalid date lines from I/O read failures.
Make the date reader iterator surface read errors and stop iteration after the first read failure.
Add unit/integration tests and a new localized error string for read-failure reporting.

File	Description
tests/by-util/test_date.rs	Adds a Linux integration test asserting a read error is reported for a problematic input file.
src/uu/date/src/date.rs	Adds `DateInputError`, propagates read errors distinctly, and adds a unit test for reader failure after valid/invalid lines.
src/uu/date/locales/en-US.ftl	Adds localized message template for read errors.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-09-21T17:02:31Z

GNU testsuite comparison:

GNU test failed: tests/df/skip-rootfs. tests/df/skip-rootfs is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/id/smack. tests/id/smack is passing on 'main'. Maybe you have to rebase?
GNU test failed: tests/mkdir/smack-root. tests/mkdir/smack-root is passing on 'main'. Maybe you have to rebase?

sylvestre · 2026-09-21T17:15:33Z

  when using an option to specify date(s), any non-option
  argument must be a format string beginning with '+'
+
+date-error-read = {$path}: read error: {$error}


did you check the exact GNU wording/exit code with LANG=C?

Yes. With LANG=C, GNU coreutils 9.12 exits 1, writes no stdout, and reports gnudate: /proc/self/mem: read error: Input/output error on this glibc host. The uutils result matches after normalizing the executable name. The test keeps the OS error suffix flexible because musl spells it I/O error.

sylvestre · 2026-09-21T17:15:44Z

                });
            }
+            Err(DateInputError::Read(error)) => {
+                stdout.flush().map_err(DateError::Write)?;


just above we do let _ = stdout.flush(), could we be consistent?

Made this consistent with the invalid-date arm by using let _ = stdout.flush() in f4a7f1d.

sylvestre · 2026-09-21T17:15:56Z

            }
+            Err(DateInputError::Read(error)) => {
+                stdout.flush().map_err(DateError::Write)?;
+                let path = match &settings.date_source {


could you please carry the path in DateInputError::Read? re-deriving it from date_source with a hardcoded "-" fallback is fragile

DateInputError::Read now carries the input path, including the explicit stdin name, in f4a7f1d.

sylvestre · 2026-09-21T17:16:04Z

+                    DateSource::File(path) => path.as_os_str().maybe_quote().to_string(),
+                    _ => "-".to_string(),
+                };
+                return Err(uucore::error::USimpleError::new(


please add a Read variant to DateError instead of USimpleError here

Added DateError::Read and removed the USimpleError construction in f4a7f1d.

sylvestre · 2026-09-21T17:16:12Z

-    Box::new(lines.map_while(Result::ok).map(move |mut bytes| {
+) -> Box<dyn Iterator<Item = Result<ParsedDateTime, DateInputError>> + '_> {
+    let mut lines = BufReader::new(reader).split(b'\n');
+    let mut failed = false;


the manual failed flag + from_fn is a bit heavy, can't we just fuse after the error?

Replaced the manual flag and from_fn with a mapped iterator plus scan that yields the first read error and then stops in f4a7f1d.

Copilot

Copilot review overview

🟡 Changes recommended

parse_dates_from_reader currently uses matches!(result, ...) in a way that moves result, which makes the iterator scan closure not compile as written.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

Avoid moving result in matches! before reuse · New
Remove unnecessary clone from strip_errno result · New

Resolved since last review (2)

This integration test is likely to be flaky across Linux environments because /proc/self/mem…
input.clone() is unnecessary here since input is already owned and not used afterward in this…

ddy314 · 2026-09-22T06:59:23Z

+        if *failed {
+            None
+        } else {
+            *failed = matches!(result, Err(DateInputError::Read { .. }));
+            Some(result)
        }


Fixed in cdf4e79 by matching on a shared reference to result, so the value remains available for Some(result). cargo test -p uu_date (37 tests) and clippy both pass.

ddy314 · 2026-09-22T06:59:04Z

+                return Err(Box::new(DateError::Read {
+                    path,
+                    error: strip_errno(&error).clone(),
+                }));


Fixed in cdf4e79 by moving the owned String returned by strip_errno directly into DateError::Read.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 · 2 · 1

Open (4)

Avoid moving result in matches! before reuse
A new Fluent key is introduced here; if the project expects locale key parity across all… · New
Using /proc/self/mem as a deterministic read-error source may be flaky across environments… · New
Remove unnecessary clone from strip_errno result

  when using an option to specify date(s), any non-option
  argument must be a format string beginning with '+'
+
+date-error-read = {$path}: read error: {$error}


+    new_ucmd!()
+        .env("LC_ALL", "C")
+        .args(&["-f", "/proc/self/mem"])
+        .fails_with_code(1)
+        .no_stdout()
+        .stderr_contains("/proc/self/mem: read error:");


date: report errors while reading date files

e0aca98

Copilot AI lite review requested due to automatic review settings September 21, 2026 12:50

Copilot AI reviewed Sep 21, 2026

View reviewed changes

Comment thread tests/by-util/test_date.rs Outdated

Comment thread src/uu/date/src/date.rs Outdated

Copilot started reviewing on behalf of ddy314 September 21, 2026 13:35 View session

sylvestre reviewed Sep 21, 2026

View reviewed changes

date: refine read error propagation

f4a7f1d

Copilot AI review requested due to automatic review settings September 22, 2026 06:19

Copilot started reviewing on behalf of ddy314 September 22, 2026 06:19 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

date: address read error review feedback

cdf4e79

Copilot AI review requested due to automatic review settings September 22, 2026 06:58

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Copilot started reviewing on behalf of ddy314 September 22, 2026 07:32 View session

Uh oh!

Conversation

ddy314 commented Sep 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Sep 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ddy314 Sep 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ddy314 Sep 22, 2026 •

edited

Loading