Skip to content

touch: read -d dates in the local zone and honor their offsets - #15292

Open
jay23606 wants to merge 9 commits into
uutils:mainfrom
jay23606:fix/15291-touch-local-time
Open

jay23606 wants to merge 9 commits into
uutils:mainfrom
jay23606:fix/15291-touch-local-time

Conversation

@jay23606

@jay23606 jay23606 commented Oct 13, 2026 •

Copy link
Copy Markdown

touch -d read a date without an offset, such as 2026-08-01 12:00:00, as UTC. It now reads it in the system zone, as the documentation describes.

This also changes dates that have an offset. The offset used to be dropped, so 2004-01-16 12:00 +0200 was set to 12:00 UTC. It now honors the offset and sets 10:00 UTC. test_touch_set_date7 has its expected value updated to match.

Closes #15291

Integration tests set TZ to a non-UTC zone and check fixed timestamps for both forms. Run locally on Windows only. Six symlink tests fail there for a privilege reason, unrelated to this change.

Copilot AI balanced review requested due to automatic review settings October 13, 2026 00:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The locale-format path remains UTC, and the regression test is ineffective on UTC hosts.

1 open finding
What changed in this PR

Updates touch -d to interpret offset-less numeric timestamps in the system time zone.

Changes:

  • Uses the local zone for numeric dates without offsets.
  • Adds a unit regression test.
File Description
src/​uu/​touch/​src/​touch.rs Updates date parsing and adds coverage.

🧠 Review effort: Balanced


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

Comment thread src/uu/touch/src/touch.rs Outdated
Copilot AI balanced review requested due to automatic review settings October 13, 2026 01:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Explicit offsets are discarded, and the regression tests are ineffective on UTC CI hosts.

2 open findings
1 resolved since last review

🧠 Review effort: Balanced

Comment thread src/uu/touch/src/touch.rs Outdated
Comment thread src/uu/touch/src/touch.rs Outdated
Copilot AI balanced review requested due to automatic review settings October 13, 2026 01:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The regression tests pass under the previous behavior on UTC-based CI runners and need an explicit non-UTC zone.

2 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 13, 2026 02:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The primary regression test is nondeterministic on UTC hosts, and the description omits the explicit-offset behavior change.

2 open findings
1 resolved since last review

🧠 Review effort: Balanced

Comment thread src/uu/touch/src/touch.rs
Copilot AI balanced review requested due to automatic review settings October 13, 2026 02:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The description incorrectly claims explicit-offset behavior is unchanged.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

@jay23606 jay23606 changed the title touch: read a date without an offset in the local zone touch: read -d dates in the local zone and honor their offsets Oct 13, 2026
Copilot AI balanced review requested due to automatic review settings October 13, 2026 22:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused implementation correctly addresses both reported parsing errors with adequate regression coverage.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 13, 2026 23:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Nonexistent local times during DST gaps can fall through and be silently normalized.

1 open finding

🧠 Review effort: Balanced

Comment thread src/uu/touch/src/touch.rs Outdated
Copilot AI balanced review requested due to automatic review settings October 14, 2026 03:58

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused implementation is consistent with the reported behavior and includes adequate regression coverage.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 14, 2026 05:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

DST-gap detection misses accepted formats, and existing fixed-epoch tests become host-timezone-dependent.

1 open finding

🧠 Review effort: Balanced

Comment thread src/uu/touch/src/touch.rs Outdated
Copilot AI balanced review requested due to automatic review settings October 14, 2026 06:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The DST-gap guard misses accepted minute-only timestamps, allowing silent normalization.

2 open findings

🧠 Review effort: Balanced

Comment thread src/uu/touch/src/touch.rs
Copilot AI balanced review requested due to automatic review settings October 14, 2026 07:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

DST-gap detection misses the supported minute-only date format.

2 open findings

🧠 Review effort: Balanced

Copilot AI balanced review requested due to automatic review settings October 14, 2026 18:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Minute-only timestamps bypass the new daylight-saving gap validation.

2 open findings
Previously missed (1)

In code that hasn't changed since last review

Medium severity Reject nonexistent DST times consistently regardless of minute format

src/​uu/​touch/​src/​touch.rs:885

Include %Y-%m-%d %H:%M in this guard and add a regression test: with TZ=EST+5EDT,M3.2.0/2,M11.1.0/2, -d '2020-03-08 02:30' bypasses the gap check and reaches the normalizing fallback, while the equivalent input ending in :00 is rejected.

🧠 Review effort: Balanced

@codspeed

codspeed Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 17.48%

⚠️ 120 benchmarks 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

ℹ️ 1 benchmark has no flame graph

The flamegraph could not be generated because too much time was spent in runtime internals, making the visualization not useful for performance analysis.

⚠️ 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

❌ 3 regressed benchmarks
✅ 398 untouched benchmarks
⏩ 70 skipped benchmarks1

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 386.1 µs -28.08%
❌ Simulation true_consecutive_calls 294.7 ns 350.2 ns -15.86%
❌ Memory du_deep_tree[(100, 3)] 76.5 KB 82.4 KB -7.14%

Tip

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


Comparing jay23606:fix/15291-touch-local-time (d3a9360) with main (5fda18e)2

Open in CodSpeed

Footnotes

  1. 70 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. ↩

  2. No successful run was found on main (b77b664) during the generation of this report, so 5fda18e was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Copilot AI balanced review requested due to automatic review settings October 15, 2026 10:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-platform timezone behavior still needs confirmation from Linux, macOS, and Windows CI.

0 open findings

2 resolved since last review

🧠 Review effort: Balanced

jay23606 and others added 8 commits October 19, 2026 03:44
touch -d read "YYYY-MM-DD HH:MM:SS" as UTC. GNU touch reads it as local
time. Use the system zone for the formats that carry no offset. Adds a
unit test that fails on the UTC behaviour.
The unit tests compared against the system zone, which is UTC on UTC CI
hosts, so they passed without the local-zone behavior. Move the coverage
to integration tests that set TZ to a non-UTC zone and check fixed epochs.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
parse_datetime normalizes a wall-clock time the zone skipped, so -d read
02:30 on a spring-forward day as a real time. Refuse it before the
fallback, as the date utility does, and cover it with a -d test.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
parse_datetime accepts 'YYYY-MM-DD HH:MM' and normalizes a time the zone
skipped, while the seconds forms were already refused. Check the minute-only
form too, with a regression test.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
@jay23606
jay23606 force-pushed the fix/15291-touch-local-time branch from 09e5fac to 81e4bfa Compare October 19, 2026 07:45
Copilot AI balanced review requested due to automatic review settings October 19, 2026 07:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 issues

Review effort: Lite

Comment thread src/uu/touch/src/touch.rs Outdated
Comment thread src/uu/touch/src/touch.rs Outdated
Comment thread src/uu/touch/src/touch.rs
Comment thread tests/by-util/test_touch.rs Outdated
A wall-clock time that happens twice, such as 01:30 on the autumn
fall-back day, was rejected by every local-time conversion because
`unambiguous()` fails for folds as well as gaps. `touch -t 202011010130`
therefore reported an invalid date format.

Add zoned_earlier, which takes the earlier instant for a fold and still
rejects a gap. Use it at each local-time conversion, and compute the
system time zone once in is_system_zone_gap.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Signed-off-by: jay23606 <4602056+jay23606@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 20, 2026 02:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

touch -d interprets a date/time without timezone as UTC instead of local time

2 participants