touch: move from time to chrono

[tertsdiepraam]

chrono has better support for daylight savings time, so I was able to support that and enable the test for it. It's also just easier to work with, so we can remove a lot of verbosity with this too.

For reviewing, the DST and leap second handling is probably most important. The formats should also be checked, even though they are (hopefully) well-tested.

[sylvestre]

Some rustfmt & clippy warnings

[tertsdiepraam]

Yeah I'll look into those after the PR it depends on is merged.

[sylvestre]

Windows is broken:

---- test_touch::test_touch_mtime_dst_fails stdout ----
run: D:\a\coreutils\coreutils\target\debug\coreutils.exe touch -m -t 202003080200 test_touch_set_mtime_dst_fails
thread 'test_touch::test_touch_mtime_dst_fails' panicked at 'Command was expected to fail.
stdout = 
 stderr = ', tests\by-util\test_touch.rs:627:6
stack backtrace:
   0: std::panicking::begin_panic_handler
             at /rustc/28a29282f6dde2e4aba6e1e4cfea5c9430a00217/library\std\src\panicking.rs:577
   1: core::panicking::panic_fmt
             at /rustc/28a29282f6dde2e4aba6e1e4cfea5c9430a00217/library\core\src\panicking.rs:67
   2: tests::common::util::CmdResult::failure
             at .\tests\common\util.rs:402
   3: tests::common::util::UCommand::fails
             at .\tests\common\util.rs:1568
   4: tests::test_touch::test_touch_mtime_dst_fails
             at .\tests\by-util\test_touch.rs:618
   5: tests::test_touch::test_touch_mtime_dst_fails::closure$0
             at .\tests\by-util\test_touch.rs:608
   6: core::ops::function::FnOnce::call_once<tests::test_touch::test_touch_mtime_dst_fails::closure_env$0,tuple$<> >
             at /rustc/28a29282f6dde2e4aba6e1e4cfea5c9430a00217\library\core\src\ops\function.rs:250
   7: core::ops::function::FnOnce::call_once
             at /rustc/28a29282f6dde2e4aba6e1e4cfea5c9430a00217/library\core\src\ops\function.rs:250
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

[tertsdiepraam]

Yes, I'll look into that :)

[tertsdiepraam]

I bet Windows doesn't work because it does not parse the libc-style timezone syntax. I think it's best if I just disable the test for Windows. At least we get the correct behaviour merged then. Do you agree, @sylvestre?

[sylvestre]

@tertsdiepraam sorry, i missed your question. I am fine with disabling the test on Windows

[tertsdiepraam]

I disabled the test on windows and got this all up to date with main I think :)

[cakebaker]

Somehow I don't see what the purpose is of this and the other similar comments.

[tertsdiepraam]

It's used in this function:

coreutils/src/uu/touch/src/touch.rs

Lines 401 to 415 in 1037304

	let (format, ts) = match s.chars().count() {
	15 => (YYYYMMDDHHMM_DOT_SS, s.to_owned()),
	12 => (YYYYMMDDHHMM, s.to_owned()),
	// If we don't add "20", we have insufficient information to parse
	13 => (YYYYMMDDHHMM_DOT_SS, format!("20{}", s)),
	10 => (YYYYMMDDHHMM, format!("20{}", s)),
	11 => (YYYYMMDDHHMM_DOT_SS, format!("{}{}", current_year(), s)),
	8 => (YYYYMMDDHHMM, format!("{}{}", current_year(), s)),
	_ => {
	return Err(USimpleError::new(
	1,
	format!("invalid date format {}", s.quote()),
	))
	}
	};

It's mostly just a leftover from before, when it was harder to determine the length of the format due to the format strings from the time crate. I can remove it if you want.

[cakebaker]

I would remove them.

[cakebaker]

I would use datetime instead of dt because chrono also uses datetime in its function names.

Suggested change

	fn dt_to_filetime<T: TimeZone>(dt: &DateTime<T>) -> FileTime {
	fn datetime_to_filetime<T: TimeZone>(dt: &DateTime<T>) -> FileTime {

[tertsdiepraam]

Yeah that name was leftover from local_dt_to_filetime. I'll fix it

[cakebaker]

I guess ft is a left over from the previous code. Not sure whether this first sentence is even needed.

[tertsdiepraam]

Yeah I removed the sentence

[cakebaker]

I'm not sure whether there is something missing between "assumes" and "takes" or one of those words can be removed.

[tertsdiepraam]

Indeed, I removed "assumes"