update time crate to 0.3 #3319

g-k · 2022-03-27T16:30:18Z

This is a draft PR to fix #2884 it still needs some clean up work, but I'd like to get feedback on this approach and the scope of the issue.

Changes:

update the time crate to 0.3

Caveats / Questions:

How does this project want to handle datetimes and TZ data? This PR disables touch filetime printing daylight savings time tests. time 0.3 removes timezone support and the Timespec DST flag (upstream issue Timezones a-la chrono-tz time-rs/time#351). Unicode icu4x might provide longer term stability and allow us to localize dates and times for strftime %b and %c currently we assume LC_ALL=C e.g. from Create an Ideal Components Bag / Skeleton for DateTimeFormat unicode-org/icu4x#1317
Is Update the time crate to 0.3 #2884 for upgrading direct usage of time 0.1 or removing it from the dep tree entirely? This PR still uses time 0.1 indirectly via chrono.
How much do we care about RUSTSEC-2020-0071? To fix this we'd need chrono to update to time 0.3 and bump time chronotope/chrono#639 (comment) suggests they aren't planning to. I can see spinning this off into a separate issue.
Related to ^ this PR uses the time 0.3 uses unsound_local_offset config. My read is that racing multithreaded reads of env vars are unlikely to be an issue for single threaded, short lived CLI utils, but I can do more in this PR to isolate and limit the number of fallible calls e.g. read offset once at load.

Tests:

RUSTFLAGS="--cfg unsound_local_offset" cargo test --features=feat_require_unix_utmpx,feat_common_core passes locally with version:

rustc --version --verbose
rustc 1.58.1 (db9d1b20b 2022-01-20)
binary: rustc
commit-hash: db9d1b20bba1968c1ec1fc49616d4742c1725b4b
commit-date: 2022-01-20
host: aarch64-apple-darwin
release: 1.58.1
LLVM version: 13.0.0

sylvestre · 2022-03-27T16:53:30Z

cool stuff, bravo :)

Answer:

about sec

I don't care about that RUSTSEC-2020-0071, afaik, nobody is using this crate in production and rust sec is less worrying that C's

How does this project want to handle datetimes and TZ data?

Just like GNU :)

about time 0.1

I would like to see that update to 0.3 everywhere (or switch to chrono)

jfinkels · 2022-03-27T17:16:36Z

See also #3300

sylvestre · 2022-04-15T15:28:07Z

@g-k sorry for the ping :) do you have an update on this? This is the main blocker to have it in Debian :)
thanks

g-k · 2022-04-19T12:36:57Z

Oh getting it in Debian is exciting!

I'm not sure how to fix the remaining errors and I don't think I'll have time to finish it in a reasonable amount of time.

Happy to hand it off if someone else wants to pick it up.

sylvestre · 2022-04-26T10:41:31Z

I think I fixed all the issues but one:

$ stat -c %z /bin/sh   
2021-11-26 17:20:49.310012278 +0100
$ ./target/debug/coreutils stat -c %z /bin/sh
2021-11-26 18:20:49.310012278 +0200

(note the +01 vs +02 for the TZ)

while the following work:

$ stat -c %z Cargo.toml
2022-04-23 21:46:14.564134437 +0200
$ ./target/debug/coreutils stat -c %z Cargo.toml
2022-04-23 21:46:14.564134437 +0200

Not sure what is going on?
Maybe a DST issue when the file was created?!

tertsdiepraam · 2022-04-26T11:21:21Z

Something related to DST seems likely. The dates seem to match with when the timezone changes. If I touch 2 files and give them the dates before and after DST starts I get the same behaviour (all using GNU):

❯ touch -t 03260500 test1 # March 26 5:00
❯ touch -t 03280500 test2 # March 28 5:00, DST starts March 27
❯ stat -c %x test1
2022-03-26 05:00:00.000000000 +0100
❯ stat -c %x test2
2022-03-28 05:00:00.000000000 +0200

(I use %x because the creation time is not affected by touch).

Using uutils (from this PR):

❯ uu_stat -c %x test1
2022-03-26 05:00:00.0 +0200
❯ uu_stat -c %x test2
2022-03-28 05:00:00.0 +0200

Note also the difference in rounding.

sylvestre · 2022-04-26T18:31:07Z

Probably https://github.com/uutils/coreutils/blob/492f4fb84c56bd349dcabd72d4a11d71c3c7ad95/src/uucore/src/lib/features/fsext.rs#L744=

