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:

Skipping an intermittent issue tests/date/resolution (passes in this run but fails in the 'main' branch)

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…

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

ddy314 · 2026-09-23T05:35:38Z

  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}


Added the read-error key to fr-FR.ftl in ee0e92c. Both date locale bundles now define it.

ddy314 · 2026-09-23T05:35:38Z

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


Removed the procfs-dependent integration case in ee0e92c. The existing unit test uses a reader that fails after complete lines, checks the source path and error, and verifies iteration stops after that error. The date unit and integration suites pass.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

codspeed · 2026-09-23T06:06:20Z

Merging this PR will degrade performance by 28.01%

⚠️

1 benchmark spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

single_date_now

⚠️

Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 1 regressed benchmark
✅ 11 untouched benchmarks
⏩ 460 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	`BASE`	`HEAD`	Efficiency
❌	Simulation	`complex_relative_date`	277.7 µs	385.7 µs	-28.01%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

_{Comparing ddy314:fix/date-file-read-errors (24543cf) with main (ae34722)}

460 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. ↩

ddy314 · 2026-09-26T11:41:05Z

I checked the benchmark failures. The uu_nl job failed while installing the CodSpeed runner: the download returned a GitHub HTTP 500 page, before any benchmark ran. Could that job be rerun? The performance alert concerns factor, which this date-only PR does not change, and the report flags different runtime environments. A comparison on matching code baselines and runner environments would help verify it.

ddy314 · 2026-10-02T11:13:30Z

Updated the branch to resolve the conflict with main. The reader now retains the upstream NUL-byte truncation while reporting I/O errors. The 37 date unit tests and 164 integration tests pass (7 ignored).

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
Using /proc/self/mem as a deterministic read-error source may be flaky across environments…
A new Fluent key is introduced here; if the project expects locale key parity across all…
Remove unnecessary clone from strip_errno result

sylvestre · 2026-10-03T10:21:20Z

                File::open(path).map_err_context(|| path.as_os_str().maybe_quote().to_string())?;
            parse_dates_from_reader(
                file,
+                path.as_os_str().maybe_quote().to_string(),


could you compute the quoted path once and reuse it for the open error too?

Computed the quoted path once and reused it for both the open-error context and the reader in 23f3ae7. Updated against current main in 194d28d; all 43 date unit tests and 189 integration tests pass.

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

4 open findings

🧠 Review effort: Lite

ddy314 · 2026-10-20T09:35:08Z

Updated in 24543cf. Merged main and resolved the conflict caused by the escape_for_error rename, preserving the upstream escaping behavior and this PR’s read-error reporting.

Local validation: 43 date unit tests and 192 integration tests passed; formatting passed.

Copilot

🟢 Approval recommended

No unresolved review issues were identified.

2 open issues

Using /proc/self/mem as a deterministic read-error source may be flaky across environments…
A new Fluent key is introduced here; if the project expects locale key parity across all…

2 resolved since last review

_{Review effort: Lite}

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

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

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

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

date: localize read errors and use deterministic reader test

ee0e92c

Copilot AI review requested due to automatic review settings September 23, 2026 05:35

Copilot AI reviewed Sep 23, 2026

Merge main and preserve date read-error handling with NUL truncation

4d27a1f

Copilot AI lite review requested due to automatic review settings October 2, 2026 11:13

Copilot AI reviewed Oct 2, 2026

View reviewed changes

Copilot started reviewing on behalf of ddy314 October 2, 2026 12:09

View session

sylvestre reviewed Oct 3, 2026

View reviewed changes

date: reuse quoted input path for open and read errors

23f3ae7

Merge main to refresh date read-error handling

194d28d

Copilot AI lite review requested due to automatic review settings October 16, 2026 13:50

Copilot AI reviewed Oct 16, 2026

View reviewed changes

Copilot stopped reviewing on behalf of ddy314 due to an error October 16, 2026 14:13
The job was not acquired by a runner even after multiple attempts.

Merge main and preserve date read-error handling

24543cf

Copilot AI lite review requested due to automatic review settings October 20, 2026 09:27

Copilot started reviewing on behalf of ddy314 October 20, 2026 09:35

View session

Copilot AI reviewed Oct 20, 2026

View reviewed changes

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 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

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

Uh oh!

codspeed Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will degrade performance by 28.01%

Performance Changes

Footnotes

Uh oh!

ddy314 commented Sep 26, 2026

Uh oh!

ddy314 commented Oct 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

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

Uh oh!

ddy314 commented Oct 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

This branch has not been deployed

Reviewers

github-actions Bot commented Sep 21, 2026 •

edited

Loading

codspeed Bot commented Sep 23, 2026 •

edited

Loading