Conversation
… N)"
bc formatted its I/O errors with Rust's Display, which appends the errno
in parentheses: a write to a full disk read "cannot write output: No space
left on device (os error 28)", and a failed flush the same. Every other
utility reports plib::diag::io_error_text, the system's own message.
The interpreter's From<io::Error>, every flush failure (new report_io),
the thread-spawn failure and rustyline's errors (diag::error_text, which
also replaces a {:?} dump) now go through it.
Test: test_bc_write_error_is_reported asserts no "(os error" in stderr;
it failed before the fix.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Windows has no localtime_r or usable strftime, so plib::locale::strftime
was Unix-only and cal could not build there.
New Windows-only plib::timefmt: the C runtime's localtime_s/gmtime_s give
the broken-down time, honouring TZ in the runtime's forms (UTC0, EST5EDT)
or the system zone; the offset is the local fields' distance from the
instant, and the zone name the runtime's own strftime("%Z") (mingw's
msvcrt import library has no _get_tzname). chrono, already a workspace
dependency and now a Windows-only one of plib, formats the rest in the
POSIX locale's names, LC_TIME having no Windows meaning. The format is
rewritten first: %Z becomes the zone name, an E or O modifier is dropped,
and an unsupported conversion is copied literally as glibc does, so
chrono never meets an item it cannot format. UTC names its zone GMT, as
glibc's gmtime_r does.
locale::strftime on Windows wraps timefmt::format_time; the Unix one is
unchanged.
Tests (timefmt, run under Wine): every POSIX conversion at a fixed
instant, an ISO week year that differs from the calendar year, modifiers
and unknown conversions, a zone name containing '%'.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each platform difference is one cfg'd pair of helpers; the Unix code paths are the existing ones, moved, not changed. - date: format_time is the C library's strftime with its sentinel buffer loop on Unix, plib::timefmt on Windows; set_clock is clock_settime on Unix, SetSystemTime (kernel32; needs the system-time privilege) on Windows. show_time keeps the shared reporting. - time: CpuStart snapshots CPU times before the utility runs and reports the user/system seconds used after it is waited for: times(2) and its children's fields on Unix as before; on Windows, which keeps no children's totals, GetProcessTimes of this process plus that of the waited-for child's handle. exit_code is code-or-128+signal on Unix and the exit code on Windows. - sleep: ignoring SIGALRM is Unix-only; Windows has no such signal. - cal: unchanged, now that plib::locale::strftime builds on Windows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
expr read its operands as raw bytes through Unix's OsStrExt::as_bytes. OsStr::as_encoded_bytes is the portable spelling: on Unix it is the same bytes, so nothing changes there; on Windows a UTF-16 argument arrives as UTF-8. bc already built. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- time: the tests timed `ls -l` and `sh -c ...`, which Windows lacks. They now time this crate's own binaries: `date -u` (a hyphenated utility argument, as the option-parsing regression needs), `sleep abc` for a propagated exit status (2), and, for the CPU-bound child, this test binary re-run with `--ignored --exact time::busy_child`, an ignored test that spins for 300 ms. The non-executable file for the 126 case is written with std::fs::write instead of a Unix mode. - date: three tests are Unix-only, each saying why: a zoneinfo TZ name (the Windows C runtime reads no tz database), the 64 KiB strftime buffer limit (the C library's; and a Windows command line cannot hold such a format), and the unprivileged set form (Windows CI runs as an administrator and would really set the clock). Under Wine 9 three tests fail that pass on Windows: its msvcrt parses TZ=UTC as zone "UT" (test_tz_utc, test_default_format_utc), and its GetProcessTimes answers the caller's own times for another process (cpu_bound_child_reports_nonzero_cpu_time). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- bc: plib::tmp is Unix-only; the three tests that need a directory use a small TestDir (a pid-named directory under the temp dir, removed on drop). The missing-file and full-device diagnostics are compared with the system's own text, from calc-tests.rs's open_error and full_device_error, instead of glibc's spelled out. - expr: the non-UTF-8 operand cases are Unix-only (a Windows command line is UTF-16 and cannot carry them); the capture inside a multibyte character uses valid UTF-8 operands and becomes its own test that runs everywhere. The full-device diagnostic is compared as bc's is. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both crates join WINDOWS_CRATES, so the windows job tests them with MSVC, the lint job runs their Windows clippy, and the msrv job checks them for Windows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e and calc README listed each ported crate's utilities and every Windows difference, a section that grew with each port. It now says only that some crates build on Windows and points to WINDOWS.md, and is not edited per crate. WINDOWS.md opens with "Using the utilities on Windows": a crate/utility table and build command, and the behaviour list moved from README, adding datetime and calc: POSIX-locale month and day names whatever LC_TIME says, TZ in the C runtime's forms only, date setting the clock with the system-time privilege, time counting the utility's CPU and its own, and expr operands as text. The porting guide follows; its table gains localtime_r/gmtime_r/TZ, strftime/LC_TIME, clock_settime, times() and argv-as-bytes rows, step 6 lists a crate in WINDOWS.md rather than README, and "Wine's limits" records the three datetime tests Wine 9 fails and why, and that Wine maps /dev/full to the Linux device. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It adds substantial new cross-platform unsafe Windows FFI (strftime/tzset, GetProcessTimes, SetSystemTime) and a shared plib formatting module whose correctness cannot be verified by execution here, warranting human sign-off.
Review effort: Balanced
Findings: None
What changed in this PR
This PR extends the project's native Windows (MSVC) support from the xform/text crates to also cover datetime (cal, date, sleep, time) and calc (bc, expr). It adds a new plib::timefmt module that reimplements strftime(3) on Windows (via the C runtime for zone/TZ resolution plus chrono for POSIX-locale field formatting), wires platform-specific implementations for CPU timing and clock-setting, routes user-facing errors through the locale-aware diag::io_error_text/error_text helpers, and updates docs/CI to list the newly portable crates.
Changes:
- New
plib/src/timefmt.rs(+chronoas a Windows-onlyplibdep) providing Windowsstrftime, used byplib::locale::strftime's new#[cfg(windows)]branch. datetimedate/time/sleepsplit into#[cfg(unix)]/#[cfg(windows)]helpers (format_time,set_clock,CpuStart,exit_code); tests made portable (self-exec CPU-bound child,date/sleepinstead ofsh, cfg-gated Unix-only cases).calcbc/exprswitch error rendering todiag::io_error_text/error_text, useOsStr::as_encoded_bytesand a localTestDir; CIWINDOWS_CRATESandREADME/WINDOWS.mdupdated.
| File | Description |
|---|---|
| plib/src/timefmt.rs | New Windows strftime (CRT zone + chrono formatting) with unit tests. |
| plib/src/locale.rs | Adds #[cfg(windows)] strftime delegating to timefmt. |
| plib/src/lib.rs / plib/Cargo.toml / Cargo.lock | Register timefmt module and Windows-only chrono dependency. |
| datetime/date.rs | Refactors formatting into format_time and clock-setting into set_clock, per-platform. |
| datetime/time.rs | Replaces times() logic with per-platform CpuStart/exit_code. |
| datetime/sleep.rs | Guards SIGALRM ignore under #[cfg(unix)]. |
| datetime/tests/{date,time}/mod.rs | Portable timing/exit tests; Unix-only cases gated. |
| calc/bc.rs, expr.rs, bc_util/interpreter.rs | Locale-aware error text; portable argv byte handling. |
| calc/tests/* | Local TestDir, system-derived error strings, split non-UTF-8 test. |
| .github/workflows/TestingCI.yml | Adds datetime/calc to WINDOWS_CRATES. |
| README.md, WINDOWS.md | Reorganize Windows docs; list newly portable crates. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.