sylvestre · 2022-04-27T06:38:42Z

Warning: Congrats! The gnu test tests/touch/relative is no longer failing!
Error: GNU test failed: tests/touch/60-seconds. tests/touch/60-seconds is passing on 'main'. Maybe you have to rebase?

sylvestre · 2022-04-27T20:29:57Z

The GNU failure is

$ ./target/debug/coreutils touch -t 197001010000.60 f      
touch: invalid date ts format '197001010000.60'

tertsdiepraam

Wow, they made a lot of breaking changes in the time crate! Really nice to see this completed :)

tertsdiepraam · 2022-04-27T22:29:05Z

src/uu/touch/src/touch.rs

+    // // We have to check that ft is valid time. Due to daylight saving
+    // // time switch, local time can jump from 1:59 AM to 3:00 AM,
+    // // in which case any time between 2:00 AM and 2:59 AM is not valid.
+    // // Convert back to local time and see if we got the same value back.
+    // let ts = time::Timespec {
+    //     sec: ft.unix_seconds(),
+    //     nsec: 0,
+    // };
+    // let tm2 = time::at(ts);
+    // if tm.tm_hour != tm2.tm_hour {
+    //     return Err(USimpleError::new(
+    //         1,
+    //         format!("invalid date format {}", s.quote()),
+    //     ));
+    // }


Why is this commented out? Is it no longer necessary or is it a remaining thing to port?

I missed that, @g-k do you remember ? :)

it isn't breaking any test on our side or GNU's
So, I am in favor of remove this piece of code

Ugh yeah. This is a remaining thing to port, but also an edge case and probably hard to run into in practice.

IIRC time 0.3 dropped TZ support and removed the Tm struct and tm_isdst flag, so I wasn't sure how to proceed since it didn't look like there were good alternatives.

Dropping the code and waiting for support in icu4x seems like a reasonable path to me (icu would also potentially let coreutils handle localized strftime args).

Mentioned it a bit in this part of #3319 (comment) but I had the wrong struct and flag name:

How does this project want to handle datetimes and TZ data? This PR disables touch filetime printing daylight savings time tests. time 0.3 removes timezone support and the Timespec DST flag (upstream issue time-rs/time#351). Unicode icu4x might provide longer term stability and allow us to localize dates and times for strftime %b and %c currently we assume LC_ALL=C e.g. from unicode-org/icu4x#1317

src/uucore/Cargo.toml

src/uucore/src/lib/features/fsext.rs

sylvestre · 2022-04-29T18:13:20Z

Actually, I am wrong:
Warning: Changes from 'main': PASS -2 / FAIL -1 / ERROR +3 / SKIP +0

I will have a look to the errors

Bumps [time](https://github.com/time-rs/time) from 0.1.43 to 0.3.9. - [Release notes](https://github.com/time-rs/time/releases) - [Changelog](https://github.com/time-rs/time/blob/main/CHANGELOG.md) - [Commits](time-rs/time@v0.1.43...v0.3.9) --- updated-dependencies: - dependency-name: time dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com>

And no longer ignore RUSTSEC-2022-0013

sylvestre · 2022-05-08T21:13:30Z

Finally! :)
Warning: Changes from 'main': PASS +0 / FAIL +0 / ERROR +0 / SKIP +0

Not perfect. I am not happy with the long list of format_description in touch.rs
https://github.com/uutils/coreutils/pull/3319/files#diff-9565d8fad7b29837b3b468a61a809dc187fb351a72421ffe4b7c4879a8445b27R264=

I will land it to iterate from it

reported:
#3505

tertsdiepraam · 2022-05-08T22:16:34Z

Finally! This is amazing!

sylvestre force-pushed the 2884-time-0.3 branch from ffb2e2b to aef63de Compare March 27, 2022 16:50

g-k force-pushed the 2884-time-0.3 branch 3 times, most recently from a36fbbf to 4d95f89 Compare April 3, 2022 20:38

sylvestre force-pushed the 2884-time-0.3 branch 2 times, most recently from a44f081 to 3fc5b37 Compare April 12, 2022 07:54

sylvestre requested a review from tertsdiepraam April 26, 2022 21:25

sylvestre marked this pull request as ready for review April 27, 2022 06:38

sylvestre force-pushed the 2884-time-0.3 branch from 79ad60e to 6c6e55f Compare April 27, 2022 06:44

tertsdiepraam approved these changes Apr 27, 2022

View reviewed changes

sylvestre force-pushed the 2884-time-0.3 branch from 6c4362a to 9c998c7 Compare April 29, 2022 08:32

sylvestre added 9 commits May 1, 2022 16:58

build in verbose mode (cfg isn't used)

f810b55

time: Various fixes

3a576f2

time: Force the display of the tz sign

e23dd68

stat: add a test to verify time easily

326dc50

time: take in account the local tz

10eaaae

add time 0.1.44 to cargo deny

3b3585b

And no longer ignore RUSTSEC-2022-0013

workaround the tests/touch/60-seconds test to skip leap second

c009e1b

time: Improve the l&f

2b11d77

sylvestre added 2 commits May 1, 2022 17:01

fix gnu/tests/touch/60-seconds

31c28ee

touch: add support of -d '1970-01-01 18:43:33.023456789'

9d81d6f

sylvestre force-pushed the 2884-time-0.3 branch from 9c998c7 to 9d81d6f Compare May 1, 2022 15:02

GNU CI: show the new error

417b4a2

sylvestre mentioned this pull request May 1, 2022

build(deps): bump time from 0.1.43 to 0.3.9 #3300

Closed

sylvestre added 4 commits May 2, 2022 18:14

Merge branch 'main' into 2884-time-0.3

9b69e6f

Merge branch 'main' into 2884-time-0.3

0b9d703

touch: add support of -d '1970-01-01 18:43:33.023456789'

e70b99d

to_local: manage the error

65d0f5b

sylvestre force-pushed the 2884-time-0.3 branch 2 times, most recently from a201b23 to f5d1021 Compare May 6, 2022 06:41

touch: improve the -d option support of other dates

06ef89b

sylvestre force-pushed the 2884-time-0.3 branch from f5d1021 to 06ef89b Compare May 6, 2022 07:23

sylvestre added 5 commits May 6, 2022 23:54

fix the GNU error detection

39520a8

Merge branch 'main' into 2884-time-0.3

56264eb

Do not use the Rust/touch for tests/ls/abmon-align.sh

087d4b1

also support for tests/touch/no-rights.sh format

d556984

also support for tests/touch/relative.sh

f65d72e

sylvestre merged commit 779eca4 into uutils:main May 8, 2022

g-k deleted the 2884-time-0.3 branch May 8, 2022 22:31

tertsdiepraam mentioned this pull request Mar 23, 2023

Default environment variables for tests #4593

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update time crate to 0.3 #3319

update time crate to 0.3 #3319

g-k commented Mar 27, 2022

sylvestre commented Mar 27, 2022

jfinkels commented Mar 27, 2022

sylvestre commented Apr 15, 2022

g-k commented Apr 19, 2022

sylvestre commented Apr 26, 2022

tertsdiepraam commented Apr 26, 2022 •

edited

Loading

sylvestre commented Apr 26, 2022

sylvestre commented Apr 27, 2022

sylvestre commented Apr 27, 2022

tertsdiepraam left a comment

tertsdiepraam Apr 27, 2022

sylvestre Apr 28, 2022

sylvestre Apr 29, 2022

g-k Apr 29, 2022

sylvestre commented Apr 29, 2022 •

edited

Loading

sylvestre commented May 8, 2022

tertsdiepraam commented May 8, 2022

update time crate to 0.3 #3319

update time crate to 0.3 #3319

Conversation

g-k commented Mar 27, 2022

sylvestre commented Mar 27, 2022

jfinkels commented Mar 27, 2022

sylvestre commented Apr 15, 2022

g-k commented Apr 19, 2022

sylvestre commented Apr 26, 2022

tertsdiepraam commented Apr 26, 2022 • edited Loading

sylvestre commented Apr 26, 2022

sylvestre commented Apr 27, 2022

sylvestre commented Apr 27, 2022

tertsdiepraam left a comment

Choose a reason for hiding this comment

tertsdiepraam Apr 27, 2022

Choose a reason for hiding this comment

sylvestre Apr 28, 2022

Choose a reason for hiding this comment

sylvestre Apr 29, 2022

Choose a reason for hiding this comment

g-k Apr 29, 2022

Choose a reason for hiding this comment

sylvestre commented Apr 29, 2022 • edited Loading

sylvestre commented May 8, 2022

tertsdiepraam commented May 8, 2022

tertsdiepraam commented Apr 26, 2022 •

edited

Loading

sylvestre commented Apr 29, 2022 •

edited

Loading