Skip to content

Conversation

@naoNao89
Copy link
Contributor

Fixes #9654

where we used hardcoded format strings. Now we grab the actual locale format from nl_langinfo(D_T_FMT).

Verified

# Test case from comment #3676971020
$ LC_ALL=en_US.UTF-8 date -u -d 2025-12-14T1:00

Main branch (PR #9654):

$ git checkout main && cargo build --release
$ LC_ALL=en_US.UTF-8 target/release/coreutils date -u -d 2025-12-14T1:00
Sun Dec 14 1:00:00 AM +07 2025  # Missing leading zero

GNU coreutils:

$ LC_ALL=en_US.UTF-8 gdate -u -d 2025-12-14T1:00
Sun Dec 14 01:00:00 UTC 2025  # Has leading zero

Our fix (test/date-locale-tests):

$ git checkout test/date-locale-tests && cargo build --release
$ LC_ALL=en_US.UTF-8 target/release/coreutils date -u -d 2025-12-14T1:00
Sun Dec 14 01:00:00 +07 2025  # Has leading zero

Test 2: nl_NL.UTF-8

# Locale format
$ LC_ALL=nl_NL.UTF-8 locale -k LC_TIME | grep "^d_t_fmt"
d_t_fmt="%a %e %b %X %Y"

# GNU
$ LC_ALL=nl_NL.UTF-8 gdate -u -d 2025-12-14T1:00
zo dec. 14 01:00:00 UTC 2025  # Dutch text, different order

# Our fix
$ LC_ALL=nl_NL.UTF-8 target/release/coreutils date -u -d 2025-12-14T1:00
Sun 14 Dec 01:00:00 +07 2025  # English text, different from GNU (This may be a separate localization issue beyond format strings)

Addresses #9654 comment

@naoNao89 naoNao89 changed the title date(locale): use actual locale format strings, add comprehensive tests date(locale): fix hardcoded formats with nl_langinfo(D_T_FMT) Dec 20, 2025
@naoNao89 naoNao89 force-pushed the test/date-locale-tests branch from bf9778c to 4c0695d Compare December 20, 2025 21:24
@naoNao89 naoNao89 force-pushed the test/date-locale-tests branch 2 times, most recently from d8a7568 to 5155ed1 Compare December 20, 2025 21:44

#[test]
#[cfg(unix)]
fn test_date_locale_format_structure() {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we have another tests with french as locale? thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added test_date_locale_fr_french

Copy link
Contributor

Choose a reason for hiding this comment

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

nice
but i was also thinking about the day & month in french

@naoNao89 naoNao89 force-pushed the test/date-locale-tests branch from 5155ed1 to 119ede1 Compare December 20, 2025 21:49
Replace hardcoded format string selection with direct nl_langinfo(D_T_FMT)
usage. This ensures locale-specific formatting details (leading zeros,
component ordering, hour formats) are properly respected from system
locale data instead of detecting 12/24-hour preference and returning
hardcoded alternatives.

Changes to locale.rs:
- Remove detect_12_hour_format() and uses_12_hour_format()
- Simplify get_locale_default_format() to use D_T_FMT directly
- Add timezone injection if %Z missing from locale format
- Add use nix::libc import

Add test coverage (tests/by-util/test_date.rs):
- 4 new unit tests for locale format structure validation
- 7 new integration tests verifying locale-specific behavior
- Tests prevent regression to hardcoded format strings

Addresses feedback from PR uutils#9654 comment #3676971020
@naoNao89 naoNao89 force-pushed the test/date-locale-tests branch from 119ede1 to a85255c Compare December 20, 2025 21:54
@collinfunk
Copy link

The output will still be slightly different. We want date_fmt not d_t_fmt:

$ date +"$(locale d_t_fmt)"
Sat 20 Dec 2025 01:55:21 PM PST
$ date +"$(locale date_fmt)"
Sat Dec 20 01:55:26 PM PST 2025

@naoNao89
Copy link
Contributor Author

hmm macOS does support nl_langinfo() with D_T_FMT, its locale database is much more limited than glibc ..

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/basenc/bounded-memory is now passing!

@naoNao89
Copy link
Contributor Author

naoNao89 commented Dec 20, 2025

_DATE_FMT access requires calling the C library function nl_langinfo(), which means unsafe FFI is unavoidable if you want to use it

https://man7.org/linux/man-pages/man3/nl_langinfo.3.html

https://github.com/lattera/glibc/blob/master/locale/langinfo.h#L232

@sylvestre sylvestre merged commit 585f46a into uutils:main Dec 23, 2025
127 checks passed
@collinfunk
Copy link

Even though the test is passing, this is still incorrect:

$ LC_ALL=en_US.UTF-8 date
Tue Dec 23 02:50:22 PM PST 2025
$ LC_ALL=en_US.UTF-8 ./target/debug/date
Tue 23 Dec 2025 2:50:38 PM PST

@sylvestre
Copy link
Contributor

bizarre, on my system:

date (GNU coreutils) 9.7
Packaged by Debian (9.7-3)

LC_ALL=en_US.UTF-8 /usr/bin/date
Tue Dec 23 23:58:38 CET 2025

rust:

 LC_ALL=en_US.UTF-8 ./target/debug/coreutils date
Tue Dec 23 23:58:53 CET 2025

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.

3 participants