touch: read a date with no zone in it as local time

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`
+}

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 {

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)!

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)