coreutils
coreutils copied to clipboard
date: Catch format string that is not supported by chrono
Attempting to use %#z with chrono gets a panic: https://github.com/chronotope/chrono/blob/378efb157d674c01761f538d4450705c2b1766a4/src/format/mod.rs#L683
This catches the format string and returns an error instead.
Interesting! I have two questions.
-
Why isn't this caught by the strftimeitems change? Does that function still panic in this scenario and should we report that upstream?
-
What is the expected behaviour for
%#z? Can we do something better than exiting with an error?
1. Why isn't this caught by the strftimeitems change? Does that function still panic in this scenario and should we report that upstream?
This isn't caught by the change I made in #4240 because that catches issues where a string isn't successfully parsed, here it is successfully parsed but one of the format items is explicitly forbidden.
The root cause is that chrono added %#z as a permissive time zone for parsing so there was a single specifier that would handle the different ways a time zone offset could be specified (with or without minutes) , but specifically forbade it's use when printing (see https://github.com/chronotope/chrono/pull/242)
2. What is the expected behaviour for `%#z`? Can we do something better than exiting with an error?
The expected behaviour according to GNU date is for %#z to be equivalent to %z. For GNU date # is a flag that means 'flip case if possible', e.g.
# date +%Z
GMT
# date +%#Z
gmt
However, currently chrono doesn't support the # flag so it should be considered an invalid formatting item, and using # with any other character but z will indeed result in an invalid format error.
So I think I should make the error message the same as the other invalid format string message.
Fails with:
--- TRY 3 STDERR: coreutils::tests test_date::test_unsupported_format ---
thread 'test_date::test_unsupported_format' panicked at 'assertion failed: result.stderr_str().starts_with(\"date: do not use \'%#z\'\")', tests/by-util/test_date.rs:341:5
stack backtrace:
0: rust_begin_unwind
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:584:5
1: core::panicking::panic_fmt
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/panicking.rs:142:14
2: core::panicking::panic
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/panicking.rs:48:5
3: tests::test_date::test_unsupported_format
at ./tests/by-util/test_date.rs:341:5
4: tests::test_date::test_unsupported_format::{{closure}}
at ./tests/by-util/test_date.rs:338:1
5: core::ops::function::FnOnce::call_once
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/ops/function.rs:248:5
6: core::ops::function::FnOnce::call_once
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/ops/function.rs:248:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
error: test run failed
GNU testsuite comparison:
GNU test failed: tests/tail-2/inotify-dir-recreate. tests/tail-2/inotify-dir-recreate is passing on 'main'. Maybe you have to rebase?
Can this be merged now? I don't think those test failures are related to this change
yeah, sorry, i added a comment.