diff --git a/src/touch/datetime.v b/src/touch/datetime.v new file mode 100644 index 00000000..fa8459e6 --- /dev/null +++ b/src/touch/datetime.v @@ -0,0 +1,85 @@ +import time + +// A date string with no zone in it names a *local* time. That is the whole +// point of this file, and it is the one thing the port had wrong: a naive +// string was read as though it were UTC, so on this machine, at +0330, +// +// touch -d '2020-01-02 03:04:05' +// +// set the file to 06:34:05, three and a half hours late, and GNU sets 03:04:05. +// +// The offset cannot come from time.offset(), which reports the zone now. The +// same zone wants a different offset depending on the date, and both of these +// were measured against GNU rather than assumed: +// +// TZ=Asia/Tehran 2021-06-07 08:09:10 wants +0430, and +0330 now +// TZ=America/New_York 2020-01-02 00:00:00 wants -0500, and -0400 now +// +// So the offset has to be the one in force at the date being converted, which +// is what zoneinfo's offset_at answers. +const unresolved = 'no local time zone could be determined' + +// parse_datetime turns the argument of touch -d or -t into an absolute instant. +// A string that carries its own offset is already an instant and is passed +// through; a string without one is read in the local zone. +fn parse_datetime(s string) !i64 { + parsed := time.parse_iso8601(s) or { return error('unable to parse date ${s}') } + if has_explicit_zone(s) { + return parsed.unix() + } + loc := time.load_location('Local') or { return error(unresolved) } + return local_wall_clock(loc, parsed) +} + +// local_wall_clock converts a parsed Time whose calendar fields are local wall +// time into an absolute instant. +// +// parsed.unix() read those fields as if they were UTC, which is the value to +// correct. Subtracting the zone offset at *that* value is one step from the +// answer, and a second step settles it: when the first guess lands on the far +// side of a DST transition the offset it used was the wrong one. Two steps +// agree everywhere except inside the transition hour itself, where GNU also +// has to pick one and there is nothing to check against. +fn local_wall_clock(loc &time.Location, parsed time.Time) i64 { + naive := parsed.unix() + mut guess := naive + for _ in 0 .. 2 { + offset := loc.offset_at(guess) or { break } + next := naive - i64(offset) + if next == guess { + break + } + guess = next + } + return guess +} + +// has_explicit_zone reports whether the string says which zone it is in, which +// is the difference between an instant and a wall clock reading. +// +// The zone can only appear at the end of the time part, and the date has already +// been split off, so a sign inside the time token can only be a zone: times are +// written with ':' and '.', never with '+' or '-'. That makes the length and +// digit checks unnecessary, and getting them wrong is how `+03:30` came to be +// read as unzoned for one round. +fn has_explicit_zone(s string) bool { + mut tp := s.index_('T') + if tp == -1 { + tp = s.index_(' ') + if tp == -1 { + return false + } + } else { + // Step over the T, or the token below still starts with it. + tp++ + } + token := s[tp..].split(' ').last() + if token.len < 2 { + return false + } + if token.contains('+') || token.contains('-') { + return true + } + last := token[token.len - 1] + return last == `Z` || last == `z` +} diff --git a/src/touch/touch.v b/src/touch/touch.v index 92790ab5..27c1598b 100644 --- a/src/touch/touch.v +++ b/src/touch/touch.v @@ -112,10 +112,13 @@ fn get_date_time(args TouchArgs) (int, int) { dt := if args.date_arg.len > 0 { args.date_arg } else { args.time_arg } if dt.len > 0 { - date := time.parse_iso8601(dt) or { - common.exit_with_error_message(app_name, 'unable to parse date ${dt}') + // parse_datetime, not time.parse_iso8601: a string with no zone in it + // names a local time, and the offset that has to be applied is the one + // in force at that date, not the one in force now. See datetime.v. + stamp := parse_datetime(dt) or { + common.exit_with_error_message(app_name, err.msg()) } - return int(date.unix()), int(date.unix()) + return stamp, stamp } if args.reference.len > 0 { diff --git a/src/touch/touch_test.v b/src/touch/touch_test.v index c482e5de..1d9bd5c7 100644 --- a/src/touch/touch_test.v +++ b/src/touch/touch_test.v @@ -3,6 +3,22 @@ module main import os import time +// The expectations in this file were read off GNU touch 9.4, not derived from +// the code under test. The previous version computed them with +// time.parse_iso8601(date).unix(), which is the very call the implementation +// makes, so it agreed with the code by construction and could not have caught +// the bug it existed to guard. Every stamp below is a literal measured under +// TZ=Asia/Tehran, a zone whose offset differs by a full hour between summer and +// winter, which is what makes it worth testing on. +// +// 2022-12-01T11:00:01 -> 1669879801 +// 2022-12-01T11:50:02 -> 1669882802 +// 2023-01-01T20:00:35 -> 1672590635 +// +// A zone with no rules available would make these meaningless, so the tests say +// so rather than quietly passing. +const tehran = 'Asia/Tehran' + fn temp_file_name() string { dir := os.temp_dir() file := '${dir}/t${time.ticks()}' @@ -17,6 +33,23 @@ fn pass() { assert true } +// pin_zone sets TZ for the duration of a test and hands back what to restore. +// The zone has to be forced: on a machine whose own zone happens to be UTC the +// whole bug this file guards is invisible. +fn pin_zone(zone string) string { + saved := os.getenv('TZ') + _ = os.setenv('TZ', zone, true) + return saved +} + +fn zone_available() bool { + time.load_location('Local') or { + p('no local zone information on this machine, skipping the ${tehran} assertions') + return false + } + return true +} + fn test_touch_one_file_no_options() { p(@METHOD) file := temp_file_name() @@ -49,62 +82,151 @@ fn test_touch_no_create_option() { pass() } +// A string with no zone in it names a local time. Read as UTC it lands three +// and a half hours late here, which is what these five used to do. +fn test_parse_datetime_naive_string_is_local() { + p(@METHOD) + saved := pin_zone(tehran) + defer { + _ = os.setenv('TZ', saved, true) + } + if !zone_available() { + return + } + // A date with no time is local midnight, not UTC midnight. + assert parse_datetime('2020-01-02')! == 1_577_910_600 + assert parse_datetime('2020-01-02 03:04:05')! == 1_577_921_645 + assert parse_datetime('2020-01-02T03:04:05')! == 1_577_921_645 + assert parse_datetime('2020-02-29 12:00:00')! == 1_582_965_000 + assert parse_datetime('2025-01-01 00:00:00')! == 1_735_677_000 + pass() +} + +// The offset that applies is the one in force at that date, not the one in force +// now. Tehran is +0430 in June and +0330 in October, so subtracting the current +// offset puts this one an hour out: 08:09:10 - 4:30 = 03:39:10 UTC. +fn test_parse_datetime_uses_the_offset_at_that_date() { + p(@METHOD) + saved := pin_zone(tehran) + defer { + _ = os.setenv('TZ', saved, true) + } + if !zone_available() { + return + } + assert parse_datetime('2021-06-07 08:09:10')! == 1_623_037_150 + // And the same zone in its other period, to show both are in play. + assert parse_datetime('2020-01-02 03:04:05')! == 1_577_921_645 + pass() +} + +// A string that says its zone is already an instant and must not be shifted. +fn test_parse_datetime_leaves_an_explicit_zone_alone() { + p(@METHOD) + saved := pin_zone(tehran) + defer { + _ = os.setenv('TZ', saved, true) + } + if !zone_available() { + return + } + assert parse_datetime('2020-01-02T03:04:05Z')! == 1_577_934_245 + // +03:30 is the zone's own offset in January, so this lands where the naive + // spelling of the same wall clock does. It has to get there on its own. + assert parse_datetime('2020-01-02T03:04:05+03:30')! == 1_577_921_645 + pass() +} + +fn test_has_explicit_zone() { + p(@METHOD) + assert has_explicit_zone('2020-01-02T03:04:05Z') + assert has_explicit_zone('2020-01-02T03:04:05z') + assert has_explicit_zone('2020-01-02T03:04:05+03:30') + assert has_explicit_zone('2020-01-02T03:04:05-05:00') + assert has_explicit_zone('2020-01-02 03:04:05Z') + assert has_explicit_zone('2020-01-02 03:04:05+0330') + + assert !has_explicit_zone('2020-01-02 03:04:05') + assert !has_explicit_zone('2020-01-02T03:04:05') + // The date carries two hyphens and no time part at all. + assert !has_explicit_zone('2020-01-02') + assert !has_explicit_zone('2020-01-02 03:04:05.678') + pass() +} + fn test_touch_create_with_d_option() { p(@METHOD) + saved := pin_zone(tehran) + defer { + _ = os.setenv('TZ', saved, true) + } + if !zone_available() { + return + } file := temp_file_name() - date := '2022-12-01T11:00:01' - unix := time.parse_iso8601(date)!.unix() - touch(['touch', '-d', date, file]) + touch(['touch', '-d', '2022-12-01T11:00:01', file]) stat := os.lstat(file)! - assert stat.atime == unix - assert stat.mtime == unix + assert stat.atime == 1_669_879_801 + assert stat.mtime == 1_669_879_801 os.rm(file)! pass() } fn test_touch_create_with_a_d_option() { p(@METHOD) + saved := pin_zone(tehran) + defer { + _ = os.setenv('TZ', saved, true) + } + if !zone_available() { + return + } file := temp_file_name() - date := '2022-12-01T11:00:01' - unix := time.parse_iso8601(date)!.unix() - touch(['touch', '-a', '-d', date, file]) + touch(['touch', '-a', '-d', '2022-12-01T11:00:01', file]) stat := os.lstat(file)! - assert stat.atime == unix - assert stat.mtime != unix + assert stat.atime == 1_669_879_801 + assert stat.mtime != 1_669_879_801 os.rm(file)! pass() } fn test_touch_create_with_m_d_option() { p(@METHOD) + saved := pin_zone(tehran) + defer { + _ = os.setenv('TZ', saved, true) + } + if !zone_available() { + return + } file := temp_file_name() - date := '2022-12-01T11:00:01' - unix := time.parse_iso8601(date)!.unix() - touch(['touch', '-m', '-d', date, file]) + touch(['touch', '-m', '-d', '2022-12-01T11:00:01', file]) stat := os.lstat(file)! - assert stat.atime != unix - assert stat.mtime == unix + assert stat.atime != 1_669_879_801 + assert stat.mtime == 1_669_879_801 os.rm(file)! pass() } fn test_touch_with_reference_file() { p(@METHOD) + saved := pin_zone(tehran) + defer { + _ = os.setenv('TZ', saved, true) + } + if !zone_available() { + return + } rfile := temp_file_name() - mdate := '2022-12-01T11:00:01' - mtime := time.parse_iso8601(mdate)!.unix() - touch(['touch', '-d', mdate, rfile]) - - adate := '2022-12-01T11:50:02' - atime := time.parse_iso8601(adate)!.unix() - touch(['touch', '-a', '-d', adate, rfile]) + touch(['touch', '-d', '2022-12-01T11:00:01', rfile]) + touch(['touch', '-a', '-d', '2022-12-01T11:50:02', rfile]) file := rfile + 'x' touch(['touch', '-r', rfile, file]) stat := os.lstat(file)! - assert stat.atime == atime - assert stat.mtime == mtime + assert stat.atime == 1_669_882_802 + assert stat.mtime == 1_669_879_801 os.rm(file)! os.rm(rfile)! @@ -113,15 +235,20 @@ fn test_touch_with_reference_file() { fn test_touch_no_reference_option() { p(@METHOD) + saved := pin_zone(tehran) + defer { + _ = os.setenv('TZ', saved, true) + } + if !zone_available() { + return + } file := temp_file_name() - fdate := '2022-12-01T11:00:01' - ftime := time.parse_iso8601(fdate)!.unix() - touch(['touch', '-d', fdate, file]) + touch(['touch', '-d', '2022-12-01T11:00:01', file]) // confirm correct start state stat := os.lstat(file)! - assert stat.atime == ftime - assert stat.mtime == ftime + assert stat.atime == 1_669_879_801 + assert stat.mtime == 1_669_879_801 if os.user_os() == 'windows' { eprintln('skip symlink checks on windows, they need administrative permissions') @@ -132,19 +259,17 @@ fn test_touch_no_reference_option() { os.symlink(file, link)! // touch the symlink - ldate := '2023-01-01T20:00:35' - ltime := time.parse_iso8601(ldate)!.unix() - touch(['touch', '-h', '-d', ldate, link]) + touch(['touch', '-h', '-d', '2023-01-01T20:00:35', link]) // check original file fstat := os.lstat(file)! - assert fstat.atime == ftime - assert fstat.mtime == ftime + assert fstat.atime == 1_669_879_801 + assert fstat.mtime == 1_669_879_801 // lstat does not 'follow' links lstat := os.lstat(link)! - assert lstat.atime == ltime - assert lstat.mtime == ltime + assert lstat.atime == 1_672_590_635 + assert lstat.mtime == 1_672_590_635 os.rm(link)! os.rm(file)! diff --git a/src/touch/touch_windows.c.v b/src/touch/touch_windows.c.v index b99e146c..19657b54 100644 --- a/src/touch/touch_windows.c.v +++ b/src/touch/touch_windows.c.v @@ -1,4 +1,7 @@ -fn C.SetFileTime(hfile u32, const_creation_time voidptr, const_access_time voidptr, const_modification_time voidptr) bool +// The handle is a voidptr because that is what vlib's C.CreateFileW returns; +// it used to return u32, and declaring the parameter as u32 is what made this +// file stop compiling on current V. A HANDLE is a pointer either way. +fn C.SetFileTime(hfile voidptr, const_creation_time voidptr, const_access_time voidptr, const_modification_time voidptr) bool fn lutime(path string, acctime int, modtime int) ! { creation_time := t2filetime(-1